[FEEDS-1373]feat: allow custom HTTP transport and client configuration#235
[FEEDS-1373]feat: allow custom HTTP transport and client configuration#235
Conversation
📝 WalkthroughWalkthroughAdds optional transport/http_client injection and shared HTTPX client reuse across the SDK; client instances may use an externally supplied HTTPX client (mutated in-place) or construct an owned client, with ownership tracked to control close behavior. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Stream as BaseStream
participant BaseClient as BaseClient / AsyncBaseClient
participant Sub as Sub-Clients (Chat/Video/Moderation/Feeds)
participant HTTPX as HTTPX Client
User->>Stream: __init__(transport=X or http_client=Y)
Stream->>Stream: store _transport/_http_client
Stream->>BaseClient: construct client (passes _transport/_http_client)
alt external http_client provided
BaseClient->>HTTPX: use provided client (mutate headers/params/base_url/timeout)
BaseClient->>BaseClient: _owns_http_client = False
else no external client
BaseClient->>HTTPX: create new HTTPX client (inject transport if present)
BaseClient->>BaseClient: _owns_http_client = True
end
Stream->>Stream: _shared_client = self.client (when custom config)
User->>Sub: access sub-client (e.g., stream.chat)
Sub->>BaseClient: __init__(http_client=_shared_client or resolved)
Sub->>HTTPX: reuse shared HTTPX client for requests
User->>Stream: close()
alt _owns_http_client is True
Stream->>HTTPX: close()
else
Stream->>HTTPX: skip close (no-op)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Add `transport` and `http_client` parameters to `Stream` and `AsyncStream` so users can configure connection pool limits, retries, SSL, HTTP/2, and other httpx transport-level settings — matching the Java SDK pattern where a pre-built OkHttpClient can be passed in. - `transport` (primary): user provides an `httpx.HTTPTransport` / `httpx.AsyncHTTPTransport`; the SDK builds its own client with it and manages the lifecycle. - `http_client` (escape hatch): user provides a fully built `httpx.Client` / `httpx.AsyncClient`; the SDK layers auth headers and params on top. Caller manages lifecycle. When either is provided, all sub-clients (video, chat, moderation, feeds) share a single underlying httpx client via the `stream` back-reference instead of each creating their own. Made-with: Cursor
25b1a31 to
87378dd
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
getstream/stream.py (1)
50-88:⚠️ Potential issue | 🟠 Major
as_async()now drops the new HTTP configuration.After this change,
transport/http_clientare part ofStreamstate, butStream.as_async()still rebuildsAsyncStreamwith only credentials, URL, timeout, and user agent. That means a caller can configure retries/limits/SSL on the sync client and then silently lose them when switching to async. Please either propagate this explicitly where possible or fail fast whenas_async()is used with custom HTTP config.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@getstream/stream.py` around lines 50 - 88, as_async() currently drops custom HTTP configuration because Stream stores transport/http_client but as_async() rebuilds AsyncStream only with api_key/api_secret/base_url/timeout/user_agent; update Stream.as_async (and/or AsyncStream constructor usage) to propagate transport and http_client (and preserve _shared_client) when constructing the async instance, or alternatively raise a clear ValueError if transport or http_client is set to prevent silent loss; refer to the Stream.as_async method, the AsyncStream constructor, and members transport, http_client, _shared_client to ensure the same HTTP config is passed through or disallowed.
🧹 Nitpick comments (2)
tests/test_http_client.py (2)
79-87: Avoid asserting privatehttpxinternals in this test.
_transport,_pool, and the pool limit fields are internal implementation details, so this test is likely to break on anhttpxupgrade without any SDK regression. Prefer asserting the observable contract here, e.g. that the provided transport instance is the one wired into the client.Lower-coupling alternative
def test_custom_limits_propagated(self): limits = httpx.Limits(max_connections=42, max_keepalive_connections=10) transport = httpx.HTTPTransport(limits=limits) client = Stream( api_key="k", api_secret="s", base_url="http://test", transport=transport ) - pool = client.client._transport._pool - assert pool._max_connections == 42 - assert pool._max_keepalive_connections == 10 + assert client.client._transport is transport🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/test_http_client.py` around lines 79 - 87, The test test_custom_limits_propagated is asserting private httpx internals (_transport, _pool and pool limit fields); instead replace those fragile assertions with a check that the provided transport instance is the one wired into the Stream client (e.g., assert client.client._transport is transport or otherwise access the public/expected attribute on the Stream instance that holds the transport), and remove any assertions referencing _pool, _max_connections or _max_keepalive_connections.
7-23: Please convert these transport helpers into pytest fixtures and avoidMockTransporthere.This helper pattern drives the whole file toward inline client construction plus
httpx.MockTransport, which conflicts with the repo’s test conventions. A small set of fixtures that yieldStream/AsyncStreamclients would also make cleanup of caller-managedhttpxclients deterministic.As per coding guidelines, "Use fixtures to inject objects in tests; test client fixtures can use the Stream API client" and "Do not use mocks or mock objects in tests unless directly requested".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/test_http_client.py` around lines 7 - 23, Replace the helper functions _mock_transport and _capture_transport with pytest fixtures that create and yield real http client instances (Stream/AsyncStream API) instead of using httpx.MockTransport; implement one fixture that yields a configured streaming client preloaded to return the desired status/body for tests (use a test server or httpx.MockServer/test server helper if available) and another fixture that yields a streaming client plus a request-capture container for assertions; ensure fixtures perform proper setup/teardown (closing the client) so tests no longer construct inline clients or use MockTransport directly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@getstream/chat/client.py`:
- Around line 17-19: The __init__ constructor signature in
getstream/chat/client.py is triggering Ruff/formatting errors due to the current
parameter wrapping; update the __init__ definition to conform to the project's
line-wrapping and trailing-comma conventions (e.g., one parameter per line with
proper type annotations and a trailing comma) and apply Ruff/Black formatting so
the same pattern is consistently fixed across other client constructors as well.
---
Outside diff comments:
In `@getstream/stream.py`:
- Around line 50-88: as_async() currently drops custom HTTP configuration
because Stream stores transport/http_client but as_async() rebuilds AsyncStream
only with api_key/api_secret/base_url/timeout/user_agent; update Stream.as_async
(and/or AsyncStream constructor usage) to propagate transport and http_client
(and preserve _shared_client) when constructing the async instance, or
alternatively raise a clear ValueError if transport or http_client is set to
prevent silent loss; refer to the Stream.as_async method, the AsyncStream
constructor, and members transport, http_client, _shared_client to ensure the
same HTTP config is passed through or disallowed.
---
Nitpick comments:
In `@tests/test_http_client.py`:
- Around line 79-87: The test test_custom_limits_propagated is asserting private
httpx internals (_transport, _pool and pool limit fields); instead replace those
fragile assertions with a check that the provided transport instance is the one
wired into the Stream client (e.g., assert client.client._transport is transport
or otherwise access the public/expected attribute on the Stream instance that
holds the transport), and remove any assertions referencing _pool,
_max_connections or _max_keepalive_connections.
- Around line 7-23: Replace the helper functions _mock_transport and
_capture_transport with pytest fixtures that create and yield real http client
instances (Stream/AsyncStream API) instead of using httpx.MockTransport;
implement one fixture that yields a configured streaming client preloaded to
return the desired status/body for tests (use a test server or
httpx.MockServer/test server helper if available) and another fixture that
yields a streaming client plus a request-capture container for assertions;
ensure fixtures perform proper setup/teardown (closing the client) so tests no
longer construct inline clients or use MockTransport directly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f66eab4b-be50-4072-b8b7-26040e805d72
📒 Files selected for processing (10)
getstream/base.pygetstream/chat/async_client.pygetstream/chat/client.pygetstream/feeds/client.pygetstream/moderation/async_client.pygetstream/moderation/client.pygetstream/stream.pygetstream/video/async_client.pygetstream/video/client.pytests/test_http_client.py
getstream/chat/client.py
Outdated
| def __init__( | ||
| self, api_key: str, base_url, token, timeout, stream, user_agent=None, http_client=None | ||
| ): |
There was a problem hiding this comment.
Run Ruff on the new constructor signature.
CI is already failing on formatting here, and this wrapping pattern is repeated across the other touched client constructors as well.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@getstream/chat/client.py` around lines 17 - 19, The __init__ constructor
signature in getstream/chat/client.py is triggering Ruff/formatting errors due
to the current parameter wrapping; update the __init__ definition to conform to
the project's line-wrapping and trailing-comma conventions (e.g., one parameter
per line with proper type annotations and a trailing comma) and apply Ruff/Black
formatting so the same pattern is consistently fixed across other client
constructors as well.
Move _apply_shared_client into BaseStream so sub-client files need zero HTTP-related changes. Remove _resolve_http_client from BaseClient/AsyncBaseClient. Made-with: Cursor
Avoid **dict unpacking which ty cannot type-check. Made-with: Cursor
Summary
transportandhttp_clientparameters toStream/AsyncStream, allowing users to configure connection pool limits, retries, SSL, HTTP/2, and other httpx transport-level settings.OkHttpClientcan be passed in and the SDK layers its auth on top.Two-tier API
transport(primary)httpx.HTTPTransportwith pool limits, retries, SSL config. SDK builds its own client.http_client(escape hatch)httpx.Clientfor event hooks or other advanced config. SDK adds auth headers/params.Usage
Design decisions
_http_client/_transportare set as instance attributes beforesuper().__init__()in hand-written wrappers, picked up byBaseClientviagetattr(self, ...).transportandhttp_clientraisesValueError.httpx.AsyncClientto syncStream(or vice versa) raisesTypeErrorimmediately.Test plan
tests/test_http_client.pycovering transport, http_client, sharing, close semantics, validationMade with Cursor
Summary by CodeRabbit
New Features
Bug Fixes
Tests