From 10836dadc3fcd0a1b435e79bd7cd28a828e9342c Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 4 Nov 2026 18:42:16 +0000 Subject: [PATCH] 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 Claude-Session: https://claude.ai/code/session_013FDwtoMjnHa4xxazGJJNSm --- litellm/router_python_core.py | 19 ++++---- tests/unit/test_router_weighted_failover.py | 48 +++++++++++++++++---- 2 files changed, 49 insertions(+), 18 deletions(-) diff --git a/litellm/router_python_core.py b/litellm/router_python_core.py index bf7e6fd2417..87ccf37dd83 100644 --- a/litellm/router_python_core.py +++ b/litellm/router_python_core.py @@ -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") diff --git a/tests/unit/test_router_weighted_failover.py b/tests/unit/test_router_weighted_failover.py index 1f797b02dd4..70231b620be 100644 --- a/tests/unit/test_router_weighted_failover.py +++ b/tests/unit/test_router_weighted_failover.py @@ -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"), [