From 347090d288863927f5ae6f3da86fb4f23402b366 Mon Sep 17 00:00:00 2001 From: yuneng-jiang Date: Thu, 12 Mar 2026 11:21:16 -0700 Subject: [PATCH] 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 --- .../litellm_proxy_extras/utils.py | 23 ++++++-- .../test_litellm_proxy_extras_utils.py | 58 ++++++++++++++++++- 2 files changed, 75 insertions(+), 6 deletions(-) diff --git a/litellm-proxy-extras/litellm_proxy_extras/utils.py b/litellm-proxy-extras/litellm_proxy_extras/utils.py index 16406dd15fa..58c4e174867 100644 --- a/litellm-proxy-extras/litellm_proxy_extras/utils.py +++ b/litellm-proxy-extras/litellm_proxy_extras/utils.py @@ -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( diff --git a/tests/litellm-proxy-extras/test_litellm_proxy_extras_utils.py b/tests/litellm-proxy-extras/test_litellm_proxy_extras_utils.py index d9dfd9366fa..af136e8f96f 100644 --- a/tests/litellm-proxy-extras/test_litellm_proxy_extras_utils.py +++ b/tests/litellm-proxy-extras/test_litellm_proxy_extras_utils.py @@ -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(