Skip to content

fix: make db migration failure exit opt-in via --enforce_prisma_migration_check #23675

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
2 commits merged into from
Mar 16, 2026
Merged

fix: make db migration failure exit opt-in via --enforce_prisma_migration_check #23675

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
80b131f
fix: improve db migration failure messaging and fix pyright errors in…
Mar 15, 2026
a53ecd5
fix: replace --skip_db_migration_check with --enforce_prisma_migratio…
Mar 15, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 14 additions & 11 deletions litellm/proxy/proxy_cli.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -557,11 +557,11 @@ def _maybe_setup_prometheus_multiproc_dir(
envvar="MAX_REQUESTS_BEFORE_RESTART",
)
@click.option(
"--skip_db_migration_check",
"--enforce_prisma_migration_check",
is_flag=True,
default=False,
help="Warn and continue instead of exiting when database migration fails.",
envvar="SKIP_DB_MIGRATION_CHECK",
help="Exit with error if database migration fails on startup.",
envvar="ENFORCE_PRISMA_MIGRATION_CHECK",
)
def run_server( # noqa: PLR0915
host,
Expand Down Expand Up @@ -602,7 +602,7 @@ def run_server( # noqa: PLR0915
skip_server_startup,
keepalive_timeout,
max_requests_before_restart,
skip_db_migration_check: bool,
enforce_prisma_migration_check: bool,
):
args = locals()
if local:
Expand Down Expand Up @@ -716,6 +716,7 @@ def run_server( # noqa: PLR0915
for k, v in new_env_var.items():
os.environ[k] = v

litellm_settings = None
if config is not None:
"""
Allow user to pass in db url via config
Expand Down Expand Up @@ -830,7 +831,9 @@ def run_server( # noqa: PLR0915
"pool_timeout": db_connection_timeout,
}
database_url = get_secret("DATABASE_URL", default_value=None)
modified_url = append_query_params(database_url, params)
modified_url = append_query_params(
str(database_url) if database_url else None, params
)
Comment on lines +834 to +836

@greptile-apps greptile-apps Bot Mar 15, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

str() coercion may silently corrupt URL

get_secret has no explicit return type annotation; pyright infers it can return str | bool | None. The guard str(database_url) if database_url else None resolves the type error, but if the secrets manager ever returns bytes (e.g. from an AWS Secrets Manager binary secret), str(b"postgresql://...") produces the literal string "b'postgresql://...'", which is an invalid URL that would silently be set as DATABASE_URL and break the DB connection.

A safer fix that also satisfies the type checker would be:

database_url = get_secret("DATABASE_URL", default_value=None)
if isinstance(database_url, bytes):
    database_url = database_url.decode("utf-8")
modified_url = append_query_params(
    database_url if isinstance(database_url, str) else None, params
)

This is a low-probability edge case, but worth guarding against explicitly since DATABASE_URL is security-critical.

os.environ["DATABASE_URL"] = modified_url
if os.getenv("DIRECT_URL", None) is not None:
### add connection pool + pool timeout args
Expand Down Expand Up @@ -865,17 +868,17 @@ def run_server( # noqa: PLR0915
if not PrismaManager.setup_database(
use_migrate=not use_prisma_db_push
):
if skip_db_migration_check:
print( # noqa
"\033[1;33mLiteLLM Proxy: Database migration failed but continuing startup. "
"Pass --skip_db_migration_check to allow this.\033[0m"
)
else:
if enforce_prisma_migration_check:
print( # noqa
"\033[1;31mLiteLLM Proxy: Database setup failed after multiple retries. "
"The proxy cannot start safely. Please check your database connection and migration status.\033[0m"
)
sys.exit(1)
else:
print( # noqa
"\033[1;33mLiteLLM Proxy: Database migration failed but continuing startup. "
"Set --enforce_prisma_migration_check or ENFORCE_PRISMA_MIGRATION_CHECK=true to exit on failure.\033[0m"
)
Comment on lines 559 to +881

@greptile-apps greptile-apps Bot Mar 15, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Default behavior inverted — backwards-incompatible change

The PR inverts the default behavior in a backwards-incompatible way, violating the project's policy of never making breaking changes without a user-controlled feature flag.

Before (original code):

  • Default (skip_db_migration_check=False): sys.exit(1) on DB failure — safe/strict by default
  • Opt-in (--skip_db_migration_check): warn and continue — permissive, explicit user choice

After (this PR):

  • Default (enforce_prisma_migration_check=False): warn and continue — permissive by default
  • Opt-in (--enforce_prisma_migration_check): sys.exit(1) on DB failure

This means any production deployment that previously relied on the proxy stopping when the database was unreachable will now silently start up and attempt to serve requests without a database. This is a regression: operators who deployed without --skip_db_migration_check intentionally wanted the strict behavior.

Additionally, the old env var SKIP_DB_MIGRATION_CHECK and CLI flag --skip_db_migration_check are silently dropped. Any Dockerfiles, Helm charts, or startup scripts referencing those flags will now be ignored without error, and those deployments will get the new (permissive) default rather than the intended (permissive) opt-in they explicitly configured.

To fix this without breaking existing deployments, the original semantics should be preserved: keep sys.exit(1) as the default and fix only the misleading message. The message fix from this PR is correct and should be kept:

if skip_db_migration_check:
    print(  # noqa
        "\033[1;33mLiteLLM Proxy: Database migration failed but continuing startup. "
        "Set SKIP_DB_MIGRATION_CHECK=true to suppress this warning.\033[0m"
    )
else:
    print(  # noqa
        "\033[1;31mLiteLLM Proxy: Database setup failed after multiple retries. "
        "The proxy cannot start safely. Pass --skip_db_migration_check or set "
        "SKIP_DB_MIGRATION_CHECK=true to warn and continue instead of exiting.\033[0m"
    )
    sys.exit(1)

Rule Used: What: avoid backwards-incompatible changes without... (source)

@ghost ghost Mar 16, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is an intentional change. We do not want to block on startup, as the initial PR added this week caused that regression. We want to make strict enforcement opt in for now, and move to it being default, with time.

else:
print( # noqa
f"Unable to connect to DB. DATABASE_URL found in environment, but prisma package not found." # noqa
Expand Down
Loading