Fix: Add connect_timeout parameter to get_mcp_tools method #3464
+61
−5
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
- Add Optional import to crew_base.py for type annotation - Modify get_mcp_tools to accept connect_timeout parameter with 30s default - Pass connect_timeout to MCPServerAdapter constructor - Update existing test to verify default timeout is passed - Add test for custom timeout values - Add test for backward compatibility Fixes issue #3463 where @crewbase users couldn't configure MCP server connection timeouts Co-Authored-By: João <[email protected]>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
- Add ClassVar import to tests for proper type annotations - Add type ignore comment for crewai_tools import to suppress mypy error - Fix line length violations by breaking long assert statements - Add ClassVar annotations for mutable class attributes in test classes Co-Authored-By: João <[email protected]>
|
Closing due to inactivity for more than 7 days. Configure here. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fix: Add connect_timeout parameter to get_mcp_tools method
Summary
Fixes issue #3463 by adding an optional
connect_timeoutparameter to theget_mcp_toolsmethod in@CrewBase. This allows users to configure connection timeouts when connecting to MCP servers, preventing timeout errors that were occurring with the default 30-second limit.Key Changes:
connect_timeout: Optional[int] = 30parameter toget_mcp_tools()methodOptionalto typing imports increw_base.pyconnect_timeouttoMCPServerAdapterconstructorReview & Testing Checklist for Human
MCPServerAdapter(server_params, connect_timeout=timeout)is the correct constructor signature (my tests use mocks so this wasn't verified locally)connect_timeoutonly applies during first initialization. Verify this behavior is intuitive to usersget_mcp_tools()without timeout parameter continues working unchangedRecommended Test Plan:
crew.get_mcp_tools()without timeout parameter (should use 30s default)crew.get_mcp_tools(connect_timeout=60)with custom timeoutNotes