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

refactor: Split WebSocket support into separate sync/async implementations #449

Open
wants to merge 19 commits into
base: main
Choose a base branch
from

Conversation

dolfies
Copy link
Contributor

@dolfies dolfies commented Dec 1, 2024

This is a continuation of #206, which went stale after I got quite busy. Addresses #202 WebSocket requests (fixes #202, fixes #270).

  • Separates async support into AsyncWebSocket
  • Removes Session.ws_connect() and allows initialization via WebSocket() constructor directly
    • This is because WebSockets need their own curl handle, as making a request will break the CurlOpt.CONNECT_ONLY value, so the session becomes useless if a WebSocket is created from it
    • This is not an issue with AsyncSession.connect() as it already maintains multiple curl handles, though a curl handle is unusable after a WebSocket uses it anyway
  • Changes WebSocket API to more closely match websockets
  • Changes AsyncWebSocket API to more closely match aiohttp
    • Removes sync-style callback handlers
    • Adds send/recv_str/json helper methods
  • Adds on_data callback to WebSocket
  • Adds autoclose and skip_utf8_validation parameters, and close_code and close_reason attributes
  • Adds iteration support

@dolfies dolfies changed the title Refactor WebSocket support into separate sync/async implementations refactor: Split WebSocket support into separate sync/async implementations Dec 1, 2024
@lexiforest
Copy link
Owner

Looks good, please let me know when the PR is ready for review.

@dolfies
Copy link
Contributor Author

dolfies commented Dec 2, 2024

Looks good, please let me know when the PR is ready for review.

Should be ready!

@lexiforest
Copy link
Owner

Could you please update the websocket section on README and the websocket examples in examples folder?

@dolfies
Copy link
Contributor Author

dolfies commented Dec 2, 2024

Done

curl_cffi/requests/websockets.py Show resolved Hide resolved
curl_cffi/requests/websockets.py Show resolved Hide resolved
curl_cffi/curl.py Show resolved Hide resolved
curl_cffi/curl.py Show resolved Hide resolved
curl_cffi/requests/websockets.py Outdated Show resolved Hide resolved
curl_cffi/requests/session.py Show resolved Hide resolved
curl_cffi/requests/websockets.py Outdated Show resolved Hide resolved
curl_cffi/requests/websockets.py Show resolved Hide resolved
@lexiforest
Copy link
Owner

LGTM, I will have a few trivial API adjustment and merge.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants