mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-21 00:21:49 +00:00
Fix P3009 idempotent re-deploy, error propagation, and test coverage
- P3009 idempotent path now re-runs prisma migrate deploy after resolving the failed migration, so subsequent migrations are not left unapplied - _mark_all_migrations_applied raises RuntimeError on unexpected errors instead of silently swallowing them - P3009 non-idempotent test now exercises the rollback-succeeds path and verifies rollback was called Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
parent
06b12ad985
commit
347090d288
2 changed files with 75 additions and 6 deletions
|
|
@ -274,10 +274,14 @@ class ProxyExtrasDBManager:
|
|||
)
|
||||
logger.debug(f"Resolved migration: {migration_name}")
|
||||
except subprocess.CalledProcessError as e:
|
||||
if "is already recorded as applied in the database." not in e.stderr:
|
||||
logger.warning(
|
||||
f"Failed to resolve migration {migration_name}: {e.stderr}"
|
||||
if "is already recorded as applied in the database." in e.stderr:
|
||||
logger.debug(
|
||||
f"Migration {migration_name} already recorded as applied"
|
||||
)
|
||||
else:
|
||||
raise RuntimeError(
|
||||
f"Failed to mark migration {migration_name} as applied: {e.stderr}"
|
||||
) from e
|
||||
|
||||
@staticmethod
|
||||
def setup_database(use_migrate: bool = False) -> bool:
|
||||
|
|
@ -340,8 +344,19 @@ class ProxyExtrasDBManager:
|
|||
failed_migration
|
||||
)
|
||||
logger.info(
|
||||
f"✅ Migration {failed_migration} resolved."
|
||||
f"✅ Migration {failed_migration} resolved. Re-deploying remaining migrations..."
|
||||
)
|
||||
# Re-run deploy to apply any migrations after the resolved one
|
||||
result = subprocess.run(
|
||||
[_get_prisma_command(), "migrate", "deploy"],
|
||||
timeout=60,
|
||||
check=True,
|
||||
capture_output=True,
|
||||
text=True,
|
||||
env=_get_prisma_env(),
|
||||
)
|
||||
logger.info(f"prisma migrate deploy stdout: {result.stdout}")
|
||||
logger.info("✅ All migrations applied.")
|
||||
return True
|
||||
else:
|
||||
logger.error(
|
||||
|
|
|
|||
|
|
@ -197,6 +197,22 @@ class TestMarkAllMigrationsApplied:
|
|||
# Should not raise
|
||||
ProxyExtrasDBManager._mark_all_migrations_applied("/fake/migrations/dir")
|
||||
|
||||
@patch("litellm_proxy_extras.utils.subprocess.run")
|
||||
@patch.object(
|
||||
ProxyExtrasDBManager,
|
||||
"_get_migration_names",
|
||||
return_value=["20250326162113_baseline"],
|
||||
)
|
||||
def test_raises_on_unexpected_error(self, mock_get_names, mock_run):
|
||||
"""Verify non-'already applied' errors are propagated, not silently swallowed"""
|
||||
mock_run.side_effect = subprocess.CalledProcessError(
|
||||
1,
|
||||
"prisma",
|
||||
stderr="connection refused",
|
||||
)
|
||||
with pytest.raises(RuntimeError, match="Failed to mark migration"):
|
||||
ProxyExtrasDBManager._mark_all_migrations_applied("/fake/migrations/dir")
|
||||
|
||||
|
||||
class TestSetupDatabaseFailFast:
|
||||
"""Test that setup_database fails fast on non-recoverable migration errors"""
|
||||
|
|
@ -211,17 +227,23 @@ class TestSetupDatabaseFailFast:
|
|||
self, mock_run, mock_dir, mock_getcwd, mock_chdir
|
||||
):
|
||||
"""P3009 with non-idempotent error should raise RuntimeError, not silently retry"""
|
||||
error = subprocess.CalledProcessError(
|
||||
deploy_error = subprocess.CalledProcessError(
|
||||
1,
|
||||
"prisma",
|
||||
stderr="P3009: migrate found failed migrations in the target database, `20250329084805_new_cron_job_table` migration. Error: syntax error at or near 'ALTR'",
|
||||
output="",
|
||||
)
|
||||
mock_run.side_effect = error
|
||||
# First call (migrate deploy) raises P3009; subsequent calls (roll_back) succeed
|
||||
mock_run.side_effect = [deploy_error, MagicMock(returncode=0)]
|
||||
|
||||
with pytest.raises(RuntimeError, match="requires manual intervention"):
|
||||
ProxyExtrasDBManager.setup_database(use_migrate=True)
|
||||
|
||||
# Verify rollback was called (second subprocess call)
|
||||
assert mock_run.call_count == 2
|
||||
rollback_cmd = mock_run.call_args_list[1][0][0]
|
||||
assert "--rolled-back" in rollback_cmd
|
||||
|
||||
@patch("litellm_proxy_extras.utils.os.chdir")
|
||||
@patch("litellm_proxy_extras.utils.os.getcwd", return_value="/original")
|
||||
@patch.object(
|
||||
|
|
@ -246,6 +268,38 @@ class TestSetupDatabaseFailFast:
|
|||
# Should fail on first attempt, not retry
|
||||
assert mock_run.call_count == 1
|
||||
|
||||
@patch("litellm_proxy_extras.utils.os.chdir")
|
||||
@patch("litellm_proxy_extras.utils.os.getcwd", return_value="/original")
|
||||
@patch.object(
|
||||
ProxyExtrasDBManager, "_get_prisma_dir", return_value="/fake/prisma/dir"
|
||||
)
|
||||
@patch("litellm_proxy_extras.utils.subprocess.run")
|
||||
def test_p3009_idempotent_redeploys_remaining_migrations(
|
||||
self, mock_run, mock_dir, mock_getcwd, mock_chdir
|
||||
):
|
||||
"""P3009 with idempotent error should resolve the failed migration then re-deploy"""
|
||||
deploy_error = subprocess.CalledProcessError(
|
||||
1,
|
||||
"prisma",
|
||||
stderr="P3009: migrate found failed migrations in the target database, `20250329084805_new_cron_job_table` migration. Error: column 'status' already exists",
|
||||
output="",
|
||||
)
|
||||
# Call sequence: deploy (fails P3009), roll_back, resolve, re-deploy (succeeds)
|
||||
mock_run.side_effect = [
|
||||
deploy_error,
|
||||
MagicMock(returncode=0), # roll_back
|
||||
MagicMock(returncode=0), # resolve
|
||||
MagicMock(stdout="All migrations applied", returncode=0), # re-deploy
|
||||
]
|
||||
|
||||
result = ProxyExtrasDBManager.setup_database(use_migrate=True)
|
||||
|
||||
assert result is True
|
||||
assert mock_run.call_count == 4
|
||||
# Last call should be prisma migrate deploy (re-deploy)
|
||||
last_cmd = mock_run.call_args_list[3][0][0]
|
||||
assert last_cmd == ["prisma", "migrate", "deploy"]
|
||||
|
||||
@patch("litellm_proxy_extras.utils.os.chdir")
|
||||
@patch("litellm_proxy_extras.utils.os.getcwd", return_value="/original")
|
||||
@patch.object(
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue