Skip to content

Remove duplicate code in MCPTool and MCPToolset#2053

Closed
vblagoje wants to merge 6 commits into
mainfrom
mcp_refactor_rebased
Closed

Remove duplicate code in MCPTool and MCPToolset#2053
vblagoje wants to merge 6 commits into
mainfrom
mcp_refactor_rebased

Conversation

@vblagoje
Copy link
Copy Markdown
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
Copy Markdown
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.

1 participant