Skip to content

Conversation

@vblagoje
Copy link
Member

@vblagoje vblagoje commented Jul 3, 2025

Why:

Consolidates ~300 lines of duplicate code between MCPTool and MCPToolset into ~130 lines of shared utilities 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

@github-actions github-actions bot added integration:mcp type:documentation Improvements or additions to documentation labels Jul 3, 2025
@vblagoje vblagoje changed the title Mcp refactor rebased Remove duplicate code in MCPTool and MCPToolset Jul 3, 2025
@vblagoje
Copy link
Member Author

Too outdated, will tackle with another PR

@vblagoje vblagoje closed this Nov 18, 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.

2 participants