[fix] add type field to client session config BaseModels#1127
[fix] add type field to client session config BaseModels#1127bfineran wants to merge 1 commit intomodelcontextprotocol:mainfrom
Conversation
e455823 to
55cf14d
Compare
7119405 to
cae5231
Compare
Kludex
left a comment
There was a problem hiding this comment.
I don't think this is necessary. You can handle the serialization logic in your application.
|
Hi @bfineran thank you for your contribution! And apologies for the time it took to get back tot his.
Agree with this, this seems like a concern that should be handled at the application layer and not in the SDK - within the SDK we can leverage If you're storing these configurations somehow and need to discriminate between different types, you could have a wrapper class that does this for you without having to modify the SDK types for it. Something like this: class ServerConfig(BaseModel):
type: Literal["stdio", "sse", "streamable_http"]
config: dict[str, Any]
def to_server_params(self) -> Union[StdioServerParameters, ...]:
if self.type == "stdio":
# do stuff
elif self.type == "sse":
# do stuff
elif self.type == "streamable_http":
# do stuff |
Adds a pre-set
typefield toSseServerParameters,StreamableHttpParameters, andStdioServerParametersso serialized parameters will easily be loaded into the correct class.Motivation and Context
All fields of
SseServerParametersoverlap withStreamableHttpParameters, so a third party application that would want to connect to multiple session with a config such as:would not have a way to be instantiated from a serialized config (like a yaml file) since the following is ambiguous.
Additionally, if someone were to serialize either the sse or http config classes they would not be able to reload the second class listed in the union.
Adding the preset
typefield should fix this in a backwards compatible way.How Has This Been Tested?
Existing tests cover backwards compatibility, confirmed this PR addresses the described use case
Breaking Changes
No, added fields include the only valid Literal default.
Types of changes
Checklist
Additional context