Skip to content

Conversation

vblagoje
Copy link
Member

Why:

Consolidates ~280 lines of duplicate code between MCPTool and MCPToolset into ~130 lines of shared utilities, achieving ~150 net lines saved while eliminating DRY violations and improving maintainability with zero breaking changes to public APIs.

What:

  • Added MCPConnectionManager class to encapsulate shared connection logic
  • Created _create_connection_error_message() utility for unified error handling across server types
  • Extracted create_tool_invoke_function() to share tool invocation patterns
  • Refactored both classes to use shared utilities while maintaining distinct design patterns

How can it be used:

# MCPTool usage remains identical
tool = MCPTool(name="get_time", server_info=server_info)
result = tool.invoke(timezone="UTC")

# MCPToolset usage remains identical  
toolset = MCPToolset(server_info=server_info, tool_names=["get_time"])
result = toolset.tools[0].invoke(timezone="UTC")

# Both now share connection logic internally but maintain their distinct APIs

How did you test it:

  • Verified identical behavior for connection establishment, error handling, and tool invocation
  • Validated serialization/deserialization functionality remains unchanged

Notes for the reviewer:

Focus areas: The MCPConnectionManager handles all shared lifecycle management while each class maintains its design philosophy - MCPTool as direct Tool inheritance vs MCPToolset as tool factory. Git diff shows +244/-170 due to net-per-file counting, but actual duplicate code elimination is ~280 lines replaced with ~130 shared utilities.

@github-actions github-actions bot added integration:mcp type:documentation Improvements or additions to documentation labels Jun 26, 2025
@vblagoje
Copy link
Member Author

Will rebase onto #2019 and then open a PR

@vblagoje
Copy link
Member Author

vblagoje commented Jul 3, 2025

Superseded by #2053
Closing this one

@vblagoje vblagoje closed this Jul 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
integration:mcp type:documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant