fix(proxy): return persisted DB value for general_settings in /config/list - #32171
Conversation
…/list Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
|
|
Greptile SummaryThis PR fixes a multi-worker staleness bug in
Confidence Score: 3/5The scalar-field fix is correct and well-tested, but the same stale-global read for The litellm/proxy/proxy_server.py — specifically the
|
| Filename | Overview |
|---|---|
| litellm/proxy/proxy_server.py | DB-preference logic correctly applied to scalar fields in get_config_list, but the parallel PydanticModel branch still reads field_value from the stale in-memory general_settings global rather than from db_general_settings_dict. |
| tests/test_litellm/proxy/test_proxy_server.py | New regression test correctly mocks the DB and the in-memory global with conflicting values and asserts the DB value wins; uses only mock objects, no real network calls. |
Comments Outside Diff (1)
-
litellm/proxy/proxy_server.py, line 14609-14621 (link)Same staleness bug remains in the
PydanticModelbranchThe fix was only applied to the
elsebranch (scalar types like Boolean/Integer/String). ThePydanticModelbranch at line 14613 still readsfield_valuefrom the in-memorygeneral_settingsglobal instead of preferringdb_general_settings_dict. Currentlypass_through_endpointsis the onlyPydanticModelfield inallowed_args, so if it is saved via/config/update, a stale worker will return the old in-memory value forpass_through_endpointsfrom/config/list— the same multi-worker race that this PR fixes for booleans.
Reviews (1): Last reviewed commit: "fix(proxy): return persisted DB value fo..." | Re-trigger Greptile
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Relevant issues
The "Store Prompts in Spend Logs" toggle (Admin Settings -> Logging Settings) turns on, saves with a green success popup, then flips back to off. Reported to happen after clicking around the UI and running a few LLM requests
Linear ticket
Resolves LIT-4204
Pre-Submission checklist
Root cause
This is a backend bug, separate from the UI-only load-display issue in #32145
GET /config/listreads the fresh row from the DB intodb_general_settings_dict, but when resolving each field's value it preferred the per-process in-memorygeneral_settingsglobal and only fell back to the DB when the in-memory value wasNoneFor a boolean, a stale
Falseis notNone, so the DB fallback never fires. The in-memorygeneral_settingsglobal is only refreshed by the backgroundadd_deploymentjob (30s interval, see theadd_deployment_jobscheduler), and/config/updatewrites the DB without updating the globals of other workers. So in a multi-worker / multi-pod deployment, right after savingtruethe DB holdstruewhile a sibling worker's global still holds a stalefalse. When the UI's post-save refetch lands on that worker,/config/listreturnsfalseand the toggle snaps back off. "Click around and run a few requests" is just what gives the background job time to seed that stalefalseinto the other workersThe fix makes the persisted DB value authoritative for
/config/list, falling back to the in-memory global only for settings that live purely in config.yaml / env and were never written to the DBScreenshots / Proof of Fix
Reproduced live on a local proxy running 2 workers against Postgres, hitting the real
/config/updateand/config/listendpoints (no mocks)Before the fix, save OFF, wait for the background job to seed both workers, save ON, then poll
/config/list. The DB istruebut the stale worker still answersfalseAfter the fix, same sequence, DB
true, 60/60 responses agreeRegression test
test_get_config_list_prefers_db_over_stale_in_memory_general_settingspins a fresh DB row ofTrueagainst a stale in-memorygeneral_settingsofFalseand asserts/config/listreturnsTrue. It fails on the old code and passes with the fixType
🐛 Bug Fix
Changes
litellm/proxy/proxy_server.py: inget_config_list, prefer the DB-persisted value over the in-memorygeneral_settingsglobal when a field is present in the DB rowtests/test_litellm/proxy/test_proxy_server.py: add the regression test described aboveLink to Devin session: https://app.devin.ai/sessions/2edea14c59e54828bbe342d5983ef34e
Requested by: @krrish-berri-2