Python: Turn on mypy for OpenAPI plugins. Increase code coverage.

python/mypy.ini

@@ -29,10 +29,6 @@ ignore_errors = true
 ignore_errors = true
 # TODO (eavanvalkenburg): remove this: https://github.com/microsoft/semantic-kernel/issues/7134
 
-[mypy-semantic_kernel.connectors.openapi_plugin.*]
-ignore_errors = true
-# TODO (eavanvalkenburg): remove this: https://github.com/microsoft/semantic-kernel/issues/7135
-
 [mypy-semantic_kernel.connectors.utils.*]
 ignore_errors = true
 # TODO (eavanvalkenburg): remove this: https://github.com/microsoft/semantic-kernel/issues/7136

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 urlencode, urljoin, urlparse, urlunparse
+from urllib.parse import ParseResult, urlencode, urljoin, urlparse, urlunparse
 
 from semantic_kernel.connectors.openapi_plugin.models.rest_api_operation_expected_response import (
     RestApiOperationExpectedResponse,
@@ -49,7 +49,7 @@ def __init__(
         self,
         id: str,
         method: str,
-        server_url: str,
+        server_url: str | ParseResult,
         path: str,
         summary: str | None = None,
         description: str | None = None,
@@ -60,11 +60,11 @@ def __init__(
         """Initialize the RestApiOperation."""
         self.id = id
         self.method = method.upper()
-        self.server_url = server_url
+        self.server_url = urlparse(server_url) if isinstance(server_url, str) else server_url
         self.path = path
         self.summary = summary
         self.description = description
-        self.parameters = params
+        self.parameters = params if params else []
         self.request_body = request_body
         self.responses = responses
 
@@ -163,7 +163,7 @@ def get_parameters(
         enable_payload_spacing: bool = False,
     ) -> list["RestApiOperationParameter"]:
         """Get the parameters for the operation."""
-        params = list(operation.parameters)
+        params = list(operation.parameters) if operation.parameters is not None else []
         if operation.request_body is not None:
             params.extend(
                 self.get_payload_parameters(
@@ -221,8 +221,8 @@ def _get_parameters_from_payload_metadata(
     ) -> list["RestApiOperationParameter"]:
         parameters: list[RestApiOperationParameter] = []
         for property in properties:
-            parameter_name = self._get_property_name(property, root_property_name, enable_namespacing)
-            if not property.properties:
+            parameter_name = self._get_property_name(property, root_property_name or False, enable_namespacing)
+            if not hasattr(property, "properties") or not property.properties:
                 parameters.append(
                     RestApiOperationParameter(
                         name=parameter_name,
@@ -234,9 +234,16 @@ def _get_parameters_from_payload_metadata(
                         schema=property.schema,
                     )
                 )
-            parameters.extend(
-                self._get_parameters_from_payload_metadata(property.properties, enable_namespacing, parameter_name)
-            )
+            else:
+                # Handle property.properties as a single instance or a list
+                if isinstance(property.properties, RestApiOperationPayloadProperty):
+                    nested_properties = [property.properties]
+                else:
+                    nested_properties = property.properties
+
+                parameters.extend(
+                    self._get_parameters_from_payload_metadata(nested_properties, enable_namespacing, parameter_name)
+                )
         return parameters
 
     def get_payload_parameters(
@@ -246,7 +253,7 @@ def get_payload_parameters(
         if use_parameters_from_metadata:
             if operation.request_body is None:
                 raise Exception(
-                    f"Payload parameters cannot be retrieved from the `{operation.Id}` "
+                    f"Payload parameters cannot be retrieved from the `{operation.id}` "
                     f"operation payload metadata because it is missing."
                 )
             if operation.request_body.media_type == RestApiOperation.MEDIA_TYPE_TEXT_PLAIN:
@@ -256,7 +263,7 @@ def get_payload_parameters(
 
         return [
             self.create_payload_artificial_parameter(operation),
-            self.create_content_type_artificial_parameter(operation),
+            self.create_content_type_artificial_parameter(),
         ]
 
     def get_default_response(
@@ -276,14 +283,25 @@ def get_default_return_parameter(self, preferred_responses: list[str] | None = N
         if preferred_responses is None:
             preferred_responses = self._preferred_responses
 
-        rest_operation_response = self.get_default_response(self.responses, preferred_responses)
+        responses = self.responses if self.responses is not None else {}
+
+        rest_operation_response = self.get_default_response(responses, preferred_responses)
+
+        schema_type = None
+        if rest_operation_response is not None and rest_operation_response.schema is not None:
+            schema_type = rest_operation_response.schema.get("type")
 
         if rest_operation_response:
             return KernelParameterMetadata(
                 name="return",
                 description=rest_operation_response.description,
-                type_=rest_operation_response.schema.get("type") if rest_operation_response.schema else None,
+                type_=schema_type,
                 schema_data=rest_operation_response.schema,
             )
 
-        return None
+        return KernelParameterMetadata(
+            name="return",
+            description="Default return parameter",
+            type_="string",
+            schema_data={"type": "string"},
+        )

python/semantic_kernel/connectors/openapi_plugin/models/rest_api_operation_expected_response.py

@@ -6,7 +6,7 @@
 
 @experimental_class
 class RestApiOperationExpectedResponse:
-    def __init__(self, description: str, media_type: str, schema: str | None = None):
+    def __init__(self, description: str, media_type: str, schema: dict[str, str] | None = None):
         """Initialize the RestApiOperationExpectedResponse."""
         self.description = description
         self.media_type = media_type

python/semantic_kernel/connectors/openapi_plugin/models/rest_api_operation_run_options.py

@@ -7,7 +7,7 @@
 class RestApiOperationRunOptions:
     """The options for running the REST API operation."""
 
-    def __init__(self, server_url_override=None, api_host_url=None):
+    def __init__(self, server_url_override=None, api_host_url=None) -> None:
         """Initialize the REST API operation run options."""
         self.server_url_override: str = server_url_override
         self.api_host_url: str = api_host_url

python/semantic_kernel/connectors/openapi_plugin/openapi_manager.py

@@ -46,12 +46,14 @@ def create_functions_from_openapi(
         list[KernelFunctionFromMethod]: the operations as functions
     """
     parser = OpenApiParser()
-    parsed_doc = parser.parse(openapi_document_path)
+    if (parsed_doc := parser.parse(openapi_document_path)) is None:
+        raise FunctionExecutionException(f"Error parsing OpenAPI document: {openapi_document_path}")
     operations = parser.create_rest_api_operations(parsed_doc, execution_settings=execution_settings)
 
     auth_callback = None
     if execution_settings and execution_settings.auth_callback:
         auth_callback = execution_settings.auth_callback
+
     openapi_runner = OpenApiRunner(
         parsed_openapi_document=parsed_doc,
         auth_callback=auth_callback,
@@ -129,11 +131,13 @@ async def run_openapi_operation(
             description=f"{p.description or p.name}",
             default_value=p.default_value or "",
             is_required=p.is_required,
-            type_=p.type if p.type is not None else TYPE_MAPPING.get(p.type, None),
+            type_=p.type if p.type is not None else TYPE_MAPPING.get(p.type, "object"),
             schema_data=(
                 p.schema
                 if p.schema is not None and isinstance(p.schema, dict)
-                else {"type": f"{p.type}"} if p.type else None
+                else {"type": f"{p.type}"}
+                if p.type
+                else None
             ),
         )
         for p in rest_operation_params

python/semantic_kernel/connectors/openapi_plugin/openapi_parser.py

@@ -118,13 +118,19 @@ def _get_payload_properties(self, operation_id, schema, required_properties, lev
 
     def _create_rest_api_operation_payload(
         self, operation_id: str, request_body: dict[str, Any]
-    ) -> RestApiOperationPayload:
+    ) -> RestApiOperationPayload | None:
         if request_body is None or request_body.get("content") is None:
             return None
-        media_type = next((mt for mt in OpenApiParser.SUPPORTED_MEDIA_TYPES if mt in request_body.get("content")), None)
+
+        content = request_body.get("content")
+        if content is None:
+            return None
+
+        media_type = next((mt for mt in OpenApiParser.SUPPORTED_MEDIA_TYPES if mt in content), None)
         if media_type is None:
             raise Exception(f"Neither of the media types of {operation_id} is supported.")
-        media_type_metadata = request_body.get("content")[media_type]
+
+        media_type_metadata = content[media_type]
         payload_properties = self._get_payload_properties(
             operation_id, media_type_metadata["schema"], media_type_metadata["schema"].get("required", set())
         )

python/semantic_kernel/connectors/openapi_plugin/openapi_runner.py

@@ -3,7 +3,8 @@
 import json
 import logging
 from collections import OrderedDict
-from collections.abc import Callable, Mapping
+from collections.abc import Awaitable, Callable, Mapping
+from inspect import isawaitable
 from typing import Any
 from urllib.parse import urlparse, urlunparse
 
@@ -34,13 +35,13 @@ class OpenApiRunner:
     def __init__(
         self,
         parsed_openapi_document: Mapping[str, str],
-        auth_callback: Callable[[dict[str, str]], dict[str, str]] | None = None,
+        auth_callback: Callable[..., dict[str, str] | Awaitable[dict[str, str]]] | None = None,
         http_client: httpx.AsyncClient | None = None,
         enable_dynamic_payload: bool = True,
         enable_payload_namespacing: bool = False,
     ):
         """Initialize the OpenApiRunner."""
-        self.spec = Spec.from_dict(parsed_openapi_document)
+        self.spec = Spec.from_dict(parsed_openapi_document)  # type: ignore
         self.auth_callback = auth_callback
         self.http_client = http_client
         self.enable_dynamic_payload = enable_dynamic_payload
@@ -99,19 +100,27 @@ def build_json_object(self, properties, arguments, property_namespace=None):
                 )
         return result
 
-    def build_operation_payload(self, operation: RestApiOperation, arguments: KernelArguments) -> tuple[str, str]:
+    def build_operation_payload(
+        self, operation: RestApiOperation, arguments: KernelArguments
+    ) -> tuple[str, str] | tuple[None, None]:
         """Build the operation payload."""
         if operation.request_body is None and self.payload_argument_name not in arguments:
             return None, None
-        return self.build_json_payload(operation.request_body, arguments)
+
+        if operation.request_body is not None:
+            return self.build_json_payload(operation.request_body, arguments)
+
+        return None, None
 
     def get_argument_name_for_payload(self, property_name, property_namespace=None):
         """Get argument name for the payload."""
         if not self.enable_payload_namespacing:
             return property_name
         return f"{property_namespace}.{property_name}" if property_namespace else property_name
 
-    def _get_first_response_media_type(self, responses: OrderedDict[str, RestApiOperationExpectedResponse]) -> str:
+    def _get_first_response_media_type(
+        self, responses: OrderedDict[str, RestApiOperationExpectedResponse] | None
+    ) -> str:
         if responses:
             first_response = next(iter(responses.values()))
             return first_response.media_type if first_response.media_type else self.media_type_application_json
@@ -123,30 +132,36 @@ async def run_operation(
         arguments: KernelArguments | None = None,
         options: RestApiOperationRunOptions | None = None,
     ) -> str:
-        """Run the operation."""
+        """Runs the operation defined in the OpenAPI manifest."""
+        if not arguments:
+            arguments = KernelArguments()
         url = self.build_operation_url(
             operation=operation,
             arguments=arguments,
-            server_url_override=options.server_url_override,
-            api_host_url=options.api_host_url,
+            server_url_override=options.server_url_override if options else None,
+            api_host_url=options.api_host_url if options else None,
         )
         headers = operation.build_headers(arguments=arguments)
         payload, _ = self.build_operation_payload(operation=operation, arguments=arguments)
 
-        """Runs the operation defined in the OpenAPI manifest"""
-        if headers is None:
-            headers = {}
-
         if self.auth_callback:
-            headers_update = await self.auth_callback(headers=headers)
-            headers.update(headers_update)
+            headers_update = self.auth_callback(**headers)
+            if isawaitable(headers_update):
+                headers_update = await headers_update
+            # at this point, headers_update is a valid dictionary
+            headers.update(headers_update)  # type: ignore
 
         if APP_INFO:
             headers.update(APP_INFO)
             headers = prepend_semantic_kernel_to_user_agent(headers)
 
         if "Content-Type" not in headers:
-            headers["Content-Type"] = self._get_first_response_media_type(operation.responses)
+            responses = (
+                operation.responses
+                if isinstance(operation.responses, OrderedDict)
+                else OrderedDict(operation.responses or {})
+            )
+            headers["Content-Type"] = self._get_first_response_media_type(responses)
 
         async def fetch():
             async def make_request(client: httpx.AsyncClient):