Skip to content

Commit b57abf1

Browse files
pmbrullclaude
andcommitted
fix(ometa): address PR review on SSE client
Three follow-ups from Copilot's review on PR open-metadata#28293: 1. Forward `ClientConfig.cert` to the SSE request so mTLS / client-cert setups work for SSE the same way they do for the REST client. Without this, deployments that authenticate the REST path via mutual TLS would silently fail on `/agents/dynamic/run` and similar SSE endpoints. 2. Document `data` and the new `timeout` parameter in the `stream()` docstring, including the SSE-specific note that the default of `None` disables timeouts because SSE streams can have long idle periods. 3. Migrate unit tests off `httpx` mocks: - `tests/unit/metadata/ingestion/ometa/test_sse_client.py` and `tests/unit/test_user_agent.py` now mock `requests.Session.request` instead of `httpx.Client.stream`. - `MockHTTPXClient` is replaced by `MockRequestsSession` that accepts the SDK's full kwargs set (method/url/headers/json/params/ stream/timeout/verify/allow_redirects/cookies/cert). - `MockSSEResponse.iter_lines` now accepts `decode_unicode` to match the call shape of the new code. - HTTP error / connection error fixtures use `requests.exceptions`. Not adopting Copilot's suggestion to fall back to `self.config.timeout` when the stream() arg is `None`: the REST client's default is `(10, 300)`, which would silently abort SSE streams that have legitimate idle gaps longer than 5 minutes (LLM "thinking" pauses, sparse update events). Keeping the SSE default at `None` (no timeout) preserves prior behavior and matches the working-everywhere baseline; callers can opt in. All 56 tests in the two updated files pass. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
1 parent 284e649 commit b57abf1

3 files changed

Lines changed: 109 additions & 104 deletions

File tree

ingestion/src/metadata/ingestion/ometa/sse_client.py

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,13 @@ def stream(
4848
Args:
4949
method (str): The HTTP method to use.
5050
path (str): The path to the SSE stream.
51+
data (dict | None): Request body sent as JSON for non-GET methods, or as
52+
query parameters for GET. Defaults to None (no body / no params).
53+
timeout (float | tuple[float, float] | None): Per-call timeout passed to
54+
``requests``. ``None`` (the default) disables timeouts, which matches
55+
SSE semantics where streams can have long idle periods between events.
56+
Pass a single float to set both connect and read timeouts, or a
57+
``(connect, read)`` tuple to set them independently.
5158
5259
Returns:
5360
Generator[Any, Any, None]: A generator of events.
@@ -106,6 +113,7 @@ def stream(
106113
self.config.allow_redirects if self.config.allow_redirects is not None else True
107114
),
108115
"cookies": self.config.cookies,
116+
"cert": self.config.cert,
109117
}
110118
with requests.Session() as session, session.request(**request_kwargs) as response:
111119
response.raise_for_status()

0 commit comments

Comments
 (0)