mirror of
https://github.com/BerriAI/litellm.git
synced 2026-11-11 08:38:24 +00:00
fix(proxy): leave a string general_settings row untouched on delete, warn once about it, and answer a stored-copy removal with a message
This commit is contained in:
parent
2dc56dd784
commit
fb915ff85e
3 changed files with 50 additions and 19 deletions
|
|
@ -5722,6 +5722,7 @@ class ProxyConfig:
|
|||
self._sso_environment_removals: frozenset[str] = frozenset()
|
||||
self._warned_shadowed_keys: frozenset[tuple[Section, str]] = frozenset()
|
||||
self._warned_unmanaged_keys: frozenset[str] = frozenset()
|
||||
self._warned_unreadable_db_general_settings: bool = False
|
||||
self._db_config_callback_entries: Mapping[tuple[str, str], _DbCallbackRegistration] = MappingProxyType({})
|
||||
self._settings_stores: Final[Mapping[Section, SettingsStore]] = MappingProxyType(
|
||||
{
|
||||
|
|
@ -8217,7 +8218,10 @@ class ProxyConfig:
|
|||
|
||||
async def _update_general_settings(self, db_general_settings: Mapping[str, SettingsJsonValue] | str | None) -> None:
|
||||
global general_settings
|
||||
if db_general_settings is None:
|
||||
return
|
||||
if not isinstance(db_general_settings, Mapping):
|
||||
self._warn_about_unreadable_db_general_settings()
|
||||
return
|
||||
if not isinstance(general_settings, SettingsStore):
|
||||
self.settings.load_yaml(_as_settings_mapping(general_settings))
|
||||
|
|
@ -8514,6 +8518,15 @@ class ProxyConfig:
|
|||
)
|
||||
self._warned_unmanaged_keys = unmanaged
|
||||
|
||||
def _warn_about_unreadable_db_general_settings(self) -> None:
|
||||
if self._warned_unreadable_db_general_settings:
|
||||
return
|
||||
verbose_proxy_logger.warning(
|
||||
"general_settings is stored in the database as something other than a JSON object, so the proxy "
|
||||
"ignores every stored general setting."
|
||||
)
|
||||
self._warned_unreadable_db_general_settings = True
|
||||
|
||||
def _config_sets_ignored_db_key(self, key: str) -> bool:
|
||||
return self.settings.owned_by_config(key) or (
|
||||
key in _GENERAL_SETTINGS_UI_LITELLM_FIELDS and self.config_owns(section_name="litellm_settings", key=key)
|
||||
|
|
@ -20651,9 +20664,9 @@ async def delete_config_general_settings(
|
|||
stored_copy: Final = await _stored_general_settings()
|
||||
if stored_copy is None or data.field_name not in stored_copy:
|
||||
return await _reset_general_settings_ui_litellm_field(data.field_name, user_api_key_dict)
|
||||
removed: Final = await _remove_stored_general_setting(data.field_name, stored_copy, user_api_key_dict)
|
||||
await _remove_stored_general_setting(data.field_name, stored_copy, user_api_key_dict)
|
||||
if proxy_config.config_owns(section_name="litellm_settings", key=data.field_name):
|
||||
return removed
|
||||
return {"message": f"Field {data.field_name} removed", "status": "success"}
|
||||
return await _reset_general_settings_ui_litellm_field(data.field_name, user_api_key_dict)
|
||||
|
||||
stored: Final = await _stored_general_settings()
|
||||
|
|
@ -20679,14 +20692,9 @@ async def delete_config_general_settings(
|
|||
async def _stored_general_settings() -> Mapping[str, SettingsJsonValue] | None:
|
||||
config_table: Final[TableActions[_ConfigParamValueRow]] = _config_param_table(prisma_client)
|
||||
row: Final = await config_table.find_first(where={"param_name": "general_settings"})
|
||||
if row is None or row.param_value is None:
|
||||
if row is None or not isinstance(row.param_value, Mapping):
|
||||
return None
|
||||
value: Final = (
|
||||
cast(object, json.loads(row.param_value)) # cast-ok: json.loads is untyped; the row is opaque JSON
|
||||
if isinstance(row.param_value, str)
|
||||
else row.param_value
|
||||
)
|
||||
return _SETTINGS_MAPPING.validate_python(value) if isinstance(value, Mapping) else None
|
||||
return _SETTINGS_MAPPING.validate_python(row.param_value)
|
||||
|
||||
|
||||
async def _remove_stored_general_setting(
|
||||
|
|
|
|||
|
|
@ -4422,6 +4422,26 @@ async def test_ProxyConfig__update_general_settings_says_the_config_file_applies
|
|||
]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_ProxyConfig__update_general_settings_warns_once_about_a_stored_row_that_is_not_an_object(
|
||||
monkeypatch: pytest.MonkeyPatch, caplog: pytest.LogCaptureFixture
|
||||
) -> None:
|
||||
pc: Final = ProxyConfig()
|
||||
pc._load_yaml_settings_stores({"general_settings": {}})
|
||||
monkeypatch.setattr("litellm.proxy.proxy_server.general_settings", pc.settings)
|
||||
|
||||
with caplog.at_level(logging.WARNING, logger="LiteLLM Proxy"):
|
||||
await pc._update_general_settings(None)
|
||||
await pc._update_general_settings('{"enforce_user_param": true}')
|
||||
await pc._update_general_settings('{"enforce_user_param": true}')
|
||||
|
||||
warnings: Final = [record.getMessage() for record in caplog.records if "JSON object" in record.getMessage()]
|
||||
assert warnings == [
|
||||
"general_settings is stored in the database as something other than a JSON object, so the proxy ignores "
|
||||
"every stored general setting."
|
||||
]
|
||||
|
||||
|
||||
def _paused_scheduler(monkeypatch):
|
||||
from apscheduler.schedulers.asyncio import AsyncIOScheduler
|
||||
|
||||
|
|
|
|||
|
|
@ -15546,13 +15546,14 @@ async def test_delete_config_general_settings_removes_a_stored_copy_of_a_ui_lite
|
|||
monkeypatch.setattr(litellm, "budget_rollover", True)
|
||||
|
||||
admin: Final = UserAPIKeyAuth(api_key="hashed-admin", user_id="admin-1", user_role=LitellmUserRoles.PROXY_ADMIN)
|
||||
await delete_config_general_settings(
|
||||
response: Final = await delete_config_general_settings(
|
||||
data=ConfigFieldDelete(field_name="budget_rollover", config_type="general_settings"),
|
||||
user_api_key_dict=admin,
|
||||
)
|
||||
|
||||
written: Final = json.loads(fake_prisma.db.litellm_config.upsert.await_args.kwargs["data"]["update"]["param_value"])
|
||||
assert written == {"store_prompts_in_spend_logs": True}
|
||||
assert response == {"message": "Field budget_rollover removed", "status": "success"}
|
||||
assert litellm.budget_rollover is True
|
||||
assert pc.settings.unmanaged_db_keys() == ()
|
||||
|
||||
|
|
@ -15582,26 +15583,28 @@ async def test_delete_config_general_settings_refuses_an_unknown_key_when_the_st
|
|||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_delete_config_general_settings_removes_a_key_from_a_json_string_row(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
@pytest.mark.parametrize("stored_row", ['{"max_request_size_mb": 42, "store_prompts_in_spend_logs": true}', "not json"])
|
||||
async def test_delete_config_general_settings_leaves_a_string_row_untouched(
|
||||
monkeypatch: pytest.MonkeyPatch, stored_row: str
|
||||
) -> None:
|
||||
from litellm.proxy._types import ConfigFieldDelete
|
||||
from litellm.proxy.proxy_server import ProxyConfig, delete_config_general_settings
|
||||
|
||||
pc: Final = ProxyConfig()
|
||||
pc._load_yaml_settings_stores({"general_settings": {}})
|
||||
fake_prisma: Final = _fake_prisma_with_config('{"max_request_size_mb": 42, "store_prompts_in_spend_logs": true}')
|
||||
fake_prisma: Final = _fake_prisma_with_config(stored_row)
|
||||
monkeypatch.setattr(proxy_server_module, "proxy_config", pc)
|
||||
monkeypatch.setattr(proxy_server_module, "prisma_client", fake_prisma)
|
||||
|
||||
admin: Final = UserAPIKeyAuth(api_key="hashed-admin", user_id="admin-1", user_role=LitellmUserRoles.PROXY_ADMIN)
|
||||
await delete_config_general_settings(
|
||||
data=ConfigFieldDelete(field_name="max_request_size_mb", config_type="general_settings"),
|
||||
user_api_key_dict=admin,
|
||||
)
|
||||
with pytest.raises(HTTPException) as refused:
|
||||
await delete_config_general_settings(
|
||||
data=ConfigFieldDelete(field_name="max_request_size_mb", config_type="general_settings"),
|
||||
user_api_key_dict=admin,
|
||||
)
|
||||
|
||||
written: Final = json.loads(fake_prisma.db.litellm_config.upsert.await_args.kwargs["data"]["update"]["param_value"])
|
||||
assert written == {"store_prompts_in_spend_logs": True}
|
||||
assert refused.value.status_code == 400
|
||||
fake_prisma.db.litellm_config.upsert.assert_not_awaited()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue