mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-13 23:11:40 +00:00
refactor(router): evict selectors only at model-list lifecycle sites, surface the rejected arg in warnings, always write strategy fields from the edit form
This commit is contained in:
parent
c81baa0587
commit
6a6da79320
4 changed files with 72 additions and 27 deletions
|
|
@ -1194,11 +1194,11 @@ class Router:
|
|||
)
|
||||
return valid[0] if valid else None
|
||||
|
||||
def _build_model_group_selector(self, strategy: str, args: Mapping[str, object]) -> object | None:
|
||||
def _build_model_group_selector(self, strategy: str, args: Mapping[str, object]) -> object | TypeError | ValueError:
|
||||
try:
|
||||
return self._build_strategy_selector(strategy=strategy, routing_strategy_args=args)
|
||||
except (TypeError, ValueError):
|
||||
return None
|
||||
except (TypeError, ValueError) as exc:
|
||||
return exc
|
||||
|
||||
def _live_model_group_selector_keys(self) -> frozenset[str]:
|
||||
"""
|
||||
|
|
@ -1246,15 +1246,14 @@ class Router:
|
|||
if cached is not None:
|
||||
verbose_router_logger.debug("routing_group=model-info model=%s strategy=%s", model, strategy)
|
||||
return strategy, cached
|
||||
self._evict_stale_model_group_selectors()
|
||||
built: Final = self._build_model_group_selector(strategy, args)
|
||||
if built is None:
|
||||
if built is None or isinstance(built, Exception):
|
||||
_warn_model_group_strategy_once(
|
||||
model,
|
||||
"args",
|
||||
selector_key,
|
||||
f"model_info.routing_strategy_args for model_group '{model}' cannot initialize strategy "
|
||||
f"'{strategy}'; falling back to the routing-group / top-level strategy.",
|
||||
f"'{strategy}' ({built}); falling back to the routing-group / top-level strategy.",
|
||||
)
|
||||
return None
|
||||
with self._override_selectors_lock:
|
||||
|
|
|
|||
|
|
@ -771,6 +771,41 @@ class TestDeleteModelClearsRouterRegistry:
|
|||
assert mock_router.complexity_routers.get("shared-name") is config_router
|
||||
|
||||
|
||||
class TestUpdateDbModelMergesModelInfo:
|
||||
def test_patch_omitting_routing_strategy_preserves_stored_value(self):
|
||||
"""
|
||||
The UI's edit form and any partial PATCH rely on update_db_model merging
|
||||
model_info non-destructively: keys absent from the patch keep their
|
||||
stored value instead of being wiped.
|
||||
"""
|
||||
import json as json_lib
|
||||
|
||||
from litellm.proxy.management_endpoints.model_management_endpoints import (
|
||||
update_db_model,
|
||||
)
|
||||
from litellm.types.router import Deployment, ModelInfo, updateDeployment
|
||||
|
||||
db_model = Deployment(
|
||||
model_name="quality",
|
||||
litellm_params={"model": "openai/gpt-4o-mini"},
|
||||
model_info={
|
||||
"id": "mid-1",
|
||||
"routing_strategy": "cost-based-routing",
|
||||
"routing_strategy_args": {"ttl": 60},
|
||||
},
|
||||
)
|
||||
|
||||
result = update_db_model(
|
||||
db_model=db_model,
|
||||
updated_patch=updateDeployment(model_info=ModelInfo(id="mid-1", mode="chat")),
|
||||
)
|
||||
|
||||
stored = json_lib.loads(result["model_info"])
|
||||
assert stored["routing_strategy"] == "cost-based-routing"
|
||||
assert stored["routing_strategy_args"] == {"ttl": 60}
|
||||
assert stored["mode"] == "chat"
|
||||
|
||||
|
||||
class TestUpdateModel:
|
||||
"""
|
||||
Tests for the update_model (POST /model/update) handler.
|
||||
|
|
|
|||
|
|
@ -19,6 +19,7 @@ from litellm import Router
|
|||
from litellm.router import _warn_model_group_strategy_once
|
||||
from litellm.router_strategy.lowest_cost import LowestCostLoggingHandler
|
||||
from litellm.router_strategy.lowest_latency import LowestLatencyLoggingHandler
|
||||
from litellm.types.router import Deployment
|
||||
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
|
|
@ -184,7 +185,17 @@ def test_conflict_winner_is_stable_across_model_list_order():
|
|||
assert reversed_router._get_routing_context("quality")[0] == "cost-based-routing"
|
||||
|
||||
|
||||
def test_invalid_args_evict_previously_cached_selector():
|
||||
def _upsert(router, model_name, model, deployment_id, model_info):
|
||||
router.upsert_deployment(
|
||||
Deployment(
|
||||
model_name=model_name,
|
||||
litellm_params={"model": model, "api_key": "sk-test", "api_base": "https://example.invalid"},
|
||||
model_info={"id": deployment_id, **model_info},
|
||||
)
|
||||
)
|
||||
|
||||
|
||||
def test_invalid_args_evict_previously_cached_selector(caplog):
|
||||
router = _build_router(
|
||||
[
|
||||
_deployment(
|
||||
|
|
@ -198,13 +209,20 @@ def test_invalid_args_evict_previously_cached_selector():
|
|||
_, old_selector = router._get_routing_context("quality")
|
||||
assert any("|" in k for k in router._override_selectors)
|
||||
|
||||
for idx in router.model_name_to_deployment_indices["quality"]:
|
||||
router.model_list[idx]["model_info"]["routing_strategy_args"] = {"ttl": "bogus"}
|
||||
_upsert(
|
||||
router,
|
||||
"quality",
|
||||
"openai/gpt-4o",
|
||||
"d1",
|
||||
{"routing_strategy": "latency-based-routing", "routing_strategy_args": {"ttl": "bogus"}},
|
||||
)
|
||||
|
||||
strategy, _ = router._get_routing_context("quality")
|
||||
with caplog.at_level(logging.WARNING, logger="LiteLLM Router"):
|
||||
strategy, _ = router._get_routing_context("quality")
|
||||
assert strategy == "simple-shuffle"
|
||||
assert not any("|" in k for k in router._override_selectors)
|
||||
assert all(c is not old_selector for c in litellm.callbacks)
|
||||
assert any("ttl" in r.getMessage() for r in caplog.records if "cannot initialize strategy" in r.getMessage())
|
||||
|
||||
|
||||
def test_deleting_deployment_evicts_its_selector():
|
||||
|
|
@ -309,8 +327,13 @@ def test_stale_selector_evicted_when_args_change():
|
|||
old_keys = {k for k in router._override_selectors if "|" in k}
|
||||
assert len(old_keys) == 2
|
||||
|
||||
for idx in router.model_name_to_deployment_indices["quality"]:
|
||||
router.model_list[idx]["model_info"]["routing_strategy_args"] = {"ttl": 240}
|
||||
_upsert(
|
||||
router,
|
||||
"quality",
|
||||
"openai/gpt-4o",
|
||||
"d1",
|
||||
{"routing_strategy": "latency-based-routing", "routing_strategy_args": {"ttl": 240}},
|
||||
)
|
||||
|
||||
_, new_selector = router._get_routing_context("quality")
|
||||
assert new_selector is not old_selector
|
||||
|
|
|
|||
|
|
@ -433,25 +433,13 @@ export default function ModelInfoView({
|
|||
health_check_model: values.health_check_model,
|
||||
};
|
||||
}
|
||||
const formRoutingStrategy = values.routing_strategy ?? "";
|
||||
if (formRoutingStrategy !== (localModelData.model_info?.routing_strategy ?? "")) {
|
||||
if (values.routing_strategy !== undefined || values.routing_strategy_args !== undefined) {
|
||||
updatedModelInfo = {
|
||||
...updatedModelInfo,
|
||||
routing_strategy: formRoutingStrategy,
|
||||
routing_strategy: values.routing_strategy ?? "",
|
||||
routing_strategy_args: values.routing_strategy_args ? JSON.parse(values.routing_strategy_args) : {},
|
||||
};
|
||||
}
|
||||
if (values.routing_strategy_args !== undefined) {
|
||||
const parsedArgs = values.routing_strategy_args ? JSON.parse(values.routing_strategy_args) : {};
|
||||
if (
|
||||
hasRoutingStrategyArgs(parsedArgs) ||
|
||||
hasRoutingStrategyArgs(localModelData.model_info?.routing_strategy_args)
|
||||
) {
|
||||
updatedModelInfo = {
|
||||
...updatedModelInfo,
|
||||
routing_strategy_args: parsedArgs,
|
||||
};
|
||||
}
|
||||
}
|
||||
} catch (e) {
|
||||
NotificationsManager.fromBackend("Invalid JSON in Model Info");
|
||||
return;
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue