mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-21 00:21:49 +00:00
feat(proxy): say when a stored setting is ignored because the config file owns it
The config file winning over the database was silent. An admin who had set a value through the UI and later pinned the same key in the file saw their stored value quietly stop applying, with nothing said at boot and nothing said when a later write was refused. Startup now warns once per key whose stored value differs from the file's, naming the key and what to do about it. The refusal raised on a write to a config-owned key carries the same sentence, so the log and the 400 read identically, and both call out that a stored value exists and will never be applied. The /config/update refusal gained the same detail. Keys the file does not declare are untouched: the database still owns them, and a stored value equal to the file's is not worth a warning.
This commit is contained in:
parent
209a780992
commit
7353b779c2
6 changed files with 132 additions and 15 deletions
|
|
@ -5,6 +5,6 @@ from litellm.proxy.config_resolvers._descriptors import (
|
|||
FieldSource,
|
||||
resolve_fields,
|
||||
)
|
||||
from litellm.proxy.config_resolvers.settings_store import SettingsStore
|
||||
from litellm.proxy.config_resolvers.settings_store import SettingsStore, config_ownership_message
|
||||
|
||||
__all__ = ("FieldDescriptor", "FieldSource", "SettingsStore", "resolve_fields")
|
||||
__all__ = ("FieldDescriptor", "FieldSource", "SettingsStore", "config_ownership_message", "resolve_fields")
|
||||
|
|
|
|||
|
|
@ -19,10 +19,21 @@ from litellm.proxy.config_resolvers.settings_rules import (
|
|||
|
||||
|
||||
class ConfigOwnedKeyError(RuntimeError):
|
||||
def __init__(self, section: Section, key: str) -> None:
|
||||
super().__init__(f"{section}.{key} is set in the config file and cannot be changed at runtime")
|
||||
def __init__(self, section: Section, key: str, *, shadows_db_value: bool = False) -> None:
|
||||
super().__init__(config_ownership_message(section=section, key=key, shadows_db_value=shadows_db_value))
|
||||
self.section: Final = section
|
||||
self.key: Final = key
|
||||
self.shadows_db_value: Final = shadows_db_value
|
||||
|
||||
|
||||
def config_ownership_message(*, section: Section, key: str, shadows_db_value: bool) -> str:
|
||||
stored: Final = (
|
||||
" The value stored in the database for it is ignored and will never be applied." if shadows_db_value else ""
|
||||
)
|
||||
return (
|
||||
f"{section}.{key} is set in the config file, so the config file owns it and it cannot be changed "
|
||||
f"here.{stored} Edit the config file to change it, or remove it from the file to let the database own it."
|
||||
)
|
||||
|
||||
|
||||
_EMPTY_VALUES: Final[Mapping[str, JsonValue]] = MappingProxyType({})
|
||||
|
|
@ -54,6 +65,13 @@ class SettingsStore(MutableMapping[str, JsonValue]):
|
|||
)
|
||||
)
|
||||
|
||||
def shadowed_db_keys(self) -> tuple[str, ...]:
|
||||
"""Keys the config file owns whose stored value differs, so the stored one never reaches a reader."""
|
||||
return tuple(sorted(key for key in self._yaml_values if self._db_value_is_shadowed(key)))
|
||||
|
||||
def shadows_db_value(self, key: str) -> bool:
|
||||
return self.owned_by_config(key) and self._db_value_is_shadowed(key)
|
||||
|
||||
def apply_db_row(self, row: DbRow, db_row: Mapping[str, JsonValue]) -> None:
|
||||
previous_row: Final = self._database_rows.get(row, _EMPTY_VALUES)
|
||||
self._database_rows = MappingProxyType({**self._database_rows, row: MappingProxyType(dict(db_row))})
|
||||
|
|
@ -81,7 +99,7 @@ class SettingsStore(MutableMapping[str, JsonValue]):
|
|||
|
||||
def __setitem__(self, key: str, value: JsonValue) -> None:
|
||||
if self.owned_by_config(key) and value != self.get(key):
|
||||
raise ConfigOwnedKeyError(self._section, key)
|
||||
raise ConfigOwnedKeyError(self._section, key, shadows_db_value=self._db_value_is_shadowed(key))
|
||||
self._runtime_values = MappingProxyType({**self._runtime_values, key: value})
|
||||
self._deleted_runtime_keys = self._deleted_runtime_keys - frozenset((key,))
|
||||
|
||||
|
|
@ -89,7 +107,7 @@ class SettingsStore(MutableMapping[str, JsonValue]):
|
|||
if key not in self:
|
||||
raise KeyError(key)
|
||||
if self.owned_by_config(key):
|
||||
raise ConfigOwnedKeyError(self._section, key)
|
||||
raise ConfigOwnedKeyError(self._section, key, shadows_db_value=self._db_value_is_shadowed(key))
|
||||
self._runtime_values = MappingProxyType(
|
||||
{key_: value for key_, value in self._runtime_values.items() if key_ != key}
|
||||
)
|
||||
|
|
@ -136,8 +154,14 @@ class SettingsStore(MutableMapping[str, JsonValue]):
|
|||
)
|
||||
)
|
||||
|
||||
def _resolution_for(self, key: str) -> Resolved:
|
||||
def _db_value(self, key: str) -> SettingValue:
|
||||
rule: Final = rule_for(self._section, key)
|
||||
return self._database_rows.get(rule.db_row, _EMPTY_VALUES).get(key, ABSENT)
|
||||
|
||||
def _db_value_is_shadowed(self, key: str) -> bool:
|
||||
db_value: Final = self._db_value(key)
|
||||
return not isinstance(db_value, Absent) and db_value is not None and db_value != self.get(key)
|
||||
|
||||
def _resolution_for(self, key: str) -> Resolved:
|
||||
yaml_value: Final[SettingValue] = self._yaml_values.get(key, ABSENT)
|
||||
db_value: Final[SettingValue] = self._database_rows.get(rule.db_row, _EMPTY_VALUES).get(key, ABSENT)
|
||||
return resolve(yaml_value, db_value)
|
||||
return resolve(yaml_value, self._db_value(key))
|
||||
|
|
|
|||
|
|
@ -447,7 +447,7 @@ from litellm.proxy.common_utils.user_api_key_cache import (
|
|||
project_spend_counter_key,
|
||||
tag_cache_key,
|
||||
)
|
||||
from litellm.proxy.config_resolvers import SettingsStore, resolve_fields
|
||||
from litellm.proxy.config_resolvers import SettingsStore, config_ownership_message, resolve_fields
|
||||
from litellm.proxy.config_resolvers.alerting import (
|
||||
EMAIL_DESCRIPTORS,
|
||||
MS_TEAMS_DESCRIPTORS,
|
||||
|
|
@ -4907,6 +4907,7 @@ class ProxyConfig:
|
|||
self.router_settings: Final[SettingsStore] = SettingsStore("router_settings")
|
||||
self.litellm_settings: Final[SettingsStore] = SettingsStore("litellm_settings")
|
||||
self.environment_variables: Final[SettingsStore] = SettingsStore("environment_variables")
|
||||
self._warned_shadowed_keys: frozenset[tuple[Section, str]] = frozenset()
|
||||
self._settings_stores: Final[Mapping[Section, SettingsStore]] = MappingProxyType(
|
||||
{
|
||||
"general_settings": self.settings,
|
||||
|
|
@ -5128,12 +5129,20 @@ class ProxyConfig:
|
|||
f"key '{rejected[0]}' is" if len(rejected) == 1 else f"keys {', '.join(repr(key) for key in rejected)} are"
|
||||
)
|
||||
pronoun: Final = "it" if len(rejected) == 1 else "them"
|
||||
shadowed: Final = tuple(key for key in rejected if store.shadows_db_value(key))
|
||||
stored: Final = (
|
||||
f" The {'value' if len(shadowed) == 1 else 'values'} already stored in the database for "
|
||||
f"{', '.join(shadowed)} {'is' if len(shadowed) == 1 else 'are'} ignored and will never be applied."
|
||||
if shadowed
|
||||
else ""
|
||||
)
|
||||
raise HTTPException(
|
||||
status_code=400,
|
||||
detail={
|
||||
"error": f"{section_name} {subject} set in the config file and cannot be changed here",
|
||||
"error": f"{section_name} {subject} set in the config file and cannot be changed here.{stored}",
|
||||
"keys": list(rejected),
|
||||
"section": section_name,
|
||||
"stored_database_values_ignored": list(shadowed),
|
||||
"resolution": (
|
||||
f"edit {user_config_file_path} to change {pronoun}, "
|
||||
f"or remove {pronoun} from the file to let the database own {pronoun}"
|
||||
|
|
@ -7430,8 +7439,19 @@ class ProxyConfig:
|
|||
self._prepared_db_settings_values(section, param_value),
|
||||
)
|
||||
|
||||
self._warn_about_shadowed_db_settings()
|
||||
return self._config_with_resolved_settings(config)
|
||||
|
||||
def _warn_about_shadowed_db_settings(self) -> None:
|
||||
shadowed: Final[frozenset[tuple[Section, str]]] = frozenset(
|
||||
(section, key) for section, store in self._settings_stores.items() for key in store.shadowed_db_keys()
|
||||
)
|
||||
for section, key in sorted(shadowed - self._warned_shadowed_keys):
|
||||
verbose_proxy_logger.warning(
|
||||
"%s", config_ownership_message(section=section, key=key, shadows_db_value=True)
|
||||
)
|
||||
self._warned_shadowed_keys = shadowed
|
||||
|
||||
def _prepared_db_settings_values(self, section: Section, value: object) -> Mapping[str, SettingsJsonValue]:
|
||||
if section == "environment_variables":
|
||||
decrypted: Final = self._decrypt_and_set_db_env_variables(
|
||||
|
|
|
|||
|
|
@ -497,12 +497,10 @@ def _store_allowed_ips(general_settings: MutableMapping[str, object], allowed_ip
|
|||
raise HTTPException(
|
||||
status_code=400,
|
||||
detail={ # mutable-ok: HTTPException serializes its detail as json
|
||||
"error": f"{owned.section} key '{owned.key}' is set in the config file and cannot be changed here",
|
||||
"error": str(owned),
|
||||
"keys": (owned.key,),
|
||||
"section": owned.section,
|
||||
"resolution": (
|
||||
"edit the config file to change it, or remove it from the file to let the database own it"
|
||||
),
|
||||
"stored_database_value_ignored": owned.shadows_db_value,
|
||||
},
|
||||
) from owned
|
||||
|
||||
|
|
|
|||
|
|
@ -315,3 +315,47 @@ def test_settings_store_still_accepts_a_write_to_a_key_the_config_does_not_own()
|
|||
store["max_parallel_requests"] = 7
|
||||
|
||||
assert store["max_parallel_requests"] == 7
|
||||
|
||||
|
||||
def test_settings_store_reports_a_config_owned_key_whose_stored_value_differs() -> None:
|
||||
store: Final = SettingsStore("general_settings")
|
||||
store.load_yaml({"allowed_ips": ["1.2.3.4"]})
|
||||
store.apply_db_row("general_settings", {"allowed_ips": ["1.2.3.4", "5.6.7.8"], "max_parallel_requests": 7})
|
||||
|
||||
assert store.shadowed_db_keys() == ("allowed_ips",)
|
||||
assert store.shadows_db_value("allowed_ips") is True
|
||||
assert store.shadows_db_value("max_parallel_requests") is False
|
||||
assert store["max_parallel_requests"] == 7
|
||||
|
||||
|
||||
def test_settings_store_reports_no_shadowing_when_the_stored_value_agrees() -> None:
|
||||
store: Final = SettingsStore("general_settings")
|
||||
store.load_yaml({"allowed_ips": ["1.2.3.4"]})
|
||||
store.apply_db_row("general_settings", {"allowed_ips": ["1.2.3.4"]})
|
||||
|
||||
assert store.shadowed_db_keys() == ()
|
||||
assert store.shadows_db_value("allowed_ips") is False
|
||||
|
||||
|
||||
def test_settings_store_says_the_stored_value_is_ignored_when_it_refuses_a_write() -> None:
|
||||
store: Final = SettingsStore("general_settings")
|
||||
store.load_yaml({"allowed_ips": ["1.2.3.4"]})
|
||||
store.apply_db_row("general_settings", {"allowed_ips": ["1.2.3.4", "5.6.7.8"]})
|
||||
|
||||
with pytest.raises(ConfigOwnedKeyError) as refused:
|
||||
store["allowed_ips"] = ["9.9.9.9"]
|
||||
|
||||
assert refused.value.shadows_db_value is True
|
||||
assert "stored in the database" in str(refused.value)
|
||||
|
||||
|
||||
def test_settings_store_refusal_stays_quiet_about_the_database_when_nothing_is_stored() -> None:
|
||||
store: Final = SettingsStore("general_settings")
|
||||
store.load_yaml({"allowed_ips": ["1.2.3.4"]})
|
||||
|
||||
with pytest.raises(ConfigOwnedKeyError) as refused:
|
||||
store["allowed_ips"] = ["9.9.9.9"]
|
||||
|
||||
assert refused.value.shadows_db_value is False
|
||||
assert "stored in the database" not in str(refused.value)
|
||||
assert "config file" in str(refused.value)
|
||||
|
|
|
|||
|
|
@ -5516,6 +5516,37 @@ async def test_router_settings_reload_keeps_db_values_writable(tmp_path, monkeyp
|
|||
assert proxy_config.router_settings.rejected_writes({"disable_cooldowns": False}) == ("disable_cooldowns",)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_boot_warns_that_a_shadowed_database_value_will_never_apply(tmp_path, monkeypatch, caplog):
|
||||
from litellm.proxy.proxy_server import ProxyConfig
|
||||
|
||||
config_path: Final = tmp_path / "config.yaml"
|
||||
config_path.write_text(
|
||||
yaml.safe_dump({"model_list": [], "general_settings": {"allowed_ips": ["1.2.3.4"], "max_file_size_mb": 5}})
|
||||
)
|
||||
db_row: Final = types.SimpleNamespace(param_value={"allowed_ips": ["1.2.3.4", "5.6.7.8"], "max_parallel_requests": 7})
|
||||
|
||||
async def read_config_row(_prisma_client, param_name):
|
||||
return db_row if param_name == "general_settings" else None
|
||||
|
||||
monkeypatch.setattr(proxy_server_module, "get_config_param", read_config_row)
|
||||
monkeypatch.setattr(proxy_server_module, "prisma_client", MagicMock())
|
||||
monkeypatch.setattr(proxy_server_module, "store_model_in_db", True)
|
||||
monkeypatch.setattr(proxy_server_module, "user_config_file_path", None)
|
||||
proxy_config: Final = ProxyConfig()
|
||||
|
||||
with caplog.at_level(logging.WARNING, logger="LiteLLM Proxy"):
|
||||
await proxy_config.get_config(config_file_path=str(config_path))
|
||||
|
||||
warnings: Final = " ".join(record.getMessage() for record in caplog.records)
|
||||
assert "allowed_ips" in warnings
|
||||
assert "ignored" in warnings
|
||||
assert "max_parallel_requests" not in warnings
|
||||
assert "max_file_size_mb" not in warnings
|
||||
assert proxy_config.settings["allowed_ips"] == ["1.2.3.4"]
|
||||
assert proxy_config.settings["max_parallel_requests"] == 7
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_model_info_v1_oci_secrets_not_leaked():
|
||||
"""
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue