From b69b4b3ba4875b16c4f463667d0f37d78af00beb Mon Sep 17 00:00:00 2001 From: Alex Castillo Date: Fri, 2 Oct 2026 22:40:21 -0400 Subject: [PATCH 1/3] fix(sap): fail gracefully and allow overriding orchestration deployment discovery deployment_url indexed into an empty sorted list with no deployments found, raising a bare IndexError that pointed nowhere near the real problem. get_complete_url also had no way to skip discovery: neither the deployment_url optional_param that transform_request already recognized (and silently dropped) nor an env var did anything. Raise GenAIHubOrchestrationError with a descriptive message when discovery finds nothing, warn when it finds more than one and has to pick, and let deployment_url in optional_params or AICORE_ORCHESTRATION_DEPLOYMENT_URL skip discovery entirely. Fixes #41805 --- litellm/llms/sap/chat/transformation.py | 37 ++++++-- .../llms/sap/chat/test_sap_transformation.py | 92 +++++++++++++++++++ 2 files changed, 123 insertions(+), 6 deletions(-) diff --git a/litellm/llms/sap/chat/transformation.py b/litellm/llms/sap/chat/transformation.py index 2955b8f16c5..620ff8d69f2 100755 --- a/litellm/llms/sap/chat/transformation.py +++ b/litellm/llms/sap/chat/transformation.py @@ -2,6 +2,7 @@ Translate from OpenAI's `/v1/chat/completions` to SAP Generative AI Hub's Orchestration Service`v2/completion` """ +import os from collections.abc import AsyncIterator, Iterator from functools import cached_property from typing import TYPE_CHECKING, Any, Final, Union @@ -172,7 +173,7 @@ class GenAIHubOrchestrationConfig(OpenAIGPTConfig): client: Final = litellm.module_level_client # with httpx.Client(timeout=30) as client: deployments: Final = client.get(f"{self.base_url}/lm/deployments", headers=self.headers).json() - valid: Final[list[tuple[str, str]]] = [] + valid: Final[list[tuple[str, str, str]]] = [] for dep in deployments.get("resources", []): if dep.get("scenarioId") == "orchestration": cfg = client.get( @@ -180,9 +181,29 @@ class GenAIHubOrchestrationConfig(OpenAIGPTConfig): headers=self.headers, ).json() if cfg.get("executableId") == "orchestration": - valid.append((dep["deploymentUrl"], dep["createdAt"])) - # newest first - return sorted(valid, key=lambda x: x[1], reverse=True)[0][0] + valid.append((dep["deploymentUrl"], dep["createdAt"], cfg.get("name", ""))) + if not valid: + raise GenAIHubOrchestrationError( + status_code=400, + message=( + "No orchestration deployment found for this SAP AI Core resource group. Create one, or set " + "AICORE_ORCHESTRATION_DEPLOYMENT_URL, or pass deployment_url in optional_params, to skip " + "discovery." + ), + ) + # newest first + ranked: Final = sorted(valid, key=lambda dep: dep[1], reverse=True) + if len(ranked) > 1: + chosen_url, _, chosen_name = ranked[0] + others: Final = [name for _, _, name in ranked[1:]] + litellm.verbose_logger.warning( + "SAP: %d orchestration deployments found; using newest (name=%r, url=%r). Others ignored: %r.", + len(ranked), + chosen_name, + chosen_url, + others, + ) + return ranked[0][0] @classmethod def get_config(cls): @@ -249,8 +270,12 @@ class GenAIHubOrchestrationConfig(OpenAIGPTConfig): litellm_params: dict, stream: bool | None = None, ): - api_base_: Final = f"{self.deployment_url}/v2/completion" - return api_base_ + deployment_url: Final = ( + optional_params.get("deployment_url") + or os.environ.get("AICORE_ORCHESTRATION_DEPLOYMENT_URL") + or self.deployment_url + ) + return f"{deployment_url}/v2/completion" def _build_prompt_module( self, diff --git a/tests/unit/llms/sap/chat/test_sap_transformation.py b/tests/unit/llms/sap/chat/test_sap_transformation.py index 3601bdd0d5e..79c1210f787 100644 --- a/tests/unit/llms/sap/chat/test_sap_transformation.py +++ b/tests/unit/llms/sap/chat/test_sap_transformation.py @@ -639,3 +639,95 @@ class TestSAPTransformationIntegration: config["config"]["modules"][1]["translation"]["input"]["type"] == "sap_document_translation" ) + + +class TestDeploymentUrlResolution: + """get_complete_url must skip discovery when an override is given, and + deployment_url must fail gracefully, not with a bare IndexError, when + discovery finds nothing.""" + + @pytest.fixture + def mock_config(self): + from litellm.llms.sap.chat.transformation import GenAIHubOrchestrationConfig + + config = GenAIHubOrchestrationConfig() + config.token_creator = lambda: "Bearer TEST_TOKEN" + config._base_url = "https://api.test-sap.com" + config._resource_group = "test-group" + + return config + + @staticmethod + def _mock_client(*names: str): + from unittest.mock import MagicMock + + resources = [ + { + "scenarioId": "orchestration", + "configurationId": f"cfg-{n}", + "deploymentUrl": f"https://deploy-{n}.sap.com", + "createdAt": f"2024-01-{i + 1:02d}T00:00:00Z", + } + for i, n in enumerate(names) + ] + configs = {f"cfg-{n}": {"executableId": "orchestration", "name": n} for n in names} + + def fake_get(url, headers=None): + resp = MagicMock() + if "/lm/deployments" in url: + resp.json.return_value = {"resources": resources} + else: + cfg_id = url.split("/")[-1] + resp.json.return_value = configs.get(cfg_id, {}) + return resp + + client = MagicMock() + client.get.side_effect = fake_get + return client + + def test_optional_param_skips_discovery(self, mock_config, monkeypatch): + from unittest.mock import MagicMock + + explicit = "https://custom.sap.com/deployments/abc" + mock_client = MagicMock() + monkeypatch.setattr("litellm.module_level_client", mock_client) + + url = mock_config.get_complete_url(None, None, "gpt-4o", {"deployment_url": explicit}, {}) + + assert url == f"{explicit}/v2/completion" + assert not mock_client.get.called + + def test_env_var_skips_discovery(self, mock_config, monkeypatch): + from unittest.mock import MagicMock + + env_url = "https://env.sap.com/deployments/env" + mock_client = MagicMock() + monkeypatch.setattr("litellm.module_level_client", mock_client) + monkeypatch.setenv("AICORE_ORCHESTRATION_DEPLOYMENT_URL", env_url) + + url = mock_config.get_complete_url(None, None, "gpt-4o", {}, {}) + + assert url == f"{env_url}/v2/completion" + assert not mock_client.get.called + + def test_no_deployments_raises_orchestration_error(self, mock_config, monkeypatch): + from litellm.llms.sap.chat.handler import GenAIHubOrchestrationError + + monkeypatch.setattr("litellm.module_level_client", self._mock_client()) + monkeypatch.setenv("AICORE_ORCHESTRATION_DEPLOYMENT_URL", "") + + with pytest.raises(GenAIHubOrchestrationError, match="No orchestration deployment found"): + mock_config.get_complete_url(None, None, "gpt-4o", {}, {}) + + def test_discovery_picks_newest_of_multiple_and_warns(self, mock_config, monkeypatch, caplog): + monkeypatch.setattr("litellm.module_level_client", self._mock_client("older", "newer")) + monkeypatch.setenv("AICORE_ORCHESTRATION_DEPLOYMENT_URL", "") + + with caplog.at_level("WARNING"): + url = mock_config.get_complete_url(None, None, "gpt-4o", {}, {}) + + assert url == "https://deploy-newer.sap.com/v2/completion" + assert any( + "2 orchestration deployments found" in record.getMessage() and "'older'" in record.getMessage() + for record in caplog.records + ) From fd2f5c132d55b94f96273b8a5700ceec9768804b Mon Sep 17 00:00:00 2001 From: Alex Castillo Date: Fri, 2 Oct 2026 22:48:58 -0400 Subject: [PATCH 2/3] Trigger CI rerun (mcp-integration failed on a dependency-install network error, unrelated to this change) From f8597b345346295594b2ded17a008131796b49c9 Mon Sep 17 00:00:00 2001 From: Alex Castillo Date: Sat, 3 Oct 2026 00:35:18 -0400 Subject: [PATCH 3/3] fix(sap): address Greptile review on #44314 - Strip a trailing slash from any deployment_url override before appending /v2/completion, so an override ending in / does not produce a double slash. - Include each ignored deployment's URL alongside its name in the multiple-deployments warning, so same-named deployments stay distinguishable. - Add full type annotations (Final, pytest.MonkeyPatch, pytest.LogCaptureFixture, return types) to the new test class, per AGENTS.md. --- litellm/llms/sap/chat/transformation.py | 4 +- .../llms/sap/chat/test_sap_transformation.py | 64 ++++++++++++++----- 2 files changed, 49 insertions(+), 19 deletions(-) diff --git a/litellm/llms/sap/chat/transformation.py b/litellm/llms/sap/chat/transformation.py index 620ff8d69f2..65028a2b042 100755 --- a/litellm/llms/sap/chat/transformation.py +++ b/litellm/llms/sap/chat/transformation.py @@ -195,7 +195,7 @@ class GenAIHubOrchestrationConfig(OpenAIGPTConfig): ranked: Final = sorted(valid, key=lambda dep: dep[1], reverse=True) if len(ranked) > 1: chosen_url, _, chosen_name = ranked[0] - others: Final = [name for _, _, name in ranked[1:]] + others: Final = [(name, url) for url, _, name in ranked[1:]] litellm.verbose_logger.warning( "SAP: %d orchestration deployments found; using newest (name=%r, url=%r). Others ignored: %r.", len(ranked), @@ -275,7 +275,7 @@ class GenAIHubOrchestrationConfig(OpenAIGPTConfig): or os.environ.get("AICORE_ORCHESTRATION_DEPLOYMENT_URL") or self.deployment_url ) - return f"{deployment_url}/v2/completion" + return f"{deployment_url.rstrip('/')}/v2/completion" def _build_prompt_module( self, diff --git a/tests/unit/llms/sap/chat/test_sap_transformation.py b/tests/unit/llms/sap/chat/test_sap_transformation.py index 79c1210f787..12b47b84306 100644 --- a/tests/unit/llms/sap/chat/test_sap_transformation.py +++ b/tests/unit/llms/sap/chat/test_sap_transformation.py @@ -1,7 +1,15 @@ +from __future__ import annotations + import warnings +from typing import TYPE_CHECKING, Final +from unittest.mock import MagicMock + import pytest from pydantic import ValidationError +if TYPE_CHECKING: + from litellm.llms.sap.chat.transformation import GenAIHubOrchestrationConfig + class TestSAPTransformationIntegration: """Integration tests for SAP transformation.""" @@ -641,13 +649,15 @@ class TestSAPTransformationIntegration: ) + + class TestDeploymentUrlResolution: """get_complete_url must skip discovery when an override is given, and deployment_url must fail gracefully, not with a bare IndexError, when discovery finds nothing.""" @pytest.fixture - def mock_config(self): + def mock_config(self) -> GenAIHubOrchestrationConfig: from litellm.llms.sap.chat.transformation import GenAIHubOrchestrationConfig config = GenAIHubOrchestrationConfig() @@ -658,10 +668,8 @@ class TestDeploymentUrlResolution: return config @staticmethod - def _mock_client(*names: str): - from unittest.mock import MagicMock - - resources = [ + def _mock_client(*names: str) -> MagicMock: + resources: list[dict[str, str]] = [ { "scenarioId": "orchestration", "configurationId": f"cfg-{n}", @@ -670,9 +678,11 @@ class TestDeploymentUrlResolution: } for i, n in enumerate(names) ] - configs = {f"cfg-{n}": {"executableId": "orchestration", "name": n} for n in names} + configs: dict[str, dict[str, str]] = { + f"cfg-{n}": {"executableId": "orchestration", "name": n} for n in names + } - def fake_get(url, headers=None): + def fake_get(url: str, headers: dict[str, str] | None = None) -> MagicMock: resp = MagicMock() if "/lm/deployments" in url: resp.json.return_value = {"resources": resources} @@ -685,10 +695,10 @@ class TestDeploymentUrlResolution: client.get.side_effect = fake_get return client - def test_optional_param_skips_discovery(self, mock_config, monkeypatch): - from unittest.mock import MagicMock - - explicit = "https://custom.sap.com/deployments/abc" + def test_optional_param_skips_discovery( + self, mock_config: GenAIHubOrchestrationConfig, monkeypatch: pytest.MonkeyPatch + ) -> None: + explicit: Final[str] = "https://custom.sap.com/deployments/abc" mock_client = MagicMock() monkeypatch.setattr("litellm.module_level_client", mock_client) @@ -697,10 +707,21 @@ class TestDeploymentUrlResolution: assert url == f"{explicit}/v2/completion" assert not mock_client.get.called - def test_env_var_skips_discovery(self, mock_config, monkeypatch): - from unittest.mock import MagicMock + def test_optional_param_strips_trailing_slash( + self, mock_config: GenAIHubOrchestrationConfig, monkeypatch: pytest.MonkeyPatch + ) -> None: + monkeypatch.setattr("litellm.module_level_client", MagicMock()) - env_url = "https://env.sap.com/deployments/env" + url = mock_config.get_complete_url( + None, None, "gpt-4o", {"deployment_url": "https://custom.sap.com/deployments/abc/"}, {} + ) + + assert url == "https://custom.sap.com/deployments/abc/v2/completion" + + def test_env_var_skips_discovery( + self, mock_config: GenAIHubOrchestrationConfig, monkeypatch: pytest.MonkeyPatch + ) -> None: + env_url: Final[str] = "https://env.sap.com/deployments/env" mock_client = MagicMock() monkeypatch.setattr("litellm.module_level_client", mock_client) monkeypatch.setenv("AICORE_ORCHESTRATION_DEPLOYMENT_URL", env_url) @@ -710,7 +731,9 @@ class TestDeploymentUrlResolution: assert url == f"{env_url}/v2/completion" assert not mock_client.get.called - def test_no_deployments_raises_orchestration_error(self, mock_config, monkeypatch): + def test_no_deployments_raises_orchestration_error( + self, mock_config: GenAIHubOrchestrationConfig, monkeypatch: pytest.MonkeyPatch + ) -> None: from litellm.llms.sap.chat.handler import GenAIHubOrchestrationError monkeypatch.setattr("litellm.module_level_client", self._mock_client()) @@ -719,7 +742,12 @@ class TestDeploymentUrlResolution: with pytest.raises(GenAIHubOrchestrationError, match="No orchestration deployment found"): mock_config.get_complete_url(None, None, "gpt-4o", {}, {}) - def test_discovery_picks_newest_of_multiple_and_warns(self, mock_config, monkeypatch, caplog): + def test_discovery_picks_newest_of_multiple_and_warns( + self, + mock_config: GenAIHubOrchestrationConfig, + monkeypatch: pytest.MonkeyPatch, + caplog: pytest.LogCaptureFixture, + ) -> None: monkeypatch.setattr("litellm.module_level_client", self._mock_client("older", "newer")) monkeypatch.setenv("AICORE_ORCHESTRATION_DEPLOYMENT_URL", "") @@ -728,6 +756,8 @@ class TestDeploymentUrlResolution: assert url == "https://deploy-newer.sap.com/v2/completion" assert any( - "2 orchestration deployments found" in record.getMessage() and "'older'" in record.getMessage() + "2 orchestration deployments found" in record.getMessage() + and "'older'" in record.getMessage() + and "deploy-older.sap.com" in record.getMessage() for record in caplog.records )