Skip to content

Conversation

Kludex
Copy link
Member

@Kludex Kludex commented Aug 27, 2025

Thanks to The Schema Expert (@Viicos) for the help 🙏

Copy link

github-actions bot commented Aug 27, 2025

Docs Preview

commit: bf36f23
Preview URL: https://68071c74-pydantic-ai-previews.pydantic.workers.dev

def __get_pydantic_core_schema__(cls, _: Any, __: Any) -> CoreSchema:
return core_schema.no_info_after_validator_function(
lambda dct: MCPServerStreamableHTTP(**dct),
core_schema.typed_dict_schema({'url': core_schema.typed_dict_field(core_schema.str_schema())}),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should support headers as well right?

]


def load_mcp_servers(config_path: str | Path) -> list[MCPServerStdio | MCPServerStreamableHTTP | MCPServerSSE]:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
def load_mcp_servers(config_path: str | Path) -> list[MCPServerStdio | MCPServerStreamableHTTP | MCPServerSSE]:
def load_mcp_servers(config_path: str | Path) -> list[MCPServer]:

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is technically not correct given that it needs to have the __get_pydantic... thing.

]


def load_mcp_servers(config_path: str | Path) -> list[MCPServerStdio | MCPServerStreamableHTTP | MCPServerSSE]:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd prefer this to be a classmethod on MCPServer, as it's basically an alternative bulk constructor.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a bulk operation, but I think that's exactly what merits it being its own function

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Kludex You win

@DouweM DouweM mentioned this pull request Sep 8, 2025
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants