-
Notifications
You must be signed in to change notification settings - Fork 3
Dependencies: Allow updating to FastMCP 2.9 #47
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughThe changes update dependency constraints for FastMCP, adjust how the port parameter is passed to the MCP server, refactor tests to use asynchronous FastMCP client calls, enhance test configuration with new pytest options and dependencies, and improve test reliability with timeouts. The changelog and test assertions were updated accordingly. Changes
Sequence Diagram(s)sequenceDiagram
participant Test as Test Suite
participant Client as FastMCP Client
participant MCP as MCP Server
Test->>Client: call_tool(name, args)
activate Client
Client->>MCP: send tool call request
activate MCP
MCP-->>Client: return response/error
deactivate MCP
Client-->>Test: return result/raise ToolError
deactivate Client
Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (6)
🚧 Files skipped from review as they are similar to previous changes (6)
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (1)
tests/test_mcp.py (1)
12-16: Unused fixture: Consider removing or utilizing the session-scoped client.The
clientfixture is defined but never used. The helper functions create new client instances instead of reusing this fixture. Either utilize this fixture in the helper functions for better performance, or remove it if not needed.If you want to utilize the fixture, refactor the helper functions:
-async def query_async(sql: str) -> dict: - async with Client(transport=FastMCPTransport(mcp)) as client: - response = await client.call_tool("query_sql", {"query": sql}) - return json.loads(response[0].text) +async def query_async(client, sql: str) -> dict: + response = await client.call_tool("query_sql", {"query": sql}) + return json.loads(response[0].text)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
CHANGES.md(1 hunks)cratedb_mcp/cli.py(1 hunks)pyproject.toml(3 hunks)tests/test_cli.py(2 hunks)tests/test_examples.py(1 hunks)tests/test_mcp.py(1 hunks)
🧰 Additional context used
🧠 Learnings (5)
📓 Common learnings
Learnt from: amotl
PR: crate/cratedb-mcp#24
File: pyproject.toml:76-76
Timestamp: 2025-05-17T16:02:09.342Z
Learning: For the cratedb-mcp project, FastMCP pre-2.0 releases provide the same API surface needed by the application, so a lower bound on the FastMCP dependency is not necessary.
Learnt from: amotl
PR: crate/cratedb-mcp#17
File: CHANGES.md:5-5
Timestamp: 2025-05-16T13:35:04.487Z
Learning: For the cratedb-mcp repository, version numbers and release dates in CHANGES.md are updated by a dedicated "Release vX.Y.Z" commit as part of the release process, not during documentation preparation PRs.
cratedb_mcp/cli.py (2)
Learnt from: amotl
PR: crate/cratedb-mcp#17
File: CHANGES.md:5-5
Timestamp: 2025-05-16T13:35:04.487Z
Learning: For the cratedb-mcp repository, version numbers and release dates in CHANGES.md are updated by a dedicated "Release vX.Y.Z" commit as part of the release process, not during documentation preparation PRs.
Learnt from: amotl
PR: crate/cratedb-mcp#24
File: pyproject.toml:76-76
Timestamp: 2025-05-17T16:02:09.342Z
Learning: For the cratedb-mcp project, FastMCP pre-2.0 releases provide the same API surface needed by the application, so a lower bound on the FastMCP dependency is not necessary.
CHANGES.md (3)
Learnt from: amotl
PR: crate/cratedb-mcp#17
File: CHANGES.md:5-5
Timestamp: 2025-05-16T13:35:04.487Z
Learning: For the cratedb-mcp repository, version numbers and release dates in CHANGES.md are updated by a dedicated "Release vX.Y.Z" commit as part of the release process, not during documentation preparation PRs.
Learnt from: amotl
PR: crate/cratedb-mcp#24
File: pyproject.toml:76-76
Timestamp: 2025-05-17T16:02:09.342Z
Learning: For the cratedb-mcp project, FastMCP pre-2.0 releases provide the same API surface needed by the application, so a lower bound on the FastMCP dependency is not necessary.
Learnt from: amotl
PR: crate/cratedb-mcp#0
File: :0-0
Timestamp: 2025-05-17T14:55:23.400Z
Learning: In the cratedb-mcp repository, the `backlog.md` document is intended as a functional planning document that doesn't need to adhere to strict formatting standards. Avoid pointing out formatting or layout shortcomings in this file during reviews.
pyproject.toml (2)
Learnt from: amotl
PR: crate/cratedb-mcp#24
File: pyproject.toml:76-76
Timestamp: 2025-05-17T16:02:09.342Z
Learning: For the cratedb-mcp project, FastMCP pre-2.0 releases provide the same API surface needed by the application, so a lower bound on the FastMCP dependency is not necessary.
Learnt from: amotl
PR: crate/cratedb-mcp#17
File: CHANGES.md:5-5
Timestamp: 2025-05-16T13:35:04.487Z
Learning: For the cratedb-mcp repository, version numbers and release dates in CHANGES.md are updated by a dedicated "Release vX.Y.Z" commit as part of the release process, not during documentation preparation PRs.
tests/test_mcp.py (3)
Learnt from: amotl
PR: crate/cratedb-mcp#17
File: CHANGES.md:5-5
Timestamp: 2025-05-16T13:35:04.487Z
Learning: For the cratedb-mcp repository, version numbers and release dates in CHANGES.md are updated by a dedicated "Release vX.Y.Z" commit as part of the release process, not during documentation preparation PRs.
Learnt from: amotl
PR: crate/cratedb-mcp#24
File: pyproject.toml:76-76
Timestamp: 2025-05-17T16:02:09.342Z
Learning: For the cratedb-mcp project, FastMCP pre-2.0 releases provide the same API surface needed by the application, so a lower bound on the FastMCP dependency is not necessary.
Learnt from: amotl
PR: crate/cratedb-mcp#0
File: :0-0
Timestamp: 2025-05-17T14:55:23.400Z
Learning: In the cratedb-mcp repository, the `backlog.md` document is intended as a functional planning document that doesn't need to adhere to strict formatting standards. Avoid pointing out formatting or layout shortcomings in this file during reviews.
🔇 Additional comments (10)
cratedb_mcp/cli.py (1)
49-49: Clean refactoring: Port configuration consolidated into run call.The change from setting
mcp.settings.portseparately to passingportdirectly as a keyword argument tomcp.run()improves code clarity and follows the updated FastMCP API pattern. This consolidation eliminates the need for separate settings assignment.pyproject.toml (3)
77-77: FastMCP dependency update aligns with PR objectives.The updated constraint
>=2,<2.10successfully allows FastMCP 2.9 as intended. The upper bound provides protection against potential breaking changes in 2.10+ while maintaining compatibility with the target version.
91-95: Test infrastructure enhancements support asynchronous testing.The addition of
pytest-asyncio,pytest-env, andpytest-timeoutdependencies provides the necessary infrastructure for testing asynchronous FastMCP functionality and improves test reliability with timeout controls.
153-162: Comprehensive pytest configuration for async testing and environment control.The new pytest configuration appropriately enables:
- Automatic asyncio mode with session-scoped fixtures
- Global 3-second timeout for test reliability
- FastMCP-specific test environment variables
This configuration aligns well with the FastMCP 2.9 update and asynchronous testing patterns.
CHANGES.md (1)
4-5: Changelog entries accurately document the changes.The two new entries clearly document the key changes in this PR:
- The port parameter refactoring in the CLI
- The FastMCP 2.9 dependency update
The entries are concise and follow the established changelog format.
tests/test_examples.py (1)
7-7: Test timeout marker improves reliability.The 10-second pytest timeout marker provides appropriate per-test timeout control, complementing the global timeout settings in
pyproject.toml. The timeout is reasonable given the test runs an external script with a 15-second subprocess timeout.tests/test_cli.py (2)
81-81: Test assertion correctly verifies the new port handling approach.The updated assertion
mock.call("stdio", port=8000)properly verifies that the mockedrun_asyncmethod is called with the port as a keyword argument, reflecting the CLI refactoring.
102-102: Test assertion correctly verifies custom port parameter handling.The updated assertion
mock.call("streamable-http", port=65535)properly verifies that custom port values are passed correctly as keyword arguments to the mockedrun_asyncmethod.tests/test_mcp.py (2)
1-10: LGTM! Import changes support the async refactor.The new imports properly support the transition from direct function calls to asynchronous MCP client calls. The addition of
ToolErroraligns with the standardized error handling approach.
40-99: LGTM! Test functions properly updated for async MCP client.The test functions have been correctly refactored to:
- Use the new
call_toolandquerywrappers instead of direct function calls- Handle
ToolErrorexceptions appropriately (replacingValueError/PermissionError)- Decode JSON responses where needed using the
decode_jsonparameterThe refactor maintains the same test logic while properly integrating with the FastMCP client interface.
- Calling tools now returns a `FunctionTool` object instance - PermissionError -> ToolError - Add more `pytest` utilities from FastMCP testing best practises
|
This patch has been made partly obsolete by GH-52. |
kneth
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
|
GH-48 has been refreshed, queuing up for merging, so this patch is effectively obsolete now. |
About
Supporting Dependabot on updating to higher releases of FastMCP.
Details
FunctionToolobject instancePermissionError->ToolErrorReferences
TypeError: 'FunctionTool' object is not callable#46