-
Notifications
You must be signed in to change notification settings - Fork 2.5k
Add Tool/Toolset warm_up #9849
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
Add Tool/Toolset warm_up #9849
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -185,6 +185,12 @@ def __contains__(self, item: Any) -> bool: | |
| return item in self.tools | ||
| return False | ||
|
|
||
| def warm_up(self) -> None: | ||
| """ | ||
| Warm up the Toolset. | ||
| """ | ||
| pass | ||
|
|
||
| def add(self, tool: Union[Tool, "Toolset"]) -> None: | ||
| """ | ||
| Add a new Tool or merge another Toolset. | ||
|
|
@@ -262,7 +268,7 @@ def __add__(self, other: Union[Tool, "Toolset", list[Tool]]) -> "Toolset": | |
| if isinstance(other, Tool): | ||
| combined_tools = self.tools + [other] | ||
| elif isinstance(other, Toolset): | ||
| combined_tools = self.tools + list(other) | ||
| return _ToolsetWrapper([self, other]) | ||
| elif isinstance(other, list) and all(isinstance(item, Tool) for item in other): | ||
| combined_tools = self.tools + other | ||
| else: | ||
|
|
@@ -289,3 +295,68 @@ def __getitem__(self, index): | |
| :returns: The Tool at the specified index | ||
| """ | ||
| return self.tools[index] | ||
|
|
||
|
|
||
| class _ToolsetWrapper(Toolset): | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. could you explain why we need
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not sure we need it in this form (there should be something simpler), it was a hack to enable: If they are all MCPToolset and not warmed up when addition above is executed (with current code) we'll just get garbage. We need to preserve their configs (URLs) and then after they've been warmed up they could be added (using our current code) - this way we effectively add them and preserve configs so that when we warm them up they all connect to the right servers. |
||
| """ | ||
| A wrapper that holds multiple toolsets and provides a unified interface. | ||
|
|
||
| This is used internally when combining different types of toolsets to preserve | ||
| their individual configurations while still being usable with ToolInvoker. | ||
| """ | ||
|
|
||
| def __init__(self, toolsets: list[Toolset]): | ||
| self.toolsets = toolsets | ||
| # Check for duplicate tool names across all toolsets | ||
| all_tools = [] | ||
| for toolset in toolsets: | ||
| all_tools.extend(list(toolset)) | ||
| _check_duplicate_tool_names(all_tools) | ||
| super().__init__(tools=all_tools) | ||
|
|
||
| def __iter__(self): | ||
| """Iterate over all tools from all toolsets.""" | ||
| for toolset in self.toolsets: | ||
| yield from toolset | ||
|
|
||
| def __contains__(self, item): | ||
| """Check if a tool is in any of the toolsets.""" | ||
| return any(item in toolset for toolset in self.toolsets) | ||
|
|
||
| def warm_up(self): | ||
| """Warm up all toolsets.""" | ||
| for toolset in self.toolsets: | ||
| toolset.warm_up() | ||
|
|
||
| def __len__(self): | ||
| """Return total number of tools across all toolsets.""" | ||
| return sum(len(toolset) for toolset in self.toolsets) | ||
|
|
||
| def __getitem__(self, index): | ||
| """Get a tool by index across all toolsets.""" | ||
| current_index = 0 | ||
| for toolset in self.toolsets: | ||
| toolset_len = len(toolset) | ||
| if current_index + toolset_len > index: | ||
| return toolset[index - current_index] | ||
| current_index += toolset_len | ||
| raise IndexError("ToolsetWrapper index out of range") | ||
|
|
||
| def __add__(self, other): | ||
| """Add another toolset or tool to this wrapper.""" | ||
| # Import here to avoid circular reference issues | ||
| from haystack.tools.toolset import Toolset | ||
|
|
||
| if isinstance(other, Toolset): | ||
| # Add the toolset to our list | ||
| new_toolsets = self.toolsets + [other] | ||
| elif isinstance(other, Tool): | ||
| # Convert tool to a basic toolset and add it | ||
| new_toolsets = self.toolsets + [Toolset([other])] | ||
| elif isinstance(other, list) and all(isinstance(item, Tool) for item in other): | ||
| # Convert list of tools to a basic toolset and add it | ||
| new_toolsets = self.toolsets + [Toolset(other)] | ||
| else: | ||
| raise TypeError(f"Cannot add {type(other).__name__} to ToolsetWrapper") | ||
|
|
||
| return _ToolsetWrapper(new_toolsets) | ||
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.
Iterables are needed forToolset? Could you please explain?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.
Ah that's just trick to search for Tools. Essentially if it's an iterable (like a list) BUT NOT a string, bytes, or dict then return it so we can iterate through it. See above how we search for (Tool, Toolset) without encoding specifically ChatGenerator ToolInvoker or any other component (Agent in pipeline has tools) also some others tomorrow perhaps. Rather than hardcoding this we look through fields of a component to find tools to warmup
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.
Perhaps an overkill, can simplify as well.