Remove websocket dependency and replace it with async bridging code to tokio-tungstenite
#146
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This introduces tokio and tokio_tungstenite as a dependency. The asynchronous socket supports the same interface as the normal socket with just async methods. The only way to execute it is with the tokio runtime.
This commit removes the Websocket dependency completly. Apparently the library was not maintained anymore and introduced an old hyper version with a security vulnerability. The new version uses async bridging with tokio in order to execute the futures produced by the async client.
1c3t3a
force-pushed
the
remove-websocket
branch
from
2304006 to
e73c9e4
Compare
|
Force pushed due to commit rewording. |
Codecov Report
@@ Coverage Diff @@
## main #146 +/- ##
==========================================
+ Coverage 87.83% 88.26% +0.43%
==========================================
Files 20 24 +4
Lines 1644 1662 +18
==========================================
+ Hits 1444 1467 +23
+ Misses 200 195 -5
Continue to review full report at Codecov.
|
ctrlaltf24
reviewed
Jan 25, 2022
1c3t3a
force-pushed
the
remove-websocket
branch
from
c149094 to
9a57f7d
Compare
ctrlaltf24
reviewed
Jan 26, 2022
ctrlaltf24
approved these changes
Jan 27, 2022
|
Thanks a lot for the review @nshaaban-cPacket! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR replaces the current websocket code that uses the unmaintained
websocketcrate withtokio-tungstenite.tokio-tungsteniteis async, so in order to get the current code to work I needed to use async-bridging. The idea is that the current websocket transport types hold atokio::Runtimeobject where they spawn the futures with and then communicate the results like before.This fixes #115 and does the work of pr #123.
The changes introduce tokio as an async runtime. The current version of the async clients are bound to tokio and it is apparently quite hard to not choose an async runtime, as the underlying connection with the server is currently opened by this crate and thus needs some sort of type (either tokio or async-std streams).