mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-14 23:21:35 +00:00
fix(router): scope registry cleanup to the removed deployment's own strategy
Review follow-up (cross-strategy eviction): model_names are only unique per registry, so a complexity router and a quality router may legally share a name — blanket pops across all four registries let deleting one evict the other. Route the pop by the litellm_params.model prefix. Also make the adaptive hook-count assertions baseline-relative so hooks leaked by unrelated tests can't flake them.
This commit is contained in:
parent
2f1393365f
commit
dcbff0d7b3
2 changed files with 76 additions and 8 deletions
|
|
@ -7734,11 +7734,24 @@ class Router:
|
|||
upsert/delete cycle leaks one hook (and a deleted router's hook keeps
|
||||
firing forever).
|
||||
"""
|
||||
if not (litellm_model or "").startswith("auto_router/"):
|
||||
litellm_model = litellm_model or ""
|
||||
if not litellm_model.startswith("auto_router/"):
|
||||
return
|
||||
|
||||
# scope the pop to the removed deployment's OWN strategy registry —
|
||||
# model_names are only unique per registry, so a complexity router and
|
||||
# a quality router may legally share a name; deleting one must not
|
||||
# evict the other.
|
||||
if litellm_model.startswith("auto_router/complexity_router"):
|
||||
self.complexity_routers.pop(model_name, None)
|
||||
return
|
||||
if litellm_model.startswith("auto_router/quality_router"):
|
||||
self.quality_routers.pop(model_name, None)
|
||||
return
|
||||
if not litellm_model.startswith("auto_router/adaptive_router"):
|
||||
# plain auto_router/<name> (semantic router)
|
||||
self.auto_routers.pop(model_name, None)
|
||||
return
|
||||
self.auto_routers.pop(model_name, None)
|
||||
self.complexity_routers.pop(model_name, None)
|
||||
self.quality_routers.pop(model_name, None)
|
||||
|
||||
adaptive_router = self.adaptive_routers.pop(model_name, None)
|
||||
if adaptive_router is not None:
|
||||
|
|
|
|||
|
|
@ -5518,22 +5518,77 @@ class TestStrategyRegistryCleanupGuards:
|
|||
]
|
||||
)
|
||||
|
||||
def test_deleting_one_strategy_does_not_evict_same_named_other_strategy(self):
|
||||
"""model_names are only unique per registry — a complexity router and a
|
||||
quality router may share a name; deleting one must not evict the other."""
|
||||
router = litellm.Router(
|
||||
model_list=[
|
||||
{
|
||||
"model_name": "backing-model",
|
||||
"litellm_params": {"model": "gpt-4o-mini", "api_key": "fake-key"},
|
||||
}
|
||||
]
|
||||
)
|
||||
from litellm.types.router import Deployment, LiteLLM_Params
|
||||
|
||||
tiers = {
|
||||
"SIMPLE": "backing-model",
|
||||
"MEDIUM": "backing-model",
|
||||
"COMPLEX": "backing-model",
|
||||
"REASONING": "backing-model",
|
||||
}
|
||||
router.add_deployment(
|
||||
Deployment(
|
||||
model_name="dual-name",
|
||||
litellm_params=LiteLLM_Params(
|
||||
model="auto_router/complexity_router",
|
||||
complexity_router_config={"tiers": tiers},
|
||||
complexity_router_default_model="backing-model",
|
||||
),
|
||||
model_info={"id": "dual-complexity-row"},
|
||||
)
|
||||
)
|
||||
router.add_deployment(
|
||||
Deployment(
|
||||
model_name="dual-name",
|
||||
litellm_params=LiteLLM_Params(
|
||||
model="auto_router/quality_router",
|
||||
quality_router_config={
|
||||
"quality_tiers": {"default": "backing-model"},
|
||||
"default_model": "backing-model",
|
||||
},
|
||||
),
|
||||
model_info={"id": "dual-quality-row"},
|
||||
)
|
||||
)
|
||||
assert "dual-name" in router.complexity_routers
|
||||
assert "dual-name" in router.quality_routers
|
||||
|
||||
router.delete_deployment(id="dual-complexity-row")
|
||||
|
||||
assert "dual-name" not in router.complexity_routers
|
||||
assert "dual-name" in router.quality_routers, (
|
||||
"deleting the complexity router must not evict the same-named quality router"
|
||||
)
|
||||
|
||||
def test_adaptive_router_delete_retires_post_call_hook(self):
|
||||
baseline = self._count_adaptive_hooks() # tolerate hooks leaked by other tests
|
||||
router = self._adaptive_router()
|
||||
assert "adaptive-test-router" in router.adaptive_routers
|
||||
assert self._count_adaptive_hooks() == 1
|
||||
assert self._count_adaptive_hooks() == baseline + 1
|
||||
|
||||
router.delete_deployment(id="adaptive-row")
|
||||
|
||||
assert "adaptive-test-router" not in router.adaptive_routers
|
||||
assert self._count_adaptive_hooks() == 0, "deleted adaptive router's hook must be retired"
|
||||
assert self._count_adaptive_hooks() == baseline, "deleted adaptive router's hook must be retired"
|
||||
|
||||
def test_adaptive_router_upsert_applies_new_config_without_leaking_hooks(self):
|
||||
from litellm.types.router import Deployment, LiteLLM_Params
|
||||
|
||||
baseline = self._count_adaptive_hooks() # tolerate hooks leaked by other tests
|
||||
router = self._adaptive_router()
|
||||
original = router.adaptive_routers["adaptive-test-router"]
|
||||
assert self._count_adaptive_hooks() == 1
|
||||
assert self._count_adaptive_hooks() == baseline + 1
|
||||
|
||||
router.upsert_deployment(
|
||||
Deployment(
|
||||
|
|
@ -5553,6 +5608,6 @@ class TestStrategyRegistryCleanupGuards:
|
|||
assert router.adaptive_routers["adaptive-test-router"] is not original, (
|
||||
"upsert must rebuild the adaptive router with the new config"
|
||||
)
|
||||
assert self._count_adaptive_hooks() == 1, (
|
||||
assert self._count_adaptive_hooks() == baseline + 1, (
|
||||
"exactly one hook must remain after upsert (old retired, new registered)"
|
||||
)
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue