Skip to content

Commit

Permalink
Improve support for handling multiple OpenAPI servers.
Browse files Browse the repository at this point in the history
  • Loading branch information
moonbox3 committed Nov 21, 2024
1 parent 01c1915 commit 5400c7f
Show file tree
Hide file tree
Showing 5 changed files with 149 additions and 27 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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
)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand All @@ -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
Expand Down Expand Up @@ -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):
Expand Down Expand Up @@ -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."""
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 []
),
}
Expand Down
17 changes: 15 additions & 2 deletions python/semantic_kernel/connectors/openapi_plugin/openapi_parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -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():
Expand Down
89 changes: 83 additions & 6 deletions python/tests/unit/connectors/openapi_plugin/test_sk_openapi.py
Original file line number Diff line number Diff line change
Expand Up @@ -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():
Expand Down

0 comments on commit 5400c7f

Please sign in to comment.