concurrent_startup param for mcp client to startup all mcp servers parallel async#2597
concurrent_startup param for mcp client to startup all mcp servers parallel async#2597geobio wants to merge 1 commit intoPrefectHQ:mainfrom
Conversation
Test Failure AnalysisSummary: The static analysis workflow failed because detected formatting issues in Root Cause: The newly added test file doesn't conform to the project's formatting standards enforced by . Specifically:
Suggested Solution: Run the following command locally to fix the formatting: uv run prek run --all-filesThis will auto-format the file to match project standards. Then commit and push the changes. Detailed AnalysisThe workflow shows: The specific changes needed in -@pytest.mark.skip(reason="Known issue with proxy keep-alive in tests - works in practice")
+@pytest.mark.skip(
+ reason="Known issue with proxy keep-alive in tests - works in practice"
+)
# And remove trailing blank line at end of fileRelated Files
|
|
Caution Review failedThe pull request is closed. WalkthroughThis pull request adds a Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (3)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
src/fastmcp/utilities/mcp_config.py (2)
81-92: Consider logging cleanup failures instead of silent pass.The bare
except Exceptioncatches are flagged by static analysis (BLE001, S110). While acceptable for cleanup code, silently swallowing exceptions can hide debugging information.except Exception as e: logs = "" if temp_log_path and temp_log_path.exists(): try: logs = temp_log_path.read_text() temp_log_path.unlink() - except Exception: - pass + except OSError: + # Cleanup failure is non-critical; proceed with error reporting + pass return (index, logs, e)Using
OSError(covers file I/O errors) is more specific than bareException.
45-48: Redundant import ofStdioTransport.
StdioTransportis already imported at the module level (line 9). The local import on line 48 is unnecessary.import tempfile from pathlib import Path - from fastmcp.client.transports import StdioTransport - if not transports: returnsrc/fastmcp/client/transports.py (1)
1019-1028: Consider makingshow_startup_logsconfigurable.Currently
show_startup_logs=Trueis hardcoded. Users may want to suppress startup log output in production or CI environments.def __init__( self, config: MCPConfig | dict, name_as_prefix: bool = True, concurrent_startup: bool = False, + show_startup_logs: bool = True, ): ... self.concurrent_startup = concurrent_startup + self._show_startup_logs = show_startup_logsThen in
connect_session:await warm_up_mcp_config_transports( self._underlying_transports, server_names=self._server_names, - show_startup_logs=True, + show_startup_logs=self._show_startup_logs, )This would also require propagating through
infer_transportandClient.__init__.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
tests/client/test_concurrent_startup.pyis excluded by none and included by none
📒 Files selected for processing (3)
src/fastmcp/client/client.py(3 hunks)src/fastmcp/client/transports.py(7 hunks)src/fastmcp/utilities/mcp_config.py(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
src/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.py: Python source code must use Python ≥3.10 with full type annotations
Never use bareexcept- be specific with exception types
Prioritize readable, understandable code - clarity over cleverness; avoid obfuscated or confusing patterns even if shorter
Follow existing patterns and maintain consistency in code organization and style
Files:
src/fastmcp/client/client.pysrc/fastmcp/client/transports.pysrc/fastmcp/utilities/mcp_config.py
🧠 Learnings (2)
📓 Common learnings
Learnt from: CR
Repo: jlowin/fastmcp PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-04T00:17:41.256Z
Learning: Applies to tests/**/*.py : Pass FastMCP servers directly to clients for testing using in-memory transport; only use HTTP transport when explicitly testing network features
📚 Learning: 2025-12-04T00:17:41.256Z
Learnt from: CR
Repo: jlowin/fastmcp PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-04T00:17:41.256Z
Learning: Applies to tests/**/*.py : Pass FastMCP servers directly to clients for testing using in-memory transport; only use HTTP transport when explicitly testing network features
Applied to files:
src/fastmcp/client/client.pysrc/fastmcp/client/transports.pysrc/fastmcp/utilities/mcp_config.py
🧬 Code graph analysis (2)
src/fastmcp/client/client.py (1)
src/fastmcp/client/transports.py (9)
infer_transport(1042-1042)infer_transport(1046-1046)infer_transport(1050-1050)infer_transport(1054-1054)infer_transport(1058-1058)infer_transport(1062-1064)infer_transport(1068-1072)infer_transport(1076-1076)infer_transport(1079-1160)
src/fastmcp/utilities/mcp_config.py (3)
src/fastmcp/server/server.py (2)
name(370-371)FastMCP(173-2970)src/fastmcp/client/transports.py (2)
ClientTransport(76-118)connect(378-412)src/fastmcp/server/context.py (1)
fastmcp(150-155)
🪛 Ruff (0.14.8)
src/fastmcp/utilities/mcp_config.py
79-79: Consider moving this statement to an else block
(TRY300)
81-81: Do not catch blind exception: Exception
(BLE001)
87-88: try-except-pass detected, consider logging the exception
(S110)
87-87: Do not catch blind exception: Exception
(BLE001)
108-108: Avoid specifying long messages outside the exception class
(TRY003)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Run tests: Python 3.10 on ubuntu-latest
- GitHub Check: Run tests: Python 3.10 on windows-latest
- GitHub Check: Run tests with lowest-direct dependencies
🔇 Additional comments (5)
src/fastmcp/client/client.py (1)
175-177: LGTM! Clean parameter propagation.The
concurrent_startupparameter is properly typed, documented, and correctly forwarded toinfer_transport. The docstring clearly explains the behavior and trade-offs.Also applies to: 276-276, 280-284
src/fastmcp/utilities/mcp_config.py (2)
95-99: Verify exception handling withreturn_exceptions=False.The inner function
connect_with_log_capturecatches all exceptions and returns them as tuples. However,asyncio.CancelledError(aBaseException) will escape and causeasyncio.gatherto cancel remaining tasks.If task cancellation should be handled gracefully (e.g., during shutdown), consider catching
BaseExceptionor usingreturn_exceptions=True:- except Exception as e: + except BaseException as e: + if isinstance(e, asyncio.CancelledError): + raise # Re-raise cancellation to propagate properly logs = ""Alternatively, if current behavior is intentional (cancellation should abort all servers), this is fine as-is.
111-145: LGTM! Clear startup log formatting.The
_display_startup_logshelper provides readable sequential output with proper status indicators and summary. Usingsys.stderris appropriate for startup diagnostics.src/fastmcp/client/transports.py (2)
972-987: LGTM! Well-structured concurrent startup integration.The
MCPConfigTransportcorrectly tracks server names and warm-up state. Theconcurrent_startupflag is properly stored and defaults toFalsefor backward compatibility.
1088-1096: LGTM!concurrent_startupcorrectly scoped to MCPConfig.The parameter is properly documented and only applied when creating
MCPConfigTransport. Other transport types correctly ignore it, maintaining backward compatibility.Also applies to: 1148-1153
7f1b5e6 to
f853d9c
Compare
f853d9c to
6ab5a6c
Compare
Add concurrent MCP server startup
Adds
concurrent_startupparameter toClientfor parallel initialization of multiple MCP servers.Problem
When connecting to many MCP servers via
MCPConfig, they currently start sequentially, causing slow initialization (e.g., 30 servers × 5s = 150 seconds (we have experienced this forreals)).Solution
concurrent_startupall servers start concurrently viaasyncio.gather()Changes
concurrent_startup: bool = Falseparameter toClientwarm_up_mcp_config_transports()for concurrent connectionExample