mirror of
https://github.com/BerriAI/litellm.git
synced 2026-10-12 03:48:56 +00:00
fix(proxy): stop null key/team router_settings from shadowing router defaults
The admin UI persists the whole team/key router-settings form on save,
including blank fields as explicit nulls. route_request forwarded those
nulls as per-request kwargs, and Router treats a present-but-None kwarg
as an override (kwargs.get('fallbacks', self.fallbacks)), so a saved-but-
empty team form silently disables the globally configured fallback chain
(and num_retries/timeout) for every request with that key/team - errors
surface as e.g. 401 with 'Available Model Group Fallbacks=None' even
though fallbacks are configured.
Strip null entries where stored key/team router_settings are parsed, and
guard the per-request merge so a null means 'not configured' instead of
'override with nothing'.
This commit is contained in:
parent
a8979fe054
commit
856efae1e5
4 changed files with 80 additions and 5 deletions
|
|
@ -6775,6 +6775,12 @@ class ProxyConfig:
|
|||
"""
|
||||
Parse a router_settings value that may be a dict or a JSON/YAML string.
|
||||
|
||||
Null entries are stripped: the admin UI persists the whole router-settings
|
||||
form on save, including fields left blank as explicit nulls. A null means
|
||||
"no opinion - keep the router default", never "override the default with
|
||||
nothing" - forwarding it would shadow global settings such as `fallbacks`
|
||||
or `num_retries` for every request made with that key/team.
|
||||
|
||||
Returns a non-empty dict if valid, otherwise None.
|
||||
"""
|
||||
if value is None:
|
||||
|
|
@ -6794,9 +6800,11 @@ class ProxyConfig:
|
|||
except json.JSONDecodeError:
|
||||
pass
|
||||
|
||||
if isinstance(parsed, dict) and parsed:
|
||||
return parsed
|
||||
return None
|
||||
if not isinstance(parsed, dict) or not parsed:
|
||||
return None
|
||||
|
||||
stripped = {k: v for k, v in parsed.items() if v is not None}
|
||||
return stripped or None
|
||||
|
||||
async def get_hierarchical_router_settings(
|
||||
self,
|
||||
|
|
|
|||
|
|
@ -526,9 +526,12 @@ async def _route_request_single_attempt( # noqa: ANN202 # returns unawaited pr
|
|||
"enable_tag_filtering",
|
||||
]
|
||||
|
||||
# Merge override settings into data (only if not already set in request)
|
||||
# Merge override settings into data (only if not already set in request).
|
||||
# A null override means "not configured" and must not be forwarded: it
|
||||
# would shadow the Router's own default (e.g. `fallbacks=None` in kwargs
|
||||
# disables the globally configured fallback chain for the request).
|
||||
for key in per_request_settings:
|
||||
if key in override_settings and key not in data:
|
||||
if key in override_settings and override_settings[key] is not None and key not in data:
|
||||
data[key] = override_settings[key]
|
||||
|
||||
# Use main router with overridden kwargs
|
||||
|
|
|
|||
|
|
@ -3036,6 +3036,39 @@ def test_ProxyConfig__parse_router_settings_value_invalid_returns_none():
|
|||
assert ProxyConfig._parse_router_settings_value({}) is None
|
||||
|
||||
|
||||
def test_ProxyConfig__parse_router_settings_value_strips_null_entries():
|
||||
# The admin UI persists the whole router-settings form, blank fields included.
|
||||
# Nulls must not survive parsing: as per-request kwargs they would shadow the
|
||||
# Router's own defaults (e.g. fallbacks=None disables the global fallback chain).
|
||||
ui_form_dump = {
|
||||
"routing_strategy": "simple-shuffle",
|
||||
"enable_tag_filtering": False,
|
||||
"fallbacks": None,
|
||||
"num_retries": None,
|
||||
"timeout": None,
|
||||
"retry_after": None,
|
||||
"retry_policy": None,
|
||||
"allowed_fails": None,
|
||||
"cooldown_time": None,
|
||||
"model_group_alias": None,
|
||||
"routing_strategy_args": None,
|
||||
"context_window_fallbacks": None,
|
||||
}
|
||||
assert ProxyConfig._parse_router_settings_value(ui_form_dump) == {
|
||||
"routing_strategy": "simple-shuffle",
|
||||
"enable_tag_filtering": False,
|
||||
}
|
||||
# Falsy-but-set values are NOT nulls and must be kept.
|
||||
assert ProxyConfig._parse_router_settings_value({"fallbacks": [], "timeout": 0}) == {
|
||||
"fallbacks": [],
|
||||
"timeout": 0,
|
||||
}
|
||||
# A settings dict that is *only* nulls carries no opinion -> None.
|
||||
assert ProxyConfig._parse_router_settings_value({"fallbacks": None, "timeout": None}) is None
|
||||
# Also for the JSON-string storage shape.
|
||||
assert ProxyConfig._parse_router_settings_value('{"num_retries": null, "timeout": 30}') == {"timeout": 30}
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# ProxyConfig._get_hierarchical_router_settings
|
||||
# ---------------------------------------------------------------------------
|
||||
|
|
|
|||
|
|
@ -474,6 +474,37 @@ async def test_route_request_with_router_settings_override_preserves_existing():
|
|||
assert call_kwargs["timeout"] == 30
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_route_request_with_router_settings_override_ignores_null_values():
|
||||
"""A key/team router_settings entry of null means "not configured" and must not
|
||||
be forwarded to the Router: `fallbacks=None` in kwargs shadows Router.fallbacks
|
||||
and silently disables the globally configured fallback chain for the request."""
|
||||
data = {
|
||||
"model": "gpt-3.5-turbo",
|
||||
"messages": [{"role": "user", "content": "Hello"}],
|
||||
"router_settings_override": {
|
||||
"fallbacks": None, # UI form-save of a blank field
|
||||
"num_retries": None,
|
||||
"timeout": None,
|
||||
"routing_strategy": "simple-shuffle", # actually configured
|
||||
},
|
||||
}
|
||||
|
||||
llm_router = MagicMock()
|
||||
llm_router.acompletion.return_value = "success"
|
||||
|
||||
response = await route_request(data, llm_router, None, "acompletion")
|
||||
|
||||
assert response == "success"
|
||||
call_kwargs = llm_router.acompletion.call_args[1]
|
||||
# Null overrides must not appear at all -> Router keeps its own defaults.
|
||||
assert "fallbacks" not in call_kwargs
|
||||
assert "num_retries" not in call_kwargs
|
||||
assert "timeout" not in call_kwargs
|
||||
# Configured values still merge.
|
||||
assert call_kwargs["routing_strategy"] == "simple-shuffle"
|
||||
|
||||
|
||||
def test_gated_mock_params_cover_mock_router_testing_params():
|
||||
"""``GATED_MOCK_PARAM_NAMES`` is hardcoded to avoid a cyclic import
|
||||
against ``litellm.types.router``. This test guards against drift — if a
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue