mirror of
https://github.com/BerriAI/litellm.git
synced 2026-10-07 02:59:05 +00:00
[Fix] v2 migration resolver: address Greptile review findings
- Open the psycopg connection in `_warn_if_db_ahead_of_head` with autocommit=True. Without it, psycopg3's `with conn` calls COMMIT on clean exit, which fails after the `UndefinedTable` (fresh-DB) branch left the transaction in an aborted state — crashing first-run startups. - Wrap the v2 `prisma db push` path in try/except and raise RuntimeError on CalledProcessError/TimeoutExpired. Otherwise these propagate past proxy_cli.py's `except RuntimeError` as unhandled tracebacks. - Reword the loop-exhaustion error to cover the non-timeout exit path (repeated P3005/P3009/P3018 idempotent-recovery `continue`s), not just persistent timeouts. Adds a unit test for the db_push error wrapping.
This commit is contained in:
parent
88b1823f51
commit
9049f37864
2 changed files with 30 additions and 3 deletions
|
|
@ -456,7 +456,13 @@ class ProxyExtrasDBManager:
|
|||
known = set(ProxyExtrasDBManager._get_migration_names(migrations_dir))
|
||||
|
||||
try:
|
||||
with psycopg.connect(cleaned_url, connect_timeout=10) as conn:
|
||||
# autocommit=True keeps the SELECT outside a transaction. Without
|
||||
# it, psycopg3's `with conn` calls COMMIT on clean exit — which
|
||||
# fails after `UndefinedTable` (fresh DB) leaves the transaction
|
||||
# in an aborted state.
|
||||
with psycopg.connect(
|
||||
cleaned_url, connect_timeout=10, autocommit=True
|
||||
) as conn:
|
||||
try:
|
||||
rows = conn.execute(
|
||||
"SELECT migration_name FROM _prisma_migrations "
|
||||
|
|
@ -521,6 +527,13 @@ class ProxyExtrasDBManager:
|
|||
env=_get_prisma_env(),
|
||||
)
|
||||
return True
|
||||
except (
|
||||
subprocess.CalledProcessError,
|
||||
subprocess.TimeoutExpired,
|
||||
) as e:
|
||||
# Re-raise as RuntimeError so proxy_cli.py's
|
||||
# `except RuntimeError` catches it and exits cleanly.
|
||||
raise RuntimeError(f"prisma db push failed.\n\nDetail: {e}") from e
|
||||
finally:
|
||||
os.chdir(original_dir)
|
||||
|
||||
|
|
@ -624,8 +637,10 @@ class ProxyExtrasDBManager:
|
|||
) from e
|
||||
|
||||
raise RuntimeError(
|
||||
"Database migration failed after 4 attempts (persistent timeouts). "
|
||||
"Check database connectivity and load."
|
||||
"Database migration failed after 4 attempts (retry loop "
|
||||
"exhausted by timeouts or repeated idempotent-recovery "
|
||||
"continues). Check database connectivity, load, and "
|
||||
"_prisma_migrations ledger state."
|
||||
)
|
||||
finally:
|
||||
os.chdir(original_dir)
|
||||
|
|
|
|||
|
|
@ -132,6 +132,18 @@ def test_v1_default_still_calls_resolve_all_migrations(monkeypatch, tmp_path):
|
|||
assert resolve_called["n"] == 1, "v1 default should still invoke the legacy path"
|
||||
|
||||
|
||||
def test_v2_db_push_wraps_subprocess_error_as_runtime_error(monkeypatch, tmp_path):
|
||||
"""v2: a failing `prisma db push` must raise RuntimeError, not leak
|
||||
CalledProcessError past proxy_cli.py's `except RuntimeError`."""
|
||||
monkeypatch.setattr(ProxyExtrasDBManager, "_get_prisma_dir", lambda: str(tmp_path))
|
||||
(tmp_path / "schema.prisma").write_text("// stub")
|
||||
|
||||
stderr = "db push error"
|
||||
with patch("subprocess.run", side_effect=_fake_migrate_deploy_failure(1, stderr)):
|
||||
with pytest.raises(RuntimeError, match="prisma db push failed"):
|
||||
ProxyExtrasDBManager.setup_database(use_migrate=False, use_v2_resolver=True)
|
||||
|
||||
|
||||
def test_v2_does_not_call_resolve_all_migrations(monkeypatch, tmp_path):
|
||||
"""v2 must never call _resolve_all_migrations — that's the bug it fixes."""
|
||||
monkeypatch.setattr(
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue