Skip to content
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

factor out HTTP logic with ConnectStrategy abstraction #6

Merged
merged 19 commits into from
Jan 4, 2023

Conversation

eli-darkly
Copy link
Contributor

This is the final set of API changes to make python-eventsource more closely resemble https://github.com/launchdarkly/okhttp-eventsource. The SSEClient class now has no knowledge of any HTTP implementation details; instead, it interacts with some implementation of ConnectStrategy, where the default implementation contains all of the HTTP logic. This is a cleaner design and more flexible, and it allows us to write SSEClient unit tests that don't require real HTTP.

@eli-darkly eli-darkly requested a review from keelerm84 December 30, 2022 22:51
@shortcut-integration
Copy link

This pull request has been linked to Shortcut Story #133323: Break Python SSE client out of the SDK.

@eli-darkly eli-darkly marked this pull request as ready for review December 30, 2022 22:51
# Conflicts:
#	ld_eventsource/__init__.py
#	ld_eventsource/actions.py
#	ld_eventsource/request_params.py
#	ld_eventsource/retry_delay_strategy.py
#	ld_eventsource/sse_client.py
#	testing/test_error_strategy.py
#	testing/test_retry_delay_strategy.py
Base automatically changed from eb/sc-133323/docs-and-modules to main January 4, 2023 18:31
# Conflicts:
#	ld_eventsource/__init__.py
#	ld_eventsource/config/__init__.py
#	ld_eventsource/request_params.py
#	ld_eventsource/sse_client.py
@eli-darkly eli-darkly merged commit c516f1f into main Jan 4, 2023
@eli-darkly eli-darkly deleted the eb/sc-133323/connect-strategy branch January 4, 2023 18:33
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.

2 participants