diff --git a/litellm/proxy/proxy_server.py b/litellm/proxy/proxy_server.py index 351f19e1638..fbd73f947fe 100644 --- a/litellm/proxy/proxy_server.py +++ b/litellm/proxy/proxy_server.py @@ -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( diff --git a/tests/unit/proxy/proxy_server/test_proxy_config.py b/tests/unit/proxy/proxy_server/test_proxy_config.py index a4f81c1645b..b7959b00f03 100644 --- a/tests/unit/proxy/proxy_server/test_proxy_config.py +++ b/tests/unit/proxy/proxy_server/test_proxy_config.py @@ -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 diff --git a/tests/unit/proxy/test_proxy_server_endpoints_and_startup.py b/tests/unit/proxy/test_proxy_server_endpoints_and_startup.py index f33a38df00d..06234de8723 100644 --- a/tests/unit/proxy/test_proxy_server_endpoints_and_startup.py +++ b/tests/unit/proxy/test_proxy_server_endpoints_and_startup.py @@ -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