Merge e3d8718fa3 into 527d882508

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
 import types
 from collections.abc import AsyncIterator, Iterator, Mapping
 from functools import cached_property
 from typing import TYPE_CHECKING, Any, Final, Union
 
 import httpx
-from pydantic import BaseModel
+from pydantic import BaseModel, ConfigDict, Field
 
 import litellm
 from litellm.litellm_core_utils.prompt_templates.common_utils import message_with_file_parts_as_image_urls
@@ -57,6 +58,21 @@ _SAP_MODEL_PARAMS_EXCLUDED_KEYS: Final[frozenset[str]] = frozenset(
 )
 
 
+class _DeploymentUrlOverride(BaseModel):
+    """The deployment_url a caller can pass in optional_params to skip discovery"""
+
+    model_config = ConfigDict(frozen=True, extra="ignore")
+    deployment_url: str | None = None
+
+
+class _OrchestrationConfiguration(BaseModel):
+    """The fields discovery reads from an AI Core /lm/configurations/{id} response"""
+
+    model_config = ConfigDict(frozen=True, extra="ignore")
+    executable_id: str = Field(default="", alias="executableId")
+    name: str = ""
+
+
 def validate_dict(
     data: Mapping[str, object], model: type[BaseModel]
 ) -> dict:  # mutable-ok: pydantic validation boundary; both input and output are untyped wire dicts
@@ -187,20 +203,40 @@ class GenAIHubOrchestrationConfig(OpenAIGPTConfig):
         # 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[AICoreDeploymentList] = client.get(
-            join_api_base(self.base_url, "lm/deployments"), headers=self.headers
-        ).json()
-        valid: Final[list[tuple[str, str]]] = []
+        deployments: Final = client.get(join_api_base(self.base_url, "lm/deployments"), headers=self.headers).json()
+        valid: Final[list[tuple[str, str, str]]] = []
         for dep in deployments.get("resources", []):
             if dep.get("scenarioId") == "orchestration":
-                cfg: AICoreConfiguration = client.get(
-                    join_api_base(self.base_url, f"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]
+                cfg = _OrchestrationConfiguration.model_validate(
+                    client.get(
+                        join_api_base(self.base_url, f"lm/configurations/{dep['configurationId']}"),
+                        headers=self.headers,
+                    ).json()
+                )
+                if cfg.executable_id == "orchestration":
+                    valid.append((dep["deploymentUrl"], dep["createdAt"], cfg.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):
@@ -267,8 +303,11 @@ class GenAIHubOrchestrationConfig(OpenAIGPTConfig):
         litellm_params: dict,
         stream: bool | None = None,
     ):
-        api_base_: Final = join_api_base(self.deployment_url, "v2/completion")
-        return api_base_
+        explicit_url: Final = _DeploymentUrlOverride.model_validate(optional_params).deployment_url
+        deployment_url: Final = (
+            explicit_url or os.environ.get("AICORE_ORCHESTRATION_DEPLOYMENT_URL") or self.deployment_url
+        )
+        return join_api_base(deployment_url, "v2/completion")
 
     def _build_prompt_module(
         self,

tests/unit/llms/sap/chat/test_sap_transformation.py

@@ -1,9 +1,16 @@
+from __future__ import annotations
+
 import warnings
-from typing import Final
+from collections.abc import Mapping
+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."""
@@ -644,6 +651,118 @@ 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) -> GenAIHubOrchestrationConfig:
+        from litellm.llms.sap.chat.transformation import GenAIHubOrchestrationConfig
+
+        config: Final = 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: Final[tuple[Mapping[str, str], ...]] = tuple(
+            {
+                "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: Final[Mapping[str, Mapping[str, str]]] = {
+            f"cfg-{n}": {"executableId": "orchestration", "name": n} for n in names
+        }
+
+        def fake_get(url: str, headers: Mapping[str, str] | None = None) -> MagicMock:
+            resp: Final = MagicMock()
+            if "/lm/deployments" in url:
+                resp.json.return_value = {"resources": resources}
+            else:
+                cfg_id: Final = url.split("/")[-1]
+                resp.json.return_value = configs.get(cfg_id, {})
+            return resp
+
+        client: Final = 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: Final = MagicMock()
+        monkeypatch.setattr("litellm.module_level_client", mock_client)
+
+        url: Final = 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: Final = 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: Final = MagicMock()
+        monkeypatch.setattr("litellm.module_level_client", mock_client)
+        monkeypatch.setenv("AICORE_ORCHESTRATION_DEPLOYMENT_URL", env_url)
+
+        url: Final = 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: Final = 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
+        )
+
+
 class TestCacheControl:
     """Unit tests for cache_control preservation on message content parts."""