Merge f8597b3453 into 635085ac14

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, 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),
+                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.rstrip('/')}/v2/completion"
 
     def _build_prompt_module(
         self,

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."""
@@ -639,3 +647,117 @@ 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) -> GenAIHubOrchestrationConfig:
+        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) -> MagicMock:
+        resources: list[dict[str, str]] = [
+            {
+                "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: dict[str, dict[str, str]] = {
+            f"cfg-{n}": {"executableId": "orchestration", "name": n} for n in names
+        }
+
+        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}
+            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: 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)
+
+        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_optional_param_strips_trailing_slash(
+        self, mock_config: GenAIHubOrchestrationConfig, monkeypatch: pytest.MonkeyPatch
+    ) -> None:
+        monkeypatch.setattr("litellm.module_level_client", MagicMock())
+
+        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)
+
+        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: GenAIHubOrchestrationConfig, monkeypatch: pytest.MonkeyPatch
+    ) -> None:
+        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: 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", "")
+
+        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()
+            and "deploy-older.sap.com" in record.getMessage()
+            for record in caplog.records
+        )