fix: Add checks and better error messages on server object for mcp run#297
Merged
ihrpr merged 2 commits intomodelcontextprotocol:mainfrom May 13, 2025
Merged
Conversation
john0312
force-pushed
the
server_obj_check
branch
from
5f82633 to
e72e167
Compare
john0312
force-pushed
the
server_obj_check
branch
from
e72e167 to
f1eed1a
Compare
Contributor
Author
|
(Force pushed to resolve merge conflict.) |
dsp-ant
requested changes
Mar 19, 2025
Member
dsp-ant
left a comment
dsp-ant
left a comment
There was a problem hiding this comment.
Happy to merge this, but there are two minor nits I'd love to be addressed.
john0312
force-pushed
the
server_obj_check
branch
4 times, most recently
from
6105346 to
2dfdada
Compare
john0312
force-pushed
the
server_obj_check
branch
2 times, most recently
from
a3621c9 to
3f2e84d
Compare
Contributor
Author
|
Hello @dsp-ant |
ihrpr
previously approved these changes
May 13, 2025
Contributor
ihrpr
left a comment
ihrpr
left a comment
There was a problem hiding this comment.
All comments addressed. Thank you!
Contributor
|
@john0312 please can you resolve merge conflict and we can merge. |
…n/dev mcp run/dev command expects the server object in the python file to be a global variable, and it must be of the FastMCP type and not the low level Server type. The previous error messages may be confusing to users who are not aware of these facts.
john0312
force-pushed
the
server_obj_check
branch
from
3f2e84d to
e03aecd
Compare
john0312
force-pushed
the
server_obj_check
branch
from
e03aecd to
9f6df59
Compare
Contributor
Author
|
@ihrpr Rebased. Somehow the force push caused the review to go stale. Please help me review again and merge if possible. Thank you. |
ihrpr
approved these changes
May 13, 2025
Contributor
ihrpr
left a comment
ihrpr
left a comment
There was a problem hiding this comment.
thank you for the quick rebase!
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
mcp run/dev command expects the server object in the python file to be a global variable, and it must be of the FastMCP type and not the low level Server type. The previous error messages may be confusing to users who are not aware of these facts.
Motivation and Context
It is a bit confusing to users who tries to run an MCP server of the low level Server variant with the mcp run command and get unrelated error messages.
How Has This Been Tested?
Run an MCP server of the low level Server variant and check that the correct error message is displayed.
Breaking Changes
None
Types of changes
Checklist
Additional context
N/A