-
Notifications
You must be signed in to change notification settings - Fork 3.1k
Support multiple open api def for toolset. #42728
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.
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
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR enhances the OpenApiTool functionality within the ToolSet to support multiple OpenAPI definitions. The main improvement allows adding multiple API definitions to a single OpenApiTool instance rather than requiring separate tools for each API specification.
Key Changes
- Modified the
add_definition
method to accept optional descriptions - Enhanced the
add
method to merge new OpenApiTool definitions into existing OpenApiTool instances - Added overloaded
remove
methods to support removing specific API definitions by name or entire OpenApiTool instances
API Change CheckAPIView identified API level changes in this PR and created the following API reviews |
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
…-sdk-for-python into howie/multi-tool-def
|
||
|
||
# Special handling for OpenApiTool - add definitions to existing tool instead of raising error | ||
if isinstance(tool, OpenApiTool): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure we should do this. Having specific type checks for certain tools in ToolSet does not feel optimal
self._tools.append(tool) | ||
|
||
def remove(self, tool_type: Type[Tool]) -> None: | ||
@overload | ||
def remove(self, tool_type: Type[OpenApiTool], *, name: str) -> None: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similar comment, I would not add tool specific overloads in Toolset. If Tool abstraction would contain add and remove then we would not need this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to discuss more if we can make this more generically
Description
Please add an informative description that covers that changes made by the pull request and link all relevant issues.
If an SDK is being regenerated based on a new API spec, a link to the pull request containing these API spec changes should be included above.
All SDK Contribution checklist:
General Guidelines and Best Practices
Testing Guidelines