mirror of
https://github.com/BerriAI/litellm.git
synced 2026-11-14 09:03:16 +00:00
fix(router): exclude the configured deployment on failover for client-side credentials
Weighted failover excluded the per-request id that client-supplied credentials generate, so the failed configured deployment stayed eligible for the re-pick. Stamp the configured id alongside it and exclude that. Also drop the vector store create stamp, which no fallback path reads, and redundant test comments. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013FDwtoMjnHa4xxazGJJNSm
This commit is contained in:
parent
d0a75d28cf
commit
10836dadc3
2 changed files with 49 additions and 18 deletions
|
|
@ -5169,13 +5169,17 @@ class PythonCore(DeprecatedRouterMembers):
|
|||
# _update_kwargs_with_deployment and stamped into kwargs["model_info"]; stamping
|
||||
# the static shared deployment's id instead would let one tenant's bad credentials
|
||||
# cool down the deployment every other tenant sharing this config relies on.
|
||||
if getattr(exception, "failed_deployment_id", None):
|
||||
return
|
||||
effective_model_info: Final = kwargs.get("model_info") or deployment.get("model_info") or MappingProxyType({})
|
||||
self._set_failed_deployment_id_on_exception(exception, MappingProxyType({"model_info": effective_model_info}))
|
||||
configured_deployment_id: Final = _MODEL_INFO_ADAPTER.validate_python(effective_model_info).get(
|
||||
"original_model_id"
|
||||
)
|
||||
if isinstance(configured_deployment_id, str) and configured_deployment_id:
|
||||
exception.failed_configured_deployment_id = configured_deployment_id # pyright: ignore[reportAttributeAccessIssue] # rebind-ok: the exception carries the failure to _maybe_run_weighted_failover
|
||||
|
||||
def _stamp_failed_attempt(self, exception: Exception, kwargs: Mapping[str, object], *, picked: bool) -> None:
|
||||
"""An attempt's exception handler: once it picked a deployment, stamp that deployment (the
|
||||
``model_info`` ``_update_kwargs_with_deployment`` put on ``kwargs``) as ``failed_deployment_id``,
|
||||
which weighted failover reads. A failed pick stamps nothing."""
|
||||
if picked:
|
||||
self._stamp_failed_deployment_id_with_effective_model_info(exception, MappingProxyType({}), kwargs)
|
||||
|
||||
|
|
@ -7996,7 +8000,6 @@ class PythonCore(DeprecatedRouterMembers):
|
|||
Returns:
|
||||
VectorStoreCreateResponse
|
||||
"""
|
||||
deployment = None # rebind-ok: pre-init so the except block can stamp a failure with no deployment picked
|
||||
try:
|
||||
# If model is None, use the factory function approach (direct SDK call)
|
||||
if model is None:
|
||||
|
|
@ -8009,7 +8012,7 @@ class PythonCore(DeprecatedRouterMembers):
|
|||
from litellm.vector_stores import acreate as avector_store_create_sdk
|
||||
|
||||
parent_otel_span: Final = get_parent_otel_span_from_kwargs(kwargs)
|
||||
deployment = await self._core_async_get_available_deployment( # rebind-ok: set on success, see pre-init
|
||||
deployment: Final = await self._core_async_get_available_deployment(
|
||||
model=model,
|
||||
messages=[{"role": "user", "content": "vector-store-api-fake-text"}],
|
||||
specific_deployment=kwargs.pop("specific_deployment", None),
|
||||
|
|
@ -8058,9 +8061,6 @@ class PythonCore(DeprecatedRouterMembers):
|
|||
)
|
||||
if model is not None:
|
||||
self.fail_calls[model] += 1
|
||||
self._stamp_failed_attempt(
|
||||
e, _REQUEST_KWARGS_ADAPTER.validate_python(kwargs), picked=deployment is not None
|
||||
)
|
||||
raise e
|
||||
|
||||
def _override_vector_store_methods_for_router(self):
|
||||
|
|
@ -9070,8 +9070,9 @@ class PythonCore(DeprecatedRouterMembers):
|
|||
kwargs[metadata_variable_name] = meta
|
||||
if not isinstance(meta, dict):
|
||||
return None
|
||||
configured_failed_id: Final[str | None] = getattr(exception, "failed_configured_deployment_id", None)
|
||||
prev_excluded: Final = set(meta.get("_failover_excluded_ids") or [])
|
||||
excluded: Final = prev_excluded | {failed_id}
|
||||
excluded: Final = prev_excluded | {configured_failed_id or failed_id}
|
||||
|
||||
all_ids: Final = {
|
||||
(d.get("model_info") or {}).get("id")
|
||||
|
|
|
|||
|
|
@ -1021,14 +1021,7 @@ async def test_failover_reaches_healthy_sibling_when_affinity_pins_failed_deploy
|
|||
assert after_a_recovers._hidden_params["model_id"] == "A"
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Every endpoint's attempt stamps the failed deployment, so weighted failover
|
||||
# covers more than chat completions
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
def _two_deployment_router(model: str) -> Router:
|
||||
"""Deployment A (always picked first) fails, deployment B answers."""
|
||||
return Router(
|
||||
model_list=[
|
||||
{
|
||||
|
|
@ -1064,8 +1057,6 @@ _ENDPOINTS: Final = [
|
|||
async def test_weighted_failover_moves_every_endpoint_to_another_deployment(
|
||||
monkeypatch: pytest.MonkeyPatch, endpoint: str, model: str, arguments: dict[str, object]
|
||||
) -> None:
|
||||
"""A failed attempt on any endpoint names its deployment, so weighted failover re-picks
|
||||
within the group instead of raising (#27980 only stamped completions and generic calls)."""
|
||||
keys_called: list[object] = []
|
||||
answer: Final = object()
|
||||
|
||||
|
|
@ -1084,6 +1075,45 @@ async def test_weighted_failover_moves_every_endpoint_to_another_deployment(
|
|||
assert keys_called == ["bad", "good"]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_weighted_failover_skips_the_failed_deployment_for_clientside_credentials(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
api_bases_called: list[object] = []
|
||||
answer: Final = object()
|
||||
|
||||
async def provider(**kwargs: object) -> object:
|
||||
api_bases_called.append(kwargs.get("api_base"))
|
||||
if kwargs.get("api_base") == "http://a":
|
||||
raise litellm.InternalServerError(message="A is down", llm_provider="openai", model="m")
|
||||
return answer
|
||||
|
||||
monkeypatch.setattr(litellm, "aembedding", provider)
|
||||
router: Final = Router(
|
||||
model_list=[
|
||||
{
|
||||
"model_name": "test-model",
|
||||
"litellm_params": {"model": "openai/text-embedding-3-small", "api_base": "http://a", "weight": 1},
|
||||
"model_info": {"id": "A"},
|
||||
},
|
||||
{
|
||||
"model_name": "test-model",
|
||||
"litellm_params": {"model": "openai/text-embedding-3-small", "api_base": "http://b", "weight": 0},
|
||||
"model_info": {"id": "B"},
|
||||
},
|
||||
],
|
||||
routing_strategy="simple-shuffle",
|
||||
num_retries=0,
|
||||
disable_cooldowns=True,
|
||||
enable_weighted_failover=True,
|
||||
)
|
||||
|
||||
response: Final = await router.aembedding(model="test-model", input=["hi"], api_key="tenant-key")
|
||||
|
||||
assert response is answer
|
||||
assert api_bases_called == ["http://a", "http://b"]
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
("endpoint", "attempt", "arguments"),
|
||||
[
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue