fix(proxy): make prisma generate and db push work out-of-the-box after pip install#30051
Conversation
Greptile SummaryThis PR fixes three failure modes that occur when the LiteLLM proxy is invoked by absolute path (Docker, Proxmox-style scripts) without an activated venv: Prisma can't find
Confidence Score: 5/5The change is additive: a PATH prepend in _get_prisma_env() and a new CLI command. Existing Prisma subprocess calls that already passed env= are unaffected; the one missing call now gets it. No authentication, routing, or request-handling code is touched. The PATH injection is isolated to the Prisma subprocess environment builder, all existing subprocess calls already used _get_prisma_env(), and the one missing env= pass is straightforward. The new db generate command is purely additive and guarded by an _PROXY_EXTRAS_AVAILABLE check. litellm/proxy/client/cli/commands/db.py — _get_venv_scripts_dir() is never called from the production code path and may confuse future readers about how PATH injection actually works.
|
| Filename | Overview |
|---|---|
| litellm-proxy-extras/litellm_proxy_extras/utils.py | Adds PATH injection via sysconfig to _get_prisma_env(); adds env= to the v1 db push subprocess call that was missing it |
| litellm/proxy/client/cli/commands/db.py | New db generate command; _get_venv_scripts_dir() is defined but never called in any production code path — it exists only for tests |
| litellm/proxy/client/cli/main.py | Registers db command group in the CLI; straightforward two-line change |
| litellm-proxy-extras/tests/test_prisma_env_path_injection.py | Unit + integration tests for _get_prisma_env() PATH injection; covers injection, promotion, dedup, fallback, and offline-mode coexistence |
| tests/test_litellm/proxy/client/cli/test_db_commands.py | Tests for db generate command; contains fragile sys.path.insert relying on CWD |
| tests/test_litellm/proxy/client/cli/test_db_generate_path_verifier.py | Adversarial tests for the PATH fix delegation chain and None-guard; thorough coverage of edge cases |
| tests/proxy_cli_integration_tests/README.md | New README for integration test directory; may conflict with policy requiring docs in the litellm-docs repo |
| tests/proxy_cli_integration_tests/test_db_generate_integration.py | Integration tests for absolute-path invocation; correctly opt-in via env var and pytest.skip guards |
| pyproject.toml | Adds integration pytest marker; no console-script registration change (litellm-proxy was already registered) |
Reviews (4): Last reviewed commit: "ci: retrigger checks (HF hub 429 flake i..." | Re-trigger Greptile
| def _patch_db_deps(prisma_dir="/fake/prisma/dir", schema_exists=True, returncode=0): | ||
| """Return a list of context managers that patch all external dependencies of db generate.""" | ||
| mock_run = MagicMock() | ||
| if returncode == 0: | ||
| mock_run.return_value = MagicMock(returncode=0) | ||
| else: | ||
| mock_run.side_effect = subprocess.CalledProcessError(returncode, "prisma") | ||
|
|
||
| return ( | ||
| patch( | ||
| "litellm.proxy.client.cli.commands.db.ProxyExtrasDBManager._get_prisma_dir", | ||
| return_value=prisma_dir, | ||
| ), | ||
| patch("litellm.proxy.client.cli.commands.db._get_prisma_command", return_value="prisma"), | ||
| patch("litellm.proxy.client.cli.commands.db._get_prisma_env", return_value=None), | ||
| patch("os.path.exists", return_value=schema_exists), | ||
| patch("subprocess.run", mock_run), | ||
| ) |
There was a problem hiding this comment.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
|
Stephen Chin seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
| """ | ||
|
|
||
| import os | ||
| import subprocess |
|
|
||
| import os | ||
| import subprocess | ||
| import sys |
| import os | ||
| import subprocess | ||
| import sys | ||
| import sysconfig |
| import sysconfig | ||
| from unittest.mock import MagicMock, patch | ||
|
|
||
| import pytest |
213c158 to
a35f008
Compare
|
recheck |
3cce7a1 to
15f8c70
Compare
…skip-gate integration test Three cleanup fixes for the PR BerriAI#30051 test suite: - Remove subprocess, sys, sysconfig, and pytest imports from test_prisma_env_path_injection.py; none were called directly (all appeared only as string arguments to patch()). - Drop the dead _patch_db_deps helper from test_db_commands.py; it was defined but never called, so it added noise without coverage. - Re-scope all os.path.exists and subprocess.run patches in test_db_commands.py from the global namespace to litellm.proxy.client.cli.commands.db.*, which is where the code under test actually resolves those names. - Add a @pytest.mark.skipif gate to test_db_generate_absolute_path_non_activated_venv so it only runs when LITELLM_RUN_INTEGRATION_TESTS=1 is set, matching the opt-in pattern used by sibling integration tests in the project.
15f8c70 to
0e0bb23
Compare
…skip-gate integration test Three cleanup fixes for the PR BerriAI#30051 test suite: - Remove subprocess, sys, sysconfig, and pytest imports from test_prisma_env_path_injection.py; none were called directly (all appeared only as string arguments to patch()). - Drop the dead _patch_db_deps helper from test_db_commands.py; it was defined but never called, so it added noise without coverage. - Re-scope all os.path.exists and subprocess.run patches in test_db_commands.py from the global namespace to litellm.proxy.client.cli.commands.db.*, which is where the code under test actually resolves those names. - Add a @pytest.mark.skipif gate to test_db_generate_absolute_path_non_activated_venv so it only runs when LITELLM_RUN_INTEGRATION_TESTS=1 is set, matching the opt-in pattern used by sibling integration tests in the project.
0e0bb23 to
2d273e9
Compare
…skip-gate integration test Three cleanup fixes for the PR BerriAI#30051 test suite: - Remove subprocess, sys, sysconfig, and pytest imports from test_prisma_env_path_injection.py; none were called directly (all appeared only as string arguments to patch()). - Drop the dead _patch_db_deps helper from test_db_commands.py; it was defined but never called, so it added noise without coverage. - Re-scope all os.path.exists and subprocess.run patches in test_db_commands.py from the global namespace to litellm.proxy.client.cli.commands.db.*, which is where the code under test actually resolves those names. - Add a @pytest.mark.skipif gate to test_db_generate_absolute_path_non_activated_venv so it only runs when LITELLM_RUN_INTEGRATION_TESTS=1 is set, matching the opt-in pattern used by sibling integration tests in the project.
4d09ba4 to
4f20d05
Compare
…skip-gate integration test Three cleanup fixes for the PR BerriAI#30051 test suite: - Remove subprocess, sys, sysconfig, and pytest imports from test_prisma_env_path_injection.py; none were called directly (all appeared only as string arguments to patch()). - Drop the dead _patch_db_deps helper from test_db_commands.py; it was defined but never called, so it added noise without coverage. - Re-scope all os.path.exists and subprocess.run patches in test_db_commands.py from the global namespace to litellm.proxy.client.cli.commands.db.*, which is where the code under test actually resolves those names. - Add a @pytest.mark.skipif gate to test_db_generate_absolute_path_non_activated_venv so it only runs when LITELLM_RUN_INTEGRATION_TESTS=1 is set, matching the opt-in pattern used by sibling integration tests in the project.
| @@ -0,0 +1,110 @@ | |||
| """Database management commands for the LiteLLM proxy CLI.""" | |||
|
|
|||
| # stdlib imports | |||
There was a problem hiding this comment.
(suggestion) These comments aren't usually necessary. Convention and isort enforce these groups.
There was a problem hiding this comment.
Good call, removed. isort already keeps the groups ordered so the comments were just noise.
|
|
||
| Runs: prisma generate --schema <path_to_schema.prisma> | ||
|
|
||
| I resolve the schema path from the installed litellm-proxy-extras package, |
There was a problem hiding this comment.
Are you sure this the right schema.prisma file to use? litellm-proxy-extras says it's only about SQL migrations. There's also one in litellm/proxy/schema.prisma, which might be better for scripts within the litellm CLI, and doesn't rely on litellm-proxy-extras being correctly installed.
There was a problem hiding this comment.
I chose the extras copy deliberately. The two schemas are kept identical by CI (sync-schema.yml generates both from the root schema.prisma, check-schema-sync.yml fails on drift), so there is no behavioral difference today. The reason I picked extras: the runtime path already resolves through it. setup_database() and migrate deploy both build their schema path from ProxyExtrasDBManager._get_prisma_dir() (utils.py:519, :701), so having db generate use the same resolver keeps every DB command sourced from one schema. Pointing it at litellm/proxy/schema.prisma would make generate the only exception.
On your robustness point: if extras is not installed the command can't work, but the _PROXY_EXTRAS_AVAILABLE guard handles that with a clear pip install 'litellm[proxy]' error instead of failing deep in Prisma. The whole proxy DB lifecycle already depends on extras, so requiring it here too keeps things consistent.
There was a problem hiding this comment.
It's not up to me, but I would expect code in the litellm package to use the schema.prisma file that is in the same package, rather than a copy from another package. Alternatively, maybe this code all belongs in litellm-proxy-extras.
It's way out of scope for this, but I don't understand why it's necessary to put the same file into two packages. Will you ever use one package without the other? That CI job only checks that the files match in the repo. It doesn't check that the pinned dependency in litellm points to the correct version of litellm-proxy-extras that contains same version of the schema file. Seems like a recipe for problems.
Also out-of-scope, but if the litellm proxy server code is using the file from litellm-proxy-extras, then that contradicts the migration runbook. But it doesn't look like it to me. litellm code only imports litellm_proxy_extras when doing creating the db with migration enabled.
There was a problem hiding this comment.
Good catch. I changed db generate to resolve the schema via PrismaManager (litellm/proxy/schema.prisma), same as db push, instead of reaching into proxy-extras, and fixed the docstring.
I will defer the large architectural refactor to another day. 😅
|
Thanks for reviewing, @jamesmyatt. 🙏 I addressed all of your feedback above. |
|
Thanks for the contribution! A couple of things to address before this is ready for merge:
We're also triggering a Greptile code review in the meantime. |
|
@Sameerlite Huggingface rate limit caused the test failure. I triggered a rerun. |
|
@Sameerlite The rerun succeeded, all tests are passing now. |
|
Thanks for your contribution! A few things to get this ready:
We're also triggering a Greptile code review: |
…r pip install Injects the venv scripts directory into PATH internally so `litellm db generate` and `db push` resolve the prisma binary without manual PATH setup after a plain pip install. Adds a `db generate` CLI command, PATH-injection in proxy-extras utils, unit tests for the CLI and path verifier, and a gated integration test (`integration` marker + LITELLM_RUN_INTEGRATION_TESTS) so the real 244s prisma generate does not run in the standard unit shard.
90e2159 to
c4a0792
Compare
|
Thanks @Sameerlite. Done. I rebased onto litellm_internal_staging and switched the PR base to match. The diff is now the 9 db-generate files with no unrelated changes, and it's a single commit. Let me know if you'd like anything else before merge. |
…y-extras Addresses review feedback (jamesmyatt): db generate now resolves schema.prisma via PrismaManager (litellm/proxy/schema.prisma) — the same schema db push uses — instead of reaching into the sibling litellm-proxy-extras package. Keeps the generate path consistent with the existing push path and corrects the docstring, which wrongly claimed db push uses the proxy-extras schema. proxy-extras is still imported for prisma-binary discovery and PATH injection (not the schema).
After rebasing onto litellm_internal_staging, db.py is a new file with a zero-Any budget. Type the proxy-extras helper bindings (Optional[Callable]) and _get_generate_env's return as Dict[str, str], guard the optional prisma command at the call site, and mark the unavoidable untyped third-party import boundary with any-ok. No behavior change.
Apply black (26.3.1, repo-pinned) to the new db generate test files so they pass the lint job's `black --check .` after the rebase onto litellm_internal_staging. Formatting only, no behavior change.
The ruff strict-rule budget (UP006) flags typing.Dict/List; switch to builtin dict[str, str] and list[str] to stay under the codebase ceiling. No behavior change.
|
Both red checks look like infra, not this PR:
The change itself is green locally and the test jobs passed on the latest run. Could you re-run the cancelled job and check the lint arg? Thanks. |
Relevant issues
Fixes #26097
Linear ticket
N/A (external contributor)
Pre-Submission checklist
make test-unit@greptileaiand received a Confidence Score of at least 4/5 before requesting a maintainer reviewScreenshots / Proof of Fix
Type
Bug Fix
Problem framing — three sequential failure modes
After a clean
pip install 'litellm[proxy]'into a virtualenv, three distinct failures occur when the venv is not activated (i.e., the proxy is invoked by absolute path, as in Docker, Proxmox scripts, and other non-shell installers):Symptom 1: Schema not found (standalone
prisma generate)Symptom 2: Generator binary not found (after Symptom 1 is fixed)
Symptom 3 (most severe): Tables never created — silent failure
The install step (
litellm --use_prisma_db_push --skip_server_startup) callsPrismaManager.setup_database()->_get_prisma_env()->os.environ.copy()with noPATH fixup. The push invokes the prisma-client-py generator via
/bin/shwhich can'tfind the binary, yet
prisma db pushcan exit 0 without creating tables. The failureonly surfaces at runtime.
Root cause
All three failure modes share the same root cause: the venv
bin/directory is not onPATH when Prisma subprocesses run. The Prisma engine shells out to
prisma-client-py(aconsole script in the venv) via
/bin/sh. That only works when the venv is activated orits
bin/is explicitly on PATH.prisma generatesearches CWD /prisma/forschema.prisma. Theschema ships inside
litellm_proxy_extras/, not CWD. Fix: always pass--schemafromProxyExtrasDBManager._get_prisma_dir()._get_prisma_env()copiesos.environwith only offline-mode npmvars. It never injects the venv scripts dir. Fix: inject
sysconfig.get_path("scripts")into PATH inside
_get_prisma_env()so every Prisma subprocess — generate, db push,migrate deploy — benefits from the same fix.
Changes
Added
litellm/proxy/client/cli/commands/db.py—dbgroup anddb generatesubcommand(Symptoms 1+2);
_get_generate_env()now delegates to_get_prisma_env()whichhandles PATH injection globally.
litellm_proxy_extras/utils.py—_get_prisma_env()now injects the venv scripts dirat PATH index 0, derived from
sysconfig.get_path("scripts")with fallback todirname(sys.executable). This one-line fix coversdb push,migrate deploy, andgeneratesimultaneously (Symptom 3).litellm-proxy-extras/tests/test_prisma_env_path_injection.py— unit + integrationtests for
_get_prisma_env()PATH injection (Symptom 3 coverage).tests/test_litellm/proxy/client/cli/test_db_commands.py— 7 tests fordb generatehappy path, schema delegation, missing schema, failure propagation, unavailable extras,
CLI registration.
tests/test_litellm/proxy/client/cli/test_db_generate_path_verifier.py— adversarialtests for the PATH fix (delegation chain + None-guard).
tests/test_litellm/proxy/client/cli/test_db_generate_integration.py— integrationtests for Symptoms 1+2 invoked by absolute path.
Modified
litellm-proxy-extras/litellm_proxy_extras/utils.py—_get_prisma_env()PATHinjection;
setup_database()v1db pushelse-branch now passesenv=_get_prisma_env().litellm/proxy/client/cli/main.py— registersdbgroup.pyproject.toml— registerslitellm-proxyconsole script.Chosen approach
Symptoms 1+2 (
db generate): Newlitellm-proxy db generatesubcommand resolvesschema via
_get_prisma_dir()._get_generate_env()delegates to the now-fixed shared_get_prisma_env().Symptom 3 (
db push/migrate deploy):_get_prisma_env()is patched to prependthe venv scripts dir to PATH. This is the single-point-of-fix approach — all three code
paths call
_get_prisma_env(), so fixing it there fixes all three without duplicatingthe PATH logic.
Alternatives considered
_get_generate_env()only: Fixes Symptoms 1+2 but leaves Symptom 3(tables never created on non-activated venv installs), because
db pushandmigrate deploynever call_get_generate_env(). Folded into the consolidated_get_prisma_env()fix so all three subprocess paths share one PATH injection.LITELLM_SCHEMA_PATHenv var: Fixes Symptom 1 only (schema discovery); doesnothing for the missing-binary failures. Not pursued.
generateon proxy startup: Would fix all symptoms but adds scope creep anda silent slow/possibly-crashing step at init time. Not pursued in favour of the
smaller, explicit fix.
Downstream-packager note
This fix makes
litellm-proxy db generateandprisma db pushwork out-of-the-box aftera standard
pip install 'litellm[proxy]'. Callers invoking the proxy by absolute path nolonger need to activate the venv, know internal site-packages paths, or prefix PATH
manually.
This is the failure that took down the LiteLLM Proxmox VE Helper script (
ct/litellm.shin community-scripts/ProxmoxVE). A default install fails at
prisma generate(community-scripts/ProxmoxVE#13854, "LiteLLM fails on default install"), and the script
was ultimately removed over recurring breakage of this kind
(community-scripts/ProxmoxVE#14294). A packager that installs into an unactivated venv hits
exactly Symptoms 1-3 above; this PR removes the need for any PATH workaround in such
installers.
Interim workaround (for reference)
Until this lands, invocations by absolute path require:
env PATH="/opt/litellm/.venv/bin:$PATH" \ /opt/litellm/.venv/bin/litellm \ --config /opt/litellm/litellm.yaml --use_prisma_db_push --skip_server_startup