-
Notifications
You must be signed in to change notification settings - Fork 23
Add async Websocket client #212
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
base: dev
Are you sure you want to change the base?
Conversation
b3ec024
to
8419b64
Compare
8419b64
to
7eb4bd0
Compare
7eb4bd0
to
e1e5f13
Compare
4130964
to
ca5995f
Compare
1f827fb
to
4e72bd2
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks really awesome! Thank you so much!!!
That just leaves the last thing to do which is to add tests for each of the async websocket methods.
I'm going to convert this to a draft PR, and then you'll convert it back when its ready to be reviewed and merged for real. |
dab88ba
to
e7310c9
Compare
e7310c9
to
5d767bc
Compare
That should be it. I can't see the coverage, but hopefully I've covered everything. |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## dev #212 +/- ##
==========================================
- Coverage 97.44% 96.49% -0.96%
==========================================
Files 25 27 +2
Lines 1647 1883 +236
==========================================
+ Hits 1605 1817 +212
- Misses 42 66 +24 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
I finally figured out why. Codecov recently changed their behavior so that github actions running on forks now require a |
@mj23000 I am happy to aid you in developing the tests we need to increase the coverage. There are some parts of the codebase that need some clever ideas to test them appropriately. |
Yes that sounds good! |
This async version of the Websocket client is mostly just an async version of the existing client, though with the addition of a
RawBaseWebsocketClient
that is inherited from by both clients.No clever handling of the client type has been implemented as with the REST API client withuse_async
. This does have the benefit of not being a breaking change.A similar structure to the existing REST API client has been implemented.
Commits will be cleaned up when the PR is more ready to be merged.