feat: Add AsyncSSEClient with aiohttp-based async/await support#58
feat: Add AsyncSSEClient with aiohttp-based async/await support#58
Conversation
Adds AsyncSSEClient as a purely additive new public API alongside the existing SSEClient. Async users install with the [async] extra to get aiohttp; sync users have no new dependencies. All existing tests pass unchanged. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Removes make_stream, retry_for_status, and no_delay from async_helpers.py as they were either unused or duplicates of the same functions in helpers.py. Updates async test files to import these from helpers.py consistently. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
|
|
||
| request_options = {} | ||
| if self.options.get("readTimeoutMs") is not None: | ||
| request_options["timeout"] = aiohttp.ClientTimeout( |
There was a problem hiding this comment.
I think, even without a read timeout, we need to customize the ClientTimeout.
The timeout uses a non-default ClientTimeout.
But if we do set it to anything, then the total will get set to None. https://docs.aiohttp.org/en/stable/client_reference.html#aiohttp.ClientTimeout.total
But if we set a ClientTimeout, we would get the default total=None.
| error = e | ||
| self.__connection_result = None | ||
| finally: | ||
| self.__last_event_id = reader.last_event_id |
There was a problem hiding this comment.
Should we close the result in finally out of an abundance of caution? By the docs you normally don't need to close it if it completes normally, and it would be closed on a network error, but the exception here here could also potentially be a decoding error, in which maybe we wouldn't close it?
|
|
||
| async def close(self): | ||
| # Only close the session if we created it ourselves | ||
| if self.__external_session is None and self.__session is not None: |
There was a problem hiding this comment.
I think the non-async version does this wrong?
python-eventsource/ld_eventsource/http.py
Line 60 in d086a6a
self.__should_close_pool = params.pool is not None
Not a problem with this PR, but maybe worth checking.
keelerm84
force-pushed
the
mk/sdk-1400/async
branch
from
525f9a4 to
aa5e52d
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
2 participants
Adds AsyncSSEClient as a purely additive new public API alongside the
existing SSEClient. Async users install with the [async] extra to get
aiohttp; sync users have no new dependencies. All existing tests pass
unchanged.
Note
Medium Risk
Adds a new async public client and optional
aiohttpdependency plus new CI/contract-test coverage; while largely additive, it introduces new concurrency and connection-handling code paths that could surface runtime issues for async users.Overview
Adds a new
AsyncSSEClientAPI for consuming SSE streams withasyncio, including async parsing/iteration, retry/backoff handling, and anAsyncConnectStrategy.http()implementation backed by optionalaiohttp.Updates packaging/docs to expose the async client without forcing
aiohttpon sync users (lazy import viald_eventsource.__getattr__, newasyncextra, Sphinx mocking), and standardizes header typing via a newHeadersalias used across sync/async actions and errors.Extends test/CI coverage with
pytest-asyncio, new unit tests for async reader/client/HTTP strategy, and a separate async contract-test service and CI steps/Makefile targets to run the shared contract test harness against the async implementation.Written by Cursor Bugbot for commit cda95c0. This will update automatically on new commits. Configure here.