mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-07 08:26:10 +00:00
fix(routing_groups): own strategy validation in router_utils and cover group swap
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
This commit is contained in:
parent
da8833fc28
commit
b829586071
8 changed files with 94 additions and 46 deletions
|
|
@ -105,7 +105,7 @@
|
|||
"limit": 109
|
||||
},
|
||||
"reportUnknownMemberType": {
|
||||
"limit": 38808
|
||||
"limit": 38806
|
||||
},
|
||||
"reportUnknownParameterType": {
|
||||
"limit": 19829
|
||||
|
|
|
|||
|
|
@ -6291,10 +6291,7 @@ class ProxyConfig:
|
|||
if raw_groups is None:
|
||||
return router_settings
|
||||
try:
|
||||
parse_routing_groups(
|
||||
TypeAdapter(list[RoutingGroup]).validate_python(raw_groups),
|
||||
validate_strategy=Router._validate_routing_strategy,
|
||||
)
|
||||
parse_routing_groups(TypeAdapter(list[RoutingGroup]).validate_python(raw_groups))
|
||||
except ValueError as validation_error:
|
||||
verbose_proxy_logger.error(
|
||||
"Ignoring invalid router_settings.routing_groups from config/DB, all other router settings still "
|
||||
|
|
@ -15790,10 +15787,7 @@ async def update_config(
|
|||
|
||||
if config_info.router_settings is not None:
|
||||
try:
|
||||
parse_routing_groups(
|
||||
config_info.router_settings.routing_groups,
|
||||
validate_strategy=Router._validate_routing_strategy,
|
||||
)
|
||||
parse_routing_groups(config_info.router_settings.routing_groups)
|
||||
except ValueError as validation_error:
|
||||
raise HTTPException(status_code=400, detail={"error": str(validation_error)})
|
||||
|
||||
|
|
|
|||
|
|
@ -179,7 +179,7 @@ from litellm.router_utils.router_callbacks.track_deployment_metrics import (
|
|||
increment_deployment_failures_for_current_minute,
|
||||
increment_deployment_successes_for_current_minute,
|
||||
)
|
||||
from litellm.router_utils.routing_groups import parse_routing_groups
|
||||
from litellm.router_utils.routing_groups import parse_routing_groups, validate_routing_strategy
|
||||
from litellm.scheduler import FlowItem, Scheduler
|
||||
from litellm.types.llms.openai import (
|
||||
AllMessageValues,
|
||||
|
|
@ -1081,19 +1081,7 @@ class Router:
|
|||
|
||||
@staticmethod
|
||||
def _validate_routing_strategy(routing_strategy: RoutingStrategy | str | None) -> None:
|
||||
# See: https://github.com/BerriAI/litellm/issues/11330
|
||||
valid_strategy_strings: Final = ["simple-shuffle", "lar1"] + [s.value for s in RoutingStrategy]
|
||||
if routing_strategy is None:
|
||||
return
|
||||
is_valid_string: Final = isinstance(routing_strategy, str) and routing_strategy in valid_strategy_strings
|
||||
is_valid_enum: Final = isinstance(routing_strategy, RoutingStrategy)
|
||||
if not is_valid_string and not is_valid_enum:
|
||||
raise ValueError(
|
||||
f"Invalid routing_strategy: '{routing_strategy}'. "
|
||||
f"Valid options: {valid_strategy_strings}. "
|
||||
f"Check 'router_settings.routing_strategy' in your config.yaml "
|
||||
f"or the 'routing_strategy' parameter if using the Router SDK directly."
|
||||
)
|
||||
validate_routing_strategy(routing_strategy)
|
||||
|
||||
def _build_strategy_selector(
|
||||
self,
|
||||
|
|
@ -1207,17 +1195,14 @@ class Router:
|
|||
self._replace_routing_groups(())
|
||||
return
|
||||
|
||||
known_model_names: Final = frozenset(m["model_name"] for m in (self.model_list or []) if m.get("model_name"))
|
||||
groups: Final = parse_routing_groups(
|
||||
groups_input,
|
||||
validate_strategy=self._validate_routing_strategy,
|
||||
known_model_names=known_model_names,
|
||||
)
|
||||
known_model_names: Final = frozenset(m["model_name"] for m in (self.model_list or ()) if m.get("model_name"))
|
||||
groups: Final = parse_routing_groups(groups_input, known_model_names=known_model_names)
|
||||
|
||||
alias_names: Final = frozenset(self.model_group_alias or ())
|
||||
shadowed_names: Final = tuple(
|
||||
group.group_name
|
||||
for group in groups
|
||||
if group.group_name in known_model_names or group.group_name in (self.model_group_alias or {})
|
||||
if group.group_name in known_model_names or group.group_name in alias_names
|
||||
)
|
||||
for shadowed_name in shadowed_names:
|
||||
verbose_router_logger.warning(
|
||||
|
|
@ -1230,7 +1215,7 @@ class Router:
|
|||
failures: Final = tuple(outcome for _, outcome in built if isinstance(outcome, ValidationError))
|
||||
if failures:
|
||||
self._unregister_router_selectors(
|
||||
[outcome for _, outcome in built if not isinstance(outcome, ValidationError)]
|
||||
tuple(outcome for _, outcome in built if not isinstance(outcome, ValidationError))
|
||||
)
|
||||
raise failures[0]
|
||||
|
||||
|
|
@ -1246,7 +1231,7 @@ class Router:
|
|||
self, "_group_selectors", {}
|
||||
)
|
||||
self._unregister_router_selectors(
|
||||
[sel for selectors in previous_selectors.values() for sel in selectors.values()]
|
||||
tuple(sel for selectors in previous_selectors.values() for sel in selectors.values())
|
||||
)
|
||||
|
||||
self._routing_groups: dict[str, RoutingGroup] = {group.group_name: group for group, _ in built}
|
||||
|
|
|
|||
|
|
@ -4,18 +4,36 @@ proxy's config-update endpoint so a config the UI saves cannot be one the
|
|||
runtime refuses to load.
|
||||
"""
|
||||
|
||||
from collections.abc import Callable, Sequence
|
||||
from collections.abc import Sequence
|
||||
from typing import Final
|
||||
|
||||
from litellm._logging import verbose_router_logger
|
||||
from litellm.types.router import RoutingGroup, RoutingStrategy
|
||||
|
||||
ValidateStrategy = Callable[[RoutingStrategy | str | None], None]
|
||||
|
||||
def validate_routing_strategy(routing_strategy: RoutingStrategy | str | None) -> None:
|
||||
"""
|
||||
Raises `ValueError` unless `routing_strategy` is a known strategy or None.
|
||||
|
||||
See: https://github.com/BerriAI/litellm/issues/11330
|
||||
"""
|
||||
if routing_strategy is None:
|
||||
return
|
||||
|
||||
valid_strategy_strings: Final = ("simple-shuffle", "lar1", *(s.value for s in RoutingStrategy))
|
||||
is_valid_string: Final = isinstance(routing_strategy, str) and routing_strategy in valid_strategy_strings
|
||||
is_valid_enum: Final = isinstance(routing_strategy, RoutingStrategy)
|
||||
if not is_valid_string and not is_valid_enum:
|
||||
raise ValueError(
|
||||
f"Invalid routing_strategy: '{routing_strategy}'. "
|
||||
f"Valid options: {list(valid_strategy_strings)}. "
|
||||
f"Check 'router_settings.routing_strategy' in your config.yaml "
|
||||
f"or the 'routing_strategy' parameter if using the Router SDK directly."
|
||||
)
|
||||
|
||||
|
||||
def parse_routing_groups(
|
||||
groups_input: Sequence[RoutingGroup | dict] | None,
|
||||
validate_strategy: ValidateStrategy,
|
||||
known_model_names: frozenset[str] = frozenset(),
|
||||
) -> tuple[RoutingGroup, ...]:
|
||||
"""
|
||||
|
|
@ -35,21 +53,21 @@ def parse_routing_groups(
|
|||
if any(group.group_name == "default" for group in groups):
|
||||
raise ValueError("routing_groups: 'default' is reserved for the implicit fallback group.")
|
||||
|
||||
names: Final = [group.group_name for group in groups]
|
||||
duplicate_names: Final = sorted({name for name in names if names.count(name) > 1})
|
||||
names: Final = tuple(group.group_name for group in groups)
|
||||
duplicate_names: Final = frozenset(name for name in names if names.count(name) > 1)
|
||||
if duplicate_names:
|
||||
raise ValueError(f"routing_groups: group names must be unique, duplicate group_name '{duplicate_names[0]}'.")
|
||||
raise ValueError(f"routing_groups: group names must be unique, duplicate group_name '{min(duplicate_names)}'.")
|
||||
|
||||
for group in groups:
|
||||
validate_strategy(group.routing_strategy)
|
||||
validate_routing_strategy(group.routing_strategy)
|
||||
|
||||
owners_by_model: Final = {
|
||||
model_name: tuple(group.group_name for group in groups if model_name in group.models)
|
||||
owners_by_model: Final = tuple(
|
||||
(model_name, tuple(group.group_name for group in groups if model_name in group.models))
|
||||
for model_name in dict.fromkeys(model_name for group in groups for model_name in group.models)
|
||||
}
|
||||
)
|
||||
conflicts: Final = tuple(
|
||||
f"model_name '{model_name}' appears in {' and '.join(repr(owner) for owner in owners)}"
|
||||
for model_name, owners in owners_by_model.items()
|
||||
for model_name, owners in owners_by_model
|
||||
if len(owners) > 1
|
||||
)
|
||||
if conflicts:
|
||||
|
|
|
|||
|
|
@ -10,7 +10,6 @@ from unittest.mock import patch
|
|||
import pytest
|
||||
from pydantic import ValidationError
|
||||
|
||||
|
||||
import litellm
|
||||
from litellm import Router
|
||||
from litellm.types.router import RoutingGroup, RoutingStrategy
|
||||
|
|
@ -486,6 +485,45 @@ def test_build_strategy_selector_constructs_for_known_strategies(monkeypatch):
|
|||
assert selector is not None
|
||||
|
||||
|
||||
def test_replace_routing_groups_swaps_state_and_drops_old_selectors(monkeypatch):
|
||||
monkeypatch.setattr(litellm, "callbacks", [])
|
||||
monkeypatch.setattr(litellm, "input_callback", [])
|
||||
router = _build_router(
|
||||
routing_groups=[
|
||||
{
|
||||
"group_name": "fast",
|
||||
"models": ["filtered-model"],
|
||||
"routing_strategy": "least-busy",
|
||||
}
|
||||
]
|
||||
)
|
||||
old_selector = router._group_selectors["fast"]["least-busy"]
|
||||
|
||||
replacement = RoutingGroup(
|
||||
group_name="quality",
|
||||
models=["other-model"],
|
||||
routing_strategy="latency-based-routing",
|
||||
)
|
||||
new_selector = router._build_strategy_selector(
|
||||
strategy=replacement.routing_strategy,
|
||||
routing_strategy_args={},
|
||||
register_callbacks=True,
|
||||
)
|
||||
router._replace_routing_groups(((replacement, new_selector),))
|
||||
|
||||
assert set(router._routing_groups) == {"quality"}
|
||||
assert router._model_to_group == {"other-model": "quality"}
|
||||
assert router._group_selectors == {"quality": {"latency-based-routing": new_selector}}
|
||||
assert all(c is not old_selector for c in litellm.callbacks)
|
||||
assert new_selector in litellm.callbacks
|
||||
|
||||
router._replace_routing_groups(())
|
||||
assert router._routing_groups == {}
|
||||
assert router._model_to_group == {}
|
||||
assert router._group_selectors == {}
|
||||
assert all(c is not new_selector for c in litellm.callbacks)
|
||||
|
||||
|
||||
def test_unregister_router_selectors_removes_by_identity(monkeypatch):
|
||||
monkeypatch.setattr(litellm, "callbacks", [])
|
||||
monkeypatch.setattr(litellm, "input_callback", [])
|
||||
|
|
|
|||
|
|
@ -3,7 +3,7 @@
|
|||
"limit": 22733
|
||||
},
|
||||
"LIT002": {
|
||||
"limit": 26864
|
||||
"limit": 26860
|
||||
},
|
||||
"LIT003": {
|
||||
"limit": 269
|
||||
|
|
|
|||
|
|
@ -52,6 +52,7 @@ const renderModal = (overrides: Partial<React.ComponentProps<typeof RoutingGroup
|
|||
strategyDescriptions={STRATEGY_DESCRIPTIONS}
|
||||
modelOptions={MODEL_OPTIONS}
|
||||
existingGroupNames={["already-taken", "other-group"]}
|
||||
groupNameByModel={{}}
|
||||
onClose={onClose}
|
||||
onSubmit={onSubmit}
|
||||
{...overrides}
|
||||
|
|
@ -288,6 +289,18 @@ describe("RoutingGroupModal", () => {
|
|||
expect(onSubmit.mock.calls[0][0]).toStrictEqual(expected);
|
||||
});
|
||||
|
||||
it("blocks a model another group already claims", async () => {
|
||||
const user = userEvent.setup();
|
||||
const { onSubmit } = renderModal({ groupNameByModel: { "gpt-4o": "cheap" } });
|
||||
|
||||
await typeName(user, "security");
|
||||
await pickModels(user, "gpt-4o");
|
||||
await save(user, "Create Group");
|
||||
|
||||
expect(await screen.findByText(/Already claimed: gpt-4o/)).toBeInTheDocument();
|
||||
expect(onSubmit).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("describes the selected strategy", async () => {
|
||||
renderModal();
|
||||
|
||||
|
|
|
|||
|
|
@ -127,7 +127,7 @@ const RoutingGroupModal: React.FC<RoutingGroupModalProps> = ({
|
|||
control={form.control}
|
||||
name="group_name"
|
||||
label="Group Name"
|
||||
description="Use this name as the model in API calls — LiteLLM routes the request to one of the group's models."
|
||||
description="Names the shared routing strategy for these models. Requests still use the model names, not this name."
|
||||
>
|
||||
{({ ref, ...field }) => <Input {...field} ref={ref} placeholder="fast-chat" disabled={mode === "edit"} />}
|
||||
</FormField>
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue