From 3c9582f46fcc63cfd7db71b9c4adaedbfee912b1 Mon Sep 17 00:00:00 2001 From: Yamac Ay Date: Fri, 18 Sep 2026 12:29:44 +0200 Subject: [PATCH 1/7] fix(sap): multi orchestration deployments better handled --- litellm/llms/sap/chat/transformation.py | 84 +++++++++--- .../llms/sap/chat/test_sap_transformation.py | 120 ++++++++++++++++++ 2 files changed, 185 insertions(+), 19 deletions(-) diff --git a/litellm/llms/sap/chat/transformation.py b/litellm/llms/sap/chat/transformation.py index 2955b8f16c5..6a65066f4e6 100755 --- a/litellm/llms/sap/chat/transformation.py +++ b/litellm/llms/sap/chat/transformation.py @@ -2,13 +2,14 @@ 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 import httpx import litellm +from litellm._logging import verbose_logger from litellm.types.llms.openai import AllMessageValues from litellm.types.utils import ModelResponse @@ -39,6 +40,10 @@ from .models import ( SAPUserMessage, ) +# Env var that an operator can set to pin a specific orchestration deployment URL. +# An empty string is treated as unset and falls through to auto-discovery. +_AICORE_ORCHESTRATION_DEPLOYMENT_URL_ENV_VAR = "AICORE_ORCHESTRATION_DEPLOYMENT_URL" + # Keys routed outside SAP orchestration `model.params` (prompt, stream, fallbacks, etc.) _SAP_MODEL_PARAMS_EXCLUDED_KEYS: Final[frozenset[str]] = frozenset( { @@ -135,6 +140,7 @@ class GenAIHubOrchestrationConfig(OpenAIGPTConfig): self.token_creator = None self._base_url = None self._resource_group = None + self._cached_deployment_url: str | None = None # None = not yet resolved def run_env_setup(self, service_key: str | None = None) -> None: try: @@ -166,23 +172,62 @@ class GenAIHubOrchestrationConfig(OpenAIGPTConfig): self.run_env_setup() return self._resource_group - @cached_property + @property def deployment_url(self) -> str: - # Keep a short, tight client lifecycle here to avoid fd leaks - 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]]] = [] - for dep in deployments.get("resources", []): - if dep.get("scenarioId") == "orchestration": - cfg = client.get( - f"{self.base_url}/lm/configurations/{dep['configurationId']}", - 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] + """Resolve the orchestration deployment URL, with caching. + + Resolution order: + 1. ``_AICORE_ORCHESTRATION_DEPLOYMENT_URL_ENV_VAR`` env var (operator-level pin). + An empty string is treated as unset and falls through to discovery. + 2. Auto-discovery via ``/lm/deployments`` (one network call, cached). + + A per-request override via ``optional_params["deployment_url"]`` is + handled one level up in ``get_complete_url`` before this property + is ever called. + """ + cached = self._cached_deployment_url + if cached is None: + env = os.environ.get(_AICORE_ORCHESTRATION_DEPLOYMENT_URL_ENV_VAR) + cached = env or self._resolve_deployment_url() + self._cached_deployment_url = cached + return cached + + def _as_orchestration_candidate(self, dep: dict[str, Any]) -> tuple[str, str, str] | None: + if dep.get("scenarioId") != "orchestration": + return None + cfg: Final = litellm.module_level_client.get( + f"{self.base_url}/lm/configurations/{dep['configurationId']}", + headers=self.headers, + ).json() + if cfg.get("executableId") != "orchestration": + return None + return (dep["deploymentUrl"], dep["createdAt"], cfg.get("name", "")) + + def _resolve_deployment_url(self) -> str: + resources: Final = ( + litellm.module_level_client.get(f"{self.base_url}/lm/deployments", headers=self.headers) + .json() + .get("resources", []) + ) + candidates: Final[list[tuple[str, str, str]]] = sorted( + filter(None, (self._as_orchestration_candidate(dep) for dep in resources)), + key=lambda c: c[1], + reverse=True, + ) + if not candidates: + raise GenAIHubOrchestrationError( + status_code=404, + message="No orchestration deployment found in SAP AI Core.", + ) + if len(candidates) > 1: + verbose_logger.warning( + "SAP: %d orchestration deployments found; using newest (name=%r, url=%r). Others ignored: %s.", + len(candidates), + candidates[0][2], + candidates[0][0], + tuple(v[2] or v[0] for v in candidates[1:]), + ) + return candidates[0][0] @classmethod def get_config(cls): @@ -249,8 +294,9 @@ class GenAIHubOrchestrationConfig(OpenAIGPTConfig): litellm_params: dict, stream: bool | None = None, ): - api_base_: Final = f"{self.deployment_url}/v2/completion" - return api_base_ + # Per-request override wins; deployment_url handles env var + discovery. + base = optional_params.get("deployment_url") or self.deployment_url + return f"{base}/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..4e1c8fd7298 100644 --- a/tests/unit/llms/sap/chat/test_sap_transformation.py +++ b/tests/unit/llms/sap/chat/test_sap_transformation.py @@ -1,5 +1,6 @@ import warnings import pytest +from unittest.mock import MagicMock, patch from pydantic import ValidationError @@ -639,3 +640,122 @@ class TestSAPTransformationIntegration: config["config"]["modules"][1]["translation"]["input"]["type"] == "sap_document_translation" ) + + +# --------------------------------------------------------------------------- +# Helpers shared by deployment-resolution tests +# --------------------------------------------------------------------------- + + +def _make_config(): + from litellm.llms.sap.chat.transformation import GenAIHubOrchestrationConfig + + cfg = GenAIHubOrchestrationConfig() + cfg.token_creator = lambda: "Bearer TEST" + cfg._base_url = "https://api.test-sap.com" + cfg._resource_group = "test-group" + return cfg + + +def _mock_client(*names: str): + """Fake httpx client that returns deployment/config payloads for given names.""" + 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 + + +class TestDeploymentResolution: + def test_single_deployment_returns_url(self): + cfg = _make_config() + with patch("litellm.module_level_client", _mock_client("orch-a")): # test-quality-ok: patching the HTTP client at the litellm transport boundary + url = cfg._resolve_deployment_url() + assert url == "https://deploy-orch-a.sap.com" + + def test_multiple_deployments_picks_newest(self): + # "older" has createdAt 2024-01-01, "newer" has 2024-01-02 + cfg = _make_config() + with patch("litellm.module_level_client", _mock_client("older", "newer")): # test-quality-ok: patching the HTTP client at the litellm transport boundary + url = cfg._resolve_deployment_url() + assert url == "https://deploy-newer.sap.com" + + def test_multiple_deployments_emits_warning(self): + cfg = _make_config() + with patch("litellm.module_level_client", _mock_client("older", "newer")): # test-quality-ok: patching the HTTP client at the litellm transport boundary + with patch("litellm.llms.sap.chat.transformation.verbose_logger") as mock_log: # test-quality-ok: patching module-level logger to intercept warning calls + url = cfg._resolve_deployment_url() + mock_log.warning.assert_called_once() + assert 2 == mock_log.warning.call_args[0][1] + assert url == "https://deploy-newer.sap.com" + + def test_no_deployments_raises(self): + from litellm.llms.sap.chat.handler import GenAIHubOrchestrationError + + cfg = _make_config() + with patch("litellm.module_level_client", _mock_client()): # test-quality-ok: patching the HTTP client at the litellm transport boundary + with pytest.raises(GenAIHubOrchestrationError) as exc: + cfg._resolve_deployment_url() + assert "No orchestration deployment found" in str(exc.value) + + +class TestGetCompleteUrl: + def test_optional_params_used_first(self): + """Step 1: deployment_url in optional_params skips discovery entirely.""" + cfg = _make_config() + explicit = "https://custom.sap.com/deployments/abc" + mock = MagicMock() + with patch("litellm.module_level_client", mock): # test-quality-ok: patching the HTTP client at the litellm transport boundary + url = cfg.get_complete_url(None, None, "gpt-4o", {"deployment_url": explicit}, {}) + assert url == f"{explicit}/v2/completion" + mock.get.assert_not_called() + + def test_env_var_used_when_no_optional_param(self): + """Step 2: AICORE_ORCHESTRATION_DEPLOYMENT_URL skips discovery.""" + cfg = _make_config() + env_url = "https://env.sap.com/deployments/env" + mock = MagicMock() + with patch("litellm.module_level_client", mock): # test-quality-ok: patching the HTTP client at the litellm transport boundary + with patch.dict("os.environ", {"AICORE_ORCHESTRATION_DEPLOYMENT_URL": env_url}): + url = cfg.get_complete_url(None, None, "gpt-4o", {}, {}) + assert url == f"{env_url}/v2/completion" + mock.get.assert_not_called() + + def test_optional_params_beats_env_var(self): + """Step 1 takes precedence over step 2.""" + cfg = _make_config() + opt_url = "https://opt.sap.com/deployments/opt" + env_url = "https://env.sap.com/deployments/env" + mock = MagicMock() + with patch("litellm.module_level_client", mock): # test-quality-ok: patching the HTTP client at the litellm transport boundary + with patch.dict("os.environ", {"AICORE_ORCHESTRATION_DEPLOYMENT_URL": env_url}): + url = cfg.get_complete_url(None, None, "gpt-4o", {"deployment_url": opt_url}, {}) + assert url == f"{opt_url}/v2/completion" + mock.get.assert_not_called() + + def test_discovery_used_when_no_override(self): + """Step 3: no optional_param, no env var — discovery runs.""" + cfg = _make_config() + env = {"AICORE_ORCHESTRATION_DEPLOYMENT_URL": ""} # empty = falsy + with patch("litellm.module_level_client", _mock_client("orch-a")): # test-quality-ok: patching the HTTP client at the litellm transport boundary + with patch.dict("os.environ", env): + url = cfg.get_complete_url(None, None, "gpt-4o", {}, {}) + assert url == "https://deploy-orch-a.sap.com/v2/completion" From 7e1a3a746e14d624b10d5cdb28347fe2b038c555 Mon Sep 17 00:00:00 2001 From: Yamac Ay Date: Thu, 24 Sep 2026 13:41:20 +0200 Subject: [PATCH 2/7] retrigger pipeline From af4b0de39b39116ef5543720609a4c4168514c33 Mon Sep 17 00:00:00 2001 From: Yamac Ay Date: Fri, 25 Sep 2026 11:38:49 +0200 Subject: [PATCH 3/7] fix: use query parameters --- litellm/llms/sap/chat/transformation.py | 27 +++++++++++++------------ 1 file changed, 14 insertions(+), 13 deletions(-) diff --git a/litellm/llms/sap/chat/transformation.py b/litellm/llms/sap/chat/transformation.py index 6a65066f4e6..a2f08f58c09 100755 --- a/litellm/llms/sap/chat/transformation.py +++ b/litellm/llms/sap/chat/transformation.py @@ -192,25 +192,26 @@ class GenAIHubOrchestrationConfig(OpenAIGPTConfig): self._cached_deployment_url = cached return cached - def _as_orchestration_candidate(self, dep: dict[str, Any]) -> tuple[str, str, str] | None: - if dep.get("scenarioId") != "orchestration": - return None - cfg: Final = litellm.module_level_client.get( - f"{self.base_url}/lm/configurations/{dep['configurationId']}", - headers=self.headers, - ).json() - if cfg.get("executableId") != "orchestration": - return None - return (dep["deploymentUrl"], dep["createdAt"], cfg.get("name", "")) - def _resolve_deployment_url(self) -> str: resources: Final = ( - litellm.module_level_client.get(f"{self.base_url}/lm/deployments", headers=self.headers) + litellm.module_level_client.get( + f"{self.base_url}/lm/deployments", + headers=self.headers, + params={ + "scenarioId": "orchestration", + "executableIds": ["orchestration"], + "status": "RUNNING", + }, + ) .json() .get("resources", []) ) candidates: Final[list[tuple[str, str, str]]] = sorted( - filter(None, (self._as_orchestration_candidate(dep) for dep in resources)), + ( + (dep["deploymentUrl"], dep["createdAt"], dep.get("name", "")) + for dep in resources + if dep.get("deploymentUrl") + ), key=lambda c: c[1], reverse=True, ) From e775a6f0fec1e99b647ba460b58046c741ef555d Mon Sep 17 00:00:00 2001 From: Yamac Ay Date: Fri, 25 Sep 2026 14:04:59 +0200 Subject: [PATCH 4/7] tests & small name -> configurationName fix --- litellm/llms/sap/chat/transformation.py | 6 ++-- .../llms/sap/chat/test_sap_transformation.py | 35 ++++++++++++------- 2 files changed, 25 insertions(+), 16 deletions(-) diff --git a/litellm/llms/sap/chat/transformation.py b/litellm/llms/sap/chat/transformation.py index a2f08f58c09..9ad1971c380 100755 --- a/litellm/llms/sap/chat/transformation.py +++ b/litellm/llms/sap/chat/transformation.py @@ -197,9 +197,9 @@ class GenAIHubOrchestrationConfig(OpenAIGPTConfig): litellm.module_level_client.get( f"{self.base_url}/lm/deployments", headers=self.headers, - params={ + params={ # mutable-ok: one-shot query-params dict passed directly to httpx, not stored "scenarioId": "orchestration", - "executableIds": ["orchestration"], + "executableIds": ["orchestration"], # mutable-ok: list literal required by httpx multi-value param serialization "status": "RUNNING", }, ) @@ -208,7 +208,7 @@ class GenAIHubOrchestrationConfig(OpenAIGPTConfig): ) candidates: Final[list[tuple[str, str, str]]] = sorted( ( - (dep["deploymentUrl"], dep["createdAt"], dep.get("name", "")) + (dep["deploymentUrl"], dep["createdAt"], dep.get("configurationName", "")) for dep in resources if dep.get("deploymentUrl") ), diff --git a/tests/unit/llms/sap/chat/test_sap_transformation.py b/tests/unit/llms/sap/chat/test_sap_transformation.py index 4e1c8fd7298..d7cd27eb71f 100644 --- a/tests/unit/llms/sap/chat/test_sap_transformation.py +++ b/tests/unit/llms/sap/chat/test_sap_transformation.py @@ -658,29 +658,25 @@ def _make_config(): def _mock_client(*names: str): - """Fake httpx client that returns deployment/config payloads for given names.""" + """Fake httpx client returning a /lm/deployments response for the given deployment names. + + The backend filters by scenarioId/executableIds/status, so there is no + second call to /lm/configurations; the mock only needs one response shape. + """ resources = [ { "scenarioId": "orchestration", "configurationId": f"cfg-{n}", "deploymentUrl": f"https://deploy-{n}.sap.com", "createdAt": f"2024-01-{i + 1:02d}T00:00:00Z", + "configurationName": n, } 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 - + resp = MagicMock() + resp.json.return_value = {"resources": resources} client = MagicMock() - client.get.side_effect = fake_get + client.get.return_value = resp return client @@ -716,6 +712,19 @@ class TestDeploymentResolution: cfg._resolve_deployment_url() assert "No orchestration deployment found" in str(exc.value) + def test_query_params_sent_to_backend(self): + """The whole point of this refactor: filtering must happen server-side.""" + cfg = _make_config() + mock = _mock_client("orch-a") + with patch("litellm.module_level_client", mock): # test-quality-ok: patching the HTTP client at the litellm transport boundary + cfg._resolve_deployment_url() + _, kwargs = mock.get.call_args + assert kwargs["params"] == { + "scenarioId": "orchestration", + "executableIds": ["orchestration"], + "status": "RUNNING", + } + class TestGetCompleteUrl: def test_optional_params_used_first(self): From cde9869e83c40e39bdeedd9b4eaa2572500713ee Mon Sep 17 00:00:00 2001 From: Yamac Ay Date: Fri, 25 Sep 2026 14:09:11 +0200 Subject: [PATCH 5/7] lint: reformatted --- litellm/llms/sap/chat/transformation.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/litellm/llms/sap/chat/transformation.py b/litellm/llms/sap/chat/transformation.py index 9ad1971c380..64d83f4db39 100755 --- a/litellm/llms/sap/chat/transformation.py +++ b/litellm/llms/sap/chat/transformation.py @@ -199,7 +199,9 @@ class GenAIHubOrchestrationConfig(OpenAIGPTConfig): headers=self.headers, params={ # mutable-ok: one-shot query-params dict passed directly to httpx, not stored "scenarioId": "orchestration", - "executableIds": ["orchestration"], # mutable-ok: list literal required by httpx multi-value param serialization + "executableIds": [ + "orchestration" + ], # mutable-ok: list literal required by httpx multi-value param serialization "status": "RUNNING", }, ) From 3c8b81bf703eea975e752c16525f10e72b0067b8 Mon Sep 17 00:00:00 2001 From: Yamac Ay Date: Fri, 25 Sep 2026 14:25:16 +0200 Subject: [PATCH 6/7] lint: reformatted --- litellm/llms/sap/chat/transformation.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/litellm/llms/sap/chat/transformation.py b/litellm/llms/sap/chat/transformation.py index 64d83f4db39..b7a5d61d3c3 100755 --- a/litellm/llms/sap/chat/transformation.py +++ b/litellm/llms/sap/chat/transformation.py @@ -201,7 +201,7 @@ class GenAIHubOrchestrationConfig(OpenAIGPTConfig): "scenarioId": "orchestration", "executableIds": [ "orchestration" - ], # mutable-ok: list literal required by httpx multi-value param serialization + ], "status": "RUNNING", }, ) From 5744a5fcc23de0f52f3079b7ab4dc8c04552eb0b Mon Sep 17 00:00:00 2001 From: Yamac Ay Date: Fri, 25 Sep 2026 14:30:03 +0200 Subject: [PATCH 7/7] lint: reformatted --- litellm/llms/sap/chat/transformation.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/litellm/llms/sap/chat/transformation.py b/litellm/llms/sap/chat/transformation.py index b7a5d61d3c3..3ca0249e7a8 100755 --- a/litellm/llms/sap/chat/transformation.py +++ b/litellm/llms/sap/chat/transformation.py @@ -199,9 +199,7 @@ class GenAIHubOrchestrationConfig(OpenAIGPTConfig): headers=self.headers, params={ # mutable-ok: one-shot query-params dict passed directly to httpx, not stored "scenarioId": "orchestration", - "executableIds": [ - "orchestration" - ], + "executableIds": ["orchestration"], "status": "RUNNING", }, )