diff --git a/python/semantic_kernel/connectors/ai/azure_ai_inference/services/azure_ai_inference_tracing.py b/python/semantic_kernel/connectors/ai/azure_ai_inference/services/azure_ai_inference_tracing.py index 03d0c6c4f53a..e326ed1d9b6b 100644 --- a/python/semantic_kernel/connectors/ai/azure_ai_inference/services/azure_ai_inference_tracing.py +++ b/python/semantic_kernel/connectors/ai/azure_ai_inference/services/azure_ai_inference_tracing.py @@ -35,7 +35,7 @@ def __enter__(self) -> None: self.diagnostics_settings.enable_otel_diagnostics or self.diagnostics_settings.enable_otel_diagnostics_sensitive ): - AIInferenceInstrumentor().instrument( + AIInferenceInstrumentor().instrument( # type: ignore enable_content_recording=self.diagnostics_settings.enable_otel_diagnostics_sensitive ) diff --git a/python/semantic_kernel/connectors/openapi_plugin/models/rest_api_operation.py b/python/semantic_kernel/connectors/openapi_plugin/models/rest_api_operation.py index 60a3f24ca924..f6150e70a0a7 100644 --- a/python/semantic_kernel/connectors/openapi_plugin/models/rest_api_operation.py +++ b/python/semantic_kernel/connectors/openapi_plugin/models/rest_api_operation.py @@ -2,7 +2,7 @@ import re from typing import Any, Final -from urllib.parse import ParseResult, urlencode, urljoin, urlparse, urlunparse +from urllib.parse import ParseResult, ParseResultBytes, urlencode, urljoin, urlparse, urlunparse from semantic_kernel.connectors.openapi_plugin.models.rest_api_expected_response import ( RestApiExpectedResponse, @@ -52,7 +52,7 @@ def __init__( self, id: str, method: str, - servers: list[str | ParseResult], + servers: list[dict[str, Any]], path: str, summary: str | None = None, description: str | None = None, @@ -64,7 +64,7 @@ def __init__( """Initialize the RestApiOperation.""" self._id = id self._method = method.upper() - self._servers = [urlparse(s) if isinstance(s, str) else s for s in servers] + self._servers = servers self._path = path self._summary = summary self._description = description @@ -117,9 +117,9 @@ def servers(self): return self._servers @servers.setter - def servers(self, value: list[str | ParseResult]): + def servers(self, value: list[dict[str, Any]]): self._throw_if_frozen() - self._servers = [urlparse(s) if isinstance(s, str) else s for s in value] + self._servers = value @property def path(self): @@ -223,26 +223,58 @@ def build_operation_url(self, arguments, server_url_override=None, api_host_url= """Build the URL for the operation.""" server_url = self.get_server_url(server_url_override, api_host_url) path = self.build_path(self.path, arguments) - return urljoin(server_url.geturl(), path.lstrip("/")) + try: + return urljoin(server_url, path.lstrip("/")) + except Exception as e: + raise FunctionExecutionException(f"Error building the URL for the operation {self.id}: {e!s}") from e - def get_server_url(self, server_url_override=None, api_host_url=None): + def get_server_url(self, server_url_override=None, api_host_url=None, arguments=None): """Get the server URL for the operation.""" - if server_url_override is not None and server_url_override.geturl() != "": - server_url = server_url_override - elif self.servers and self.servers[0].geturl() != "": - server_url = self.servers[0] + if arguments is None: + arguments = {} + + # Prioritize server_url_override + if ( + server_url_override is not None + and isinstance(server_url_override, (ParseResult, ParseResultBytes)) + and server_url_override.geturl() != b"" + ): + server_url_string = server_url_override.geturl() + elif server_url_override is not None and isinstance(server_url_override, str) and server_url_override != "": + server_url_string = server_url_override + elif self.servers and len(self.servers) > 0: + # Use the first server by default + server = self.servers[0] + server_url_string = server["url"] if isinstance(server, dict) else server + server_variables = server.get("variables", {}) if isinstance(server, dict) else {} + + # Substitute server variables if available + for variable_name, variable_def in server_variables.items(): + argument_name = variable_def.get("argument_name", variable_name) + if argument_name in arguments: + value = arguments[argument_name] + server_url_string = server_url_string.replace(f"{{{variable_name}}}", value) + elif "default" in variable_def and variable_def["default"] is not None: + # Use the default value if no argument is provided + value = variable_def["default"] + server_url_string = server_url_string.replace(f"{{{variable_name}}}", value) + else: + # Raise an exception if no value is available + raise FunctionExecutionException( + f"No argument provided for the '{variable_name}' server variable of the operation '{self.id}'." + ) + elif self.server_url: + server_url_string = self.server_url elif api_host_url is not None: - server_url = api_host_url + server_url_string = api_host_url else: raise FunctionExecutionException(f"No valid server URL for operation {self.id}") - server_url_string = server_url.geturl() - - # make sure the base URL ends with a trailing slash + # Ensure the base URL ends with a trailing slash if not server_url_string.endswith("/"): server_url_string += "/" - return urlparse(server_url_string) + return server_url_string # Return the URL string directly def build_path(self, path_template: str, arguments: dict[str, Any]) -> str: """Build the path for the operation.""" diff --git a/python/semantic_kernel/connectors/openapi_plugin/openapi_manager.py b/python/semantic_kernel/connectors/openapi_plugin/openapi_manager.py index 1acc49372597..c5823c7d559f 100644 --- a/python/semantic_kernel/connectors/openapi_plugin/openapi_manager.py +++ b/python/semantic_kernel/connectors/openapi_plugin/openapi_manager.py @@ -183,8 +183,8 @@ async def run_openapi_operation( OperationExtensions.METHOD_KEY.value: operation.method.upper(), OperationExtensions.OPERATION_KEY.value: operation, OperationExtensions.SERVER_URLS_KEY.value: ( - [operation.servers[0].geturl()] - if operation.servers and len(operation.servers) > 0 and operation.servers[0].geturl() + [operation.servers[0]["url"]] + if operation.servers and len(operation.servers) > 0 and operation.servers[0]["url"] else [] ), } diff --git a/python/semantic_kernel/connectors/openapi_plugin/openapi_parser.py b/python/semantic_kernel/connectors/openapi_plugin/openapi_parser.py index 583681fc3807..a08276de387d 100644 --- a/python/semantic_kernel/connectors/openapi_plugin/openapi_parser.py +++ b/python/semantic_kernel/connectors/openapi_plugin/openapi_parser.py @@ -217,10 +217,23 @@ def create_rest_api_operations( request_objects = {} servers = parsed_document.get("servers", []) - server_urls = [server.get("url") for server in servers] if servers else ["/"] if execution_settings and execution_settings.server_url_override: - server_urls = [execution_settings.server_url_override] + # Override the servers with the provided URL + server_urls = [{"url": execution_settings.server_url_override, "variables": {}}] + elif servers: + # Process servers, ensuring we capture their variables + server_urls = [] + for server in servers: + server_entry = { + "url": server.get("url", "/"), + "variables": server.get("variables", {}), + "description": server.get("description", ""), + } + server_urls.append(server_entry) + else: + # Default server if none specified + server_urls = [{"url": "/", "variables": {}, "description": ""}] for path, methods in paths.items(): for method, details in methods.items(): diff --git a/python/tests/unit/connectors/openapi_plugin/test_sk_openapi.py b/python/tests/unit/connectors/openapi_plugin/test_sk_openapi.py index 4cfd6d03fd4a..4dbab11ad34a 100644 --- a/python/tests/unit/connectors/openapi_plugin/test_sk_openapi.py +++ b/python/tests/unit/connectors/openapi_plugin/test_sk_openapi.py @@ -279,24 +279,101 @@ def test_build_operation_url_with_override(): def test_build_operation_url_without_override(): parameters = [RestApiParameter(name="id", type="string", location=RestApiParameterLocation.PATH, is_required=True)] operation = RestApiOperation( - id="test", method="GET", servers=["https://example.com/"], path="/resource/{id}", params=parameters + id="test", + method="GET", + servers=[{"url": "https://example.com/"}], + path="/resource/{id}", + params=parameters, ) arguments = {"id": "123"} expected_url = "https://example.com/resource/123" assert operation.build_operation_url(arguments) == expected_url -def test_get_server_url_with_override(): - operation = RestApiOperation(id="test", method="GET", servers=["https://example.com"], path="/resource/{id}") +def test_get_server_url_with_parse_result_override(): + operation = RestApiOperation( + id="test", + method="GET", + servers=[{"url": "https://example.com"}], + path="/resource/{id}", + ) server_url_override = urlparse("https://override.com") expected_url = "https://override.com/" - assert operation.get_server_url(server_url_override=server_url_override).geturl() == expected_url + assert operation.get_server_url(server_url_override=server_url_override) == expected_url + + +def test_get_server_url_with_string_override(): + operation = RestApiOperation( + id="test", + method="GET", + servers=[{"url": "https://example.com"}], + path="/resource/{id}", + ) + server_url_override = "https://override.com" + expected_url = "https://override.com/" + assert operation.get_server_url(server_url_override=server_url_override) == expected_url + + +def test_get_server_url_with_servers_no_variables(): + operation = RestApiOperation( + id="test", + method="GET", + servers=[{"url": "https://example.com"}], + path="/resource/{id}", + ) + expected_url = "https://example.com/" + assert operation.get_server_url() == expected_url + + +def test_get_server_url_with_servers_and_variables(): + operation = RestApiOperation( + id="test", + method="GET", + servers=[ + { + "url": "https://example.com/{version}", + "variables": {"version": {"default": "v1", "argument_name": "api_version"}}, + } + ], + path="/resource/{id}", + ) + arguments = {"api_version": "v2"} + expected_url = "https://example.com/v2/" + assert operation.get_server_url(arguments=arguments) == expected_url + + +def test_get_server_url_with_servers_and_default_variable(): + operation = RestApiOperation( + id="test", + method="GET", + servers=[{"url": "https://example.com/{version}", "variables": {"version": {"default": "v1"}}}], + path="/resource/{id}", + ) + expected_url = "https://example.com/v1/" + assert operation.get_server_url() == expected_url + + +def test_get_server_url_with_override(): + operation = RestApiOperation( + id="test", + method="GET", + servers=[{"url": "https://example.com"}], + path="/resource/{id}", + ) + server_url_override = "https://override.com" + expected_url = "https://override.com/" + assert operation.get_server_url(server_url_override=server_url_override) == expected_url def test_get_server_url_without_override(): - operation = RestApiOperation(id="test", method="GET", servers=["https://example.com"], path="/resource/{id}") + operation = RestApiOperation( + id="test", + method="GET", + servers=[{"url": "https://example.com"}], + path="/resource/{id}", + ) expected_url = "https://example.com/" - assert operation.get_server_url().geturl() == expected_url + assert operation.get_server_url() == expected_url def test_build_path_with_required_parameter():