| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
The method backed the get_session_id callback that streamable_http_client used to yield as the third tuple element. That callback was removed in v2, leaving the getter with no callers and no way to reach the transport instance through the public entry point. Drop it along with the stale TODO comments, and note the removal in the existing migration-guide section.
📚 Documentation preview
|
Sorry, something went wrong.
There was a problem hiding this comment.
LGTM — straightforward dead-code removal with the required migration-guide note.
What was reviewed:
The PR deletes the dead StreamableHTTPTransport.get_session_id() method (four lines, previously # pragma: no cover) and two stale TODO(Marcelo) comments from src/mcp/client/streamable_http.py, and appends one sentence to the existing get_session_id callback section of docs/migration.md. No behavior changes: the session_id attribute itself is untouched and continues to drive header stamping, GET/DELETE gating, and terminate-on-close.
None. The change removes a trivial read-only getter; it does not touch auth, headers, session validation, or any request/response handling logic.
Low. This is a mechanical dead-code removal. The only real question is whether anything still calls the method — a repo-wide grep confirms zero callers in src/, tests/, or examples/; the only get_session_id hits outside docs are the server-side private _get_session_id() in src/mcp/server/streamable_http.py, which is a different symbol entirely. The public entry point streamable_http_client never exposed a transport instance on main (it yields only the two streams), so the method was unreachable through the public API anyway.
The bug hunting system found no issues. The PR follows the repo's breaking-change policy: the removal is documented in docs/migration.md, grouped into the existing get_session_id callback section rather than a new standalone one, as AGENTS.md requires. The removal also drops a # pragma: no cover rather than adding one, in line with the coverage guidelines. main is the V2 rework branch where intentional breaking removals like this are expected.
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
Removes the dead StreamableHTTPTransport.get_session_id() method from the client streamable HTTP transport, along with the two stale TODO(Marcelo) comments about it.
Motivation and Context
The method existed to back the get_session_id callback that v1's streamable_http_client yielded as the third tuple element. That callback was already removed on main (documented in docs/migration.md), which left the getter with no callers in src/, tests/, or examples/ (it was # pragma: no cover) and no way to reach a transport instance through the public entry point anyway — streamable_http_client yields only the two streams.
The session_id attribute the getter wrapped is unchanged; it's the transport's own working state (header stamping, GET/DELETE gating, terminate-on-close).
How Has This Been Tested?
Existing client streamable HTTP tests pass. Also drove streamable_http_client end-to-end against a real MCPServer streamable HTTP app on a local port (initialize → tool call → DELETE on close) to confirm the client path is unaffected.
Breaking Changes
StreamableHTTPTransport.get_session_id() no longer exists. This was public in v1, so the removal is added to the existing "get_session_id callback removed from streamable_http_client" section of docs/migration.md, which already points users at httpx2 event hooks for capturing the session ID.
Types of changes
Checklist
Additional context
No pragmas or type-ignores added; the removal drops one # pragma: no cover.