Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Python: OpenAPI plugins don't correctly create kernel functions #6261

Closed
matthewbolanos opened this issue May 15, 2024 · 1 comment · Fixed by #6279
Closed

Python: OpenAPI plugins don't correctly create kernel functions #6261

matthewbolanos opened this issue May 15, 2024 · 1 comment · Fixed by #6279
Labels
python Pull requests for the Python Semantic Kernel triage

Comments

@matthewbolanos
Copy link
Member

When importing an OpenAPI schema, the kernel functions that are created just have the following inputs:

@kernel_function(
        description=operation.summary if operation.summary else operation.description,
        name=operation.id,
    )
    async def run_openapi_operation(
        path_params: Annotated[dict | str | None, "A dictionary of path parameters"] = None,
        query_params: Annotated[dict | str | None, "A dictionary of query parameters"] = None,
        headers: Annotated[dict | str | None, "A dictionary of headers"] = None,
        request_body: Annotated[dict | str | None, "A dictionary of the request body"] = None,
    ) -> str:
  • path_params
  • query_params
  • headers
  • request_body

Because of this, we aren't describing to the model the actual parameters that are required by the operation.

For example, the following parameters are not described to the LLM as part of the tool call in my sample's OpenAPI spec:

"ChangeSpeakerStateRequest": {
    "type": "object",
    "properties": {
        "isOn": {
            "type": "boolean",
            "description": "Specifies whether the light is turned on or off."
        },
        "hexColor": {
            "type": "string",
            "description": "The hex color code for the light.",
            "nullable": true
        },
        "brightness": {
            "enum": [
                "Low",
                "Medium",
                "High"
            ],
            "type": "string",
            "description": "The brightness level of the light."
        },
        "fadeDurationInMilliseconds": {
            "type": "integer",
            "description": "Duration for the light to fade to the new state, in milliseconds.",
            "format": "int32"
        },
        "scheduledTime": {
            "type": "integer",
            "description": "The time at which the change should occur.",
            "format": "int32"
        }
    },
    "additionalProperties": false,
    "description": "Represents a request to change the state of the light."
}
@markwallace-microsoft markwallace-microsoft added python Pull requests for the Python Semantic Kernel triage labels May 15, 2024
@matthewbolanos
Copy link
Member Author

In case it helps, here's the Java implementation: https://github.com/microsoft/semantic-kernel/blob/java-development/java/samples/semantickernel-sample-plugins/semantickernel-openapi-plugin/src/main/java/com/microsoft/semantickernel/samples/openapi/OpenAPIHttpRequestPlugin.java

@moonbox3 moonbox3 mentioned this issue May 15, 2024
Merged
4 tasks
github-merge-queue bot pushed a commit that referenced this issue May 16, 2024
@moonbox3
Python: OpenAPI plugin enhance (#6279)
142aef8 
### Motivation and Context

Python's OpenAPI manager `run_openapi_operation` was hard-coded to use
certain parameters that would ultimately be built up to make an API
request. This didn't allow the model to know which parameters were
actually defined as required in the OpenAPI spec and would cause errors
during function calling.

<!-- Thank you for your contribution to the semantic-kernel repo!
Please help reviewers and future users, providing the following
information:
  1. Why is this change required?
  2. What problem does it solve?
  3. What scenario does it contribute to?
  4. If it fixes an open issue, please link to the issue here.
-->

### Description

This PR does a quite major overhaul on the openapi manager, with the
caveat that the code is going to be refactored / cleaned up in a next
iteration (we're pressed for time right now). In the
`_create_function_from_operation` method, the `rest_operation_params`
are now built up from the operation which means we included the required
parameters and will have them during function calling. This allows us to
properly build up the url, headers, request body and paths to make the
API call.
- The concept samples were updated and are functioning with this latest
code.
- function calling was tested with the AzureKeyVault OpenAPI example and
the model was able to automatically create a secret in a test key vault.
- Old unit tests were removed. Note: in the next iteration new unit
tests for all of the new functionality will be added.
- In the next iteration, the entire `openapi_manager.py` file will be
broken apart to separate files for classes/moels to clean it up.
- Closes #6261 

<!-- Describe your changes, the overall approach, the underlying design.
These notes will help understanding how your code works. Thanks! -->

### Contribution Checklist

<!-- Before submitting this PR, please make sure: -->

- [X] The code builds clean without any errors or warnings
- [X] The PR follows the [SK Contribution
Guidelines](https://github.com/microsoft/semantic-kernel/blob/main/CONTRIBUTING.md)
and the [pre-submission formatting
script](https://github.com/microsoft/semantic-kernel/blob/main/CONTRIBUTING.md#development-scripts)
raises no violations
- [X] All unit tests pass, and I have added new tests where possible
- [ ] I didn't break anyone 😄
bochris pushed a commit to bochris/semantic-kernel that referenced this issue May 16, 2024
@moonbox3 @bochris
Python: OpenAPI plugin enhance (microsoft#6279)
45f8809 
### Motivation and Context

Python's OpenAPI manager `run_openapi_operation` was hard-coded to use
certain parameters that would ultimately be built up to make an API
request. This didn't allow the model to know which parameters were
actually defined as required in the OpenAPI spec and would cause errors
during function calling.

<!-- Thank you for your contribution to the semantic-kernel repo!
Please help reviewers and future users, providing the following
information:
  1. Why is this change required?
  2. What problem does it solve?
  3. What scenario does it contribute to?
  4. If it fixes an open issue, please link to the issue here.
-->

### Description

This PR does a quite major overhaul on the openapi manager, with the
caveat that the code is going to be refactored / cleaned up in a next
iteration (we're pressed for time right now). In the
`_create_function_from_operation` method, the `rest_operation_params`
are now built up from the operation which means we included the required
parameters and will have them during function calling. This allows us to
properly build up the url, headers, request body and paths to make the
API call.
- The concept samples were updated and are functioning with this latest
code.
- function calling was tested with the AzureKeyVault OpenAPI example and
the model was able to automatically create a secret in a test key vault.
- Old unit tests were removed. Note: in the next iteration new unit
tests for all of the new functionality will be added.
- In the next iteration, the entire `openapi_manager.py` file will be
broken apart to separate files for classes/moels to clean it up.
- Closes microsoft#6261 

<!-- Describe your changes, the overall approach, the underlying design.
These notes will help understanding how your code works. Thanks! -->

### Contribution Checklist

<!-- Before submitting this PR, please make sure: -->

- [X] The code builds clean without any errors or warnings
- [X] The PR follows the [SK Contribution
Guidelines](https://github.com/microsoft/semantic-kernel/blob/main/CONTRIBUTING.md)
and the [pre-submission formatting
script](https://github.com/microsoft/semantic-kernel/blob/main/CONTRIBUTING.md#development-scripts)
raises no violations
- [X] All unit tests pass, and I have added new tests where possible
- [ ] I didn't break anyone 😄
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
python Pull requests for the Python Semantic Kernel triage
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Python: OpenAPI plugin enhance moonbox3/semantic-kernel
2 participants
@matthewbolanos @markwallace-microsoft