-
Notifications
You must be signed in to change notification settings - Fork 3k
Add meta to Client methods
#1923
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
src/mcp/client/client.py
Outdated
| from mcp.server.fastmcp import FastMCP | ||
| from mcp.shared.session import ProgressFnT | ||
|
|
||
| logger = logging.getLogger(__name__) |
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.
Unnecessary logger.
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.
Changes here are not breaking change.
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.
Adding the meta parameter in the methods here is also not a breaking change, because they have a default value. Which means is just a feature.
| "TaskExecutionMode", | ||
| "TaskStatus", | ||
| # Base classes | ||
| "MCPModel", |
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.
MCPModel should be internal. It's not supposed to be exposed.
A lot of types here aren't supposed to be exposed.
| model_config = ConfigDict(extra="allow", alias_generator=to_camel, populate_by_name=True) | ||
|
|
||
|
|
||
| Meta: TypeAlias = dict[str, Any] |
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.
The Meta type was being used as dict[str, Any] or as class Meta(BaseModel) depending on the class, which doesn't make sense. I've set dict[str, Any] for all of them, which matches the spec and most of the types here.
src/mcp/client/client.py
Outdated
| return await self.session.list_prompts(params=types.PaginatedRequestParams(cursor=cursor)) | ||
| return await self.session.list_prompts(params=types.PaginatedRequestParams(cursor=cursor, _meta=meta)) | ||
|
|
||
| async def get_prompt(self, name: str, arguments: dict[str, str] | None = None) -> types.GetPromptResult: |
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.
meta here?
| if ctx.meta is not None and ctx.meta.model_extra: # pragma: no branch | ||
| received_meta[0] = ctx.meta.model_extra.get("custom_field") | ||
| if ctx.meta is not None: # pragma: no branch | ||
| received_meta[0] = ctx.meta.get("custom_field") |
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.
should we call out this change in the migration docs? If people are putting extra things in their meta tags I assume they'd need to do this now?
| async def handle_tool(name: str, arguments: dict) -> list[TextContent]: | ||
| ctx = server.request_context | ||
| if ctx.meta and "progress_token" in ctx.meta: | ||
| await ctx.session.send_progress_notification(ctx.meta["progress_token"], 0.5, 100) |
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.
feels weird that it's key access but makes sense given you can add anything to meta. just weird vibes.
No description provided.