Allow Configuring mcp connect_timeout and client_session_timeout_seconds

[navinpai]

TheMCPAdapt class supports configuring of connect_timeout and session_timeout as shown here: https://github.com/grll/mcpadapt/blob/main/src/mcpadapt/core.py#L172

However, these were not exposed in the MCPAdapter so it was not possible to configure these when using with CrewAI.

This PR allows configuration of those parameters.

Note: The black/isort linting automatically fixed a few other lines in existing tests in mcp_adapter_test.py. Can revert these if not required.

[joaomdmoura]

Disclaimer: This review was made by a crew of AI Agents.

Code Review Comment for PR #374

Overview

This pull request introduces timeout parameters in the MCPServerAdapter class, enhancing its functionality and providing a greater level of control over server connection and session timeouts. The associated tests ensure these changes are validated according to expected behavior.

Key Improvements

• New Parameters: The inclusion of connect_timeout and client_session_timeout_seconds adds flexibility, enabling users to tailor the adapter's performance based on their specific requirements.
• Documentation: Improvements in docstrings clarify the purpose of each parameter, enhancing code readability.

Suggestions for Improvement

1. Import Organization:

   • It’s advisable to separate standard library imports from local module imports for improved readability. For example:

     # Suggested:
     from datetime import timedelta
     from typing import TYPE_CHECKING, Any
     from crewai.tools import BaseTool
     from crewai_tools.adapters.tool_collection import ToolCollection

2. Enhanced Parameter Documentation:

   • Ensure that the parameter docstring explicitly outlines the expected types and behaviors, especially for invalid inputs. This will greatly aid in maintaining code clarity.

3. Comprehensive Parameter Validation:

   • Implement robust type and value checks for the new parameters:

     if not isinstance(connect_timeout, int) or connect_timeout <= 0:
         raise ValueError("connect_timeout must be a positive integer")
     
     if client_session_timeout_seconds is not None and not isinstance(client_session_timeout_seconds, (float, timedelta)):
         raise TypeError("client_session_timeout_seconds must be float, timedelta or None")

4. Structured Test Organization:

   • Group timeout-related tests together using appropriate pytest markers for clarity. This can help in maintaining organized and easily navigable tests.

5. Error Cases Testing:

   • Enhance your testing suite by including cases that examine incorrect timeout settings, ensuring the adapter behaves as expected even in invalid scenarios:

     def test_invalid_timeout_values(self, server_params):
         with pytest.raises(ValueError, match="connect_timeout must be a positive integer"):
             MCPServerAdapter(server_params, connect_timeout=-1)
         
         with pytest.raises(TypeError, match="client_session_timeout_seconds must be float, timedelta or None"):
             MCPServerAdapter(server_params, client_session_timeout_seconds="invalid")

6. Documentation Updates Needed:

   • Ensure the README.md is updated to reflect the added parameters and include examples of typical uses, reinforcing good practices for timeout settings.

Historical Context from Related PRs

Referencing previous pull requests, especially those that added new features or introduced similar patterns, would be beneficial for ensuring consistency in implementation across the codebase. This PR is an excellent step towards better user configurability, following the patterns of related works that also focused on enhancing usability.

Conclusion

Overall, the implementation is a significant enhancement to the MCPServerAdapter, allowing for improved timeout management which could enhance network reliability and performance. Incorporating the above suggestions will lead to a more robust and user-friendly implementation. Thank you for your efforts in improving the code quality with this feature addition!

[navinpai]

@joaomdmoura Is anything required here from my side?