From 9f54191e2da28906cb71ea20c2f2b6706f15d890 Mon Sep 17 00:00:00 2001 From: Yamac Ay Date: Thu, 25 Jun 2026 10:56:01 +0200 Subject: [PATCH] refactor(sap): replace deployment name filter with direct deployment_url passthrough Remove the SAP_ORCHESTRATION_DEPLOYMENT_NAME env var and per-call deployment_name parameter. Both invented mechanisms that have no analogue in the native gen-ai-hub-sdk, which selects deployments at instantiation time via constructor params, not per-call filters. The per-call path also bypassed @cached_property on every LLM call. Users who need a specific deployment pass deployment_url in litellm_params, which bypasses discovery entirely. The multi-deployment auto-discovery warning stays; it now directs users to the deployment_url param. --- litellm/llms/sap/chat/transformation.py | 28 ++------ .../llms/sap/chat/test_sap_transformation.py | 65 +++++++------------ 2 files changed, 29 insertions(+), 64 deletions(-) diff --git a/litellm/llms/sap/chat/transformation.py b/litellm/llms/sap/chat/transformation.py index 51cb734be34..47dbff1384f 100755 --- a/litellm/llms/sap/chat/transformation.py +++ b/litellm/llms/sap/chat/transformation.py @@ -2,7 +2,6 @@ Translate from OpenAI's `/v1/chat/completions` to SAP Generative AI Hub's Orchestration Service`v2/completion` """ -import os from functools import cached_property from typing import ( Any, @@ -179,9 +178,9 @@ class GenAIHubOrchestrationConfig(OpenAIGPTConfig): @cached_property def deployment_url(self) -> str: - return self._resolve_deployment_url(deployment_name=None) + return self._resolve_deployment_url() - def _resolve_deployment_url(self, deployment_name: Optional[str]) -> str: + def _resolve_deployment_url(self) -> str: client = litellm.module_level_client deployments = client.get( f"{self.base_url}/lm/deployments", headers=self.headers @@ -204,20 +203,6 @@ class GenAIHubOrchestrationConfig(OpenAIGPTConfig): message="No orchestration deployment found in SAP AI Core.", ) - name_filter = deployment_name or os.environ.get("SAP_ORCHESTRATION_DEPLOYMENT_NAME") - if name_filter: - filtered = [v for v in valid if v[2] == name_filter] - if not filtered: - available = [v[2] for v in valid] - raise GenAIHubOrchestrationError( - status_code=404, - message=( - f"No orchestration deployment named {name_filter!r} found. " - f"Available: {available}" - ), - ) - valid = filtered - if len(valid) > 1: sorted_valid = sorted(valid, key=lambda x: x[1], reverse=True) chosen = sorted_valid[0] @@ -226,8 +211,7 @@ class GenAIHubOrchestrationConfig(OpenAIGPTConfig): f"SAP: found {len(valid)} orchestration deployments; using the newest one " f"(name={chosen[2]!r}, url={chosen[0]!r}). " f"Others ignored: {others}. " - "Set SAP_ORCHESTRATION_DEPLOYMENT_NAME to select a specific one, " - "or use the deployment_name per-call parameter." + "Pass `deployment_url` in `litellm_params` to route to a specific deployment." ) return chosen[0] @@ -298,11 +282,7 @@ class GenAIHubOrchestrationConfig(OpenAIGPTConfig): litellm_params: dict, stream: Optional[bool] = None, ): - deployment_name = litellm_params.get("deployment_name") - if deployment_name: - base = self._resolve_deployment_url(deployment_name=deployment_name) - else: - base = self.deployment_url + base = litellm_params.get("deployment_url") or self.deployment_url return f"{base}/v2/completion" def _build_prompt_module( diff --git a/tests/test_litellm/llms/sap/chat/test_sap_transformation.py b/tests/test_litellm/llms/sap/chat/test_sap_transformation.py index d488a4071b1..eb1c8beebb5 100644 --- a/tests/test_litellm/llms/sap/chat/test_sap_transformation.py +++ b/tests/test_litellm/llms/sap/chat/test_sap_transformation.py @@ -656,7 +656,7 @@ def _mock_deployments(*names: str): """Return a mock httpx client whose GET responses simulate AI Core deployment/config APIs.""" deployments_payload = { "resources": [ - {"scenarioId": "orchestration", "configurationId": f"cfg-{n}", "deploymentUrl": f"https://deploy-{n}.sap.com", "createdAt": f"2024-01-0{i+1}T00:00:00Z"} + {"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) ] } @@ -683,54 +683,24 @@ class TestDeploymentSelection: def test_single_deployment_returns_its_url(self): config = _make_config() with patch("litellm.module_level_client", _mock_deployments("my-orch")): - url = config._resolve_deployment_url(deployment_name=None) + url = config._resolve_deployment_url() assert url == "https://deploy-my-orch.sap.com" def test_multiple_deployments_picks_newest(self): config = _make_config() with patch("litellm.module_level_client", _mock_deployments("older", "newer")): - url = config._resolve_deployment_url(deployment_name=None) + url = config._resolve_deployment_url() assert url == "https://deploy-newer.sap.com" def test_multiple_deployments_emits_warning(self): config = _make_config() with patch("litellm.module_level_client", _mock_deployments("older", "newer")): with patch("litellm.llms.sap.chat.transformation.verbose_logger") as mock_log: - config._resolve_deployment_url(deployment_name=None) + config._resolve_deployment_url() mock_log.warning.assert_called_once() msg = mock_log.warning.call_args[0][0] assert "2 orchestration deployments" in msg - assert "SAP_ORCHESTRATION_DEPLOYMENT_NAME" in msg - - def test_deployment_name_arg_selects_by_name(self): - config = _make_config() - with patch("litellm.module_level_client", _mock_deployments("grounding-orch", "plain-orch")): - url = config._resolve_deployment_url(deployment_name="grounding-orch") - assert url == "https://deploy-grounding-orch.sap.com" - - def test_env_var_selects_by_name(self, monkeypatch): - monkeypatch.setenv("SAP_ORCHESTRATION_DEPLOYMENT_NAME", "plain-orch") - config = _make_config() - with patch("litellm.module_level_client", _mock_deployments("grounding-orch", "plain-orch")): - url = config._resolve_deployment_url(deployment_name=None) - assert url == "https://deploy-plain-orch.sap.com" - - def test_deployment_name_arg_takes_priority_over_env_var(self, monkeypatch): - monkeypatch.setenv("SAP_ORCHESTRATION_DEPLOYMENT_NAME", "plain-orch") - config = _make_config() - with patch("litellm.module_level_client", _mock_deployments("grounding-orch", "plain-orch")): - url = config._resolve_deployment_url(deployment_name="grounding-orch") - assert url == "https://deploy-grounding-orch.sap.com" - - def test_unknown_name_raises_with_available_list(self): - from litellm.llms.sap.chat.handler import GenAIHubOrchestrationError - - config = _make_config() - with patch("litellm.module_level_client", _mock_deployments("alpha", "beta")): - with pytest.raises(GenAIHubOrchestrationError) as exc_info: - config._resolve_deployment_url(deployment_name="gamma") - assert "gamma" in str(exc_info.value) - assert "alpha" in str(exc_info.value) or "beta" in str(exc_info.value) + assert "deployment_url" in msg def test_no_deployments_raises(self): from litellm.llms.sap.chat.handler import GenAIHubOrchestrationError @@ -738,17 +708,32 @@ class TestDeploymentSelection: config = _make_config() with patch("litellm.module_level_client", _mock_deployments()): with pytest.raises(GenAIHubOrchestrationError) as exc_info: - config._resolve_deployment_url(deployment_name=None) + config._resolve_deployment_url() assert "No orchestration deployment found" in str(exc_info.value) - def test_get_complete_url_uses_deployment_name_from_litellm_params(self): + def test_get_complete_url_respects_deployment_url_in_litellm_params(self): config = _make_config() - with patch("litellm.module_level_client", _mock_deployments("grounding-orch", "plain-orch")): + explicit_url = "https://my-custom-deploy.sap.com/v2/inference/deployments/abc123" + mock_client = MagicMock() + with patch("litellm.module_level_client", mock_client): url = config.get_complete_url( api_base=None, api_key=None, model="gpt-4o", optional_params={}, - litellm_params={"deployment_name": "plain-orch"}, + litellm_params={"deployment_url": explicit_url}, ) - assert url == "https://deploy-plain-orch.sap.com/v2/completion" + assert url == f"{explicit_url}/v2/completion" + mock_client.get.assert_not_called() + + def test_get_complete_url_falls_back_to_discovery_when_no_deployment_url(self): + config = _make_config() + with patch("litellm.module_level_client", _mock_deployments("my-orch")): + url = config.get_complete_url( + api_base=None, + api_key=None, + model="gpt-4o", + optional_params={}, + litellm_params={}, + ) + assert url == "https://deploy-my-orch.sap.com/v2/completion"