Skip to content

StreamableHttp - Update examples and README #351

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

Merged
merged 6 commits into from
Apr 17, 2025
Merged

Conversation

ihrpr
Copy link
Contributor

@ihrpr ihrpr commented Apr 16, 2025

Now StreamableHttp is available, we need to update README and examples.

Note

stacked on top of #347

Copy link

@irvinebroque irvinebroque left a comment

Choose a reason for hiding this comment

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

Is it possibly confusing to remove SSE at top level, given that people already currently use it? Know it's covered below under backwards compat, just wonder if maybe needs louder callout?

Copy link
Member

@jspahrsummers jspahrsummers left a comment

Choose a reason for hiding this comment

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

itshappening

@@ -9,7 +9,7 @@ import { CallToolResult } from '../../types.js';
* (protocol version 2024-11-05). It mainly used for testing backward compatible clients.
*
* The server exposes two endpoints:
* - /sse: For establishing the SSE stream (GET)
* - /mcp: For establishing the SSE stream (GET)
Copy link
Member

Choose a reason for hiding this comment

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

Should we use /sse, though?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

technically it doesn't matter and in this way it's much easier to re-use the same client for testing. THis example should not be used now for use cases, only for testing clients. Ideally should be deleted soon.

Base automatically changed from ihrpr/backwards_compatibility to main April 17, 2025 16:45
@ihrpr
Copy link
Contributor Author

ihrpr commented Apr 17, 2025

Is it possibly confusing to remove SSE at top level, given that people already currently use it? Know it's covered below under backwards compat, just wonder if maybe needs louder callout?
@irvinebroque it's still in the examples folders, hope it's sufficient. I think the root README should reflect the most recent spec, given SDK implements the changes in the spec.

@ihrpr ihrpr merged commit 64653f5 into main Apr 17, 2025
4 checks passed
@ihrpr ihrpr deleted the ihrpr/clean-up-examples branch April 17, 2025 17:32
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