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.
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
[feat] Add New Binance Websocket Source #11
[feat] Add New Binance Websocket Source #11
Changes from 16 commits
c75f157
b5d1a89
56a24dc
2f01e52
0517535
ee05365
8f19455
bba1526
f00e402
d09a40b
858c4b3
7eaa92f
21e2031
d304e88
f6038c5
8b41fcb
4052ff9
e3d5206
ac49660
c06cb9e
e2337e5
c65e6d8
6451ce1
156d749
8473881
ef206be
6143cbc
4c06bdb
6ef569f
bfac8e0
156d330
36e072b
e423854
4fc5f11
b77f62e
04e1a9a
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
what happens if we try to unscribe symbols that we never subscribe? will we see error anywhere?
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.
This should not be error::unknown, we should propagate whatever error we got from receiver.next
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.
I don't think we should use DashMap. They do not offer async api. Let's use
RwLock<HashMap<..>>
(RwLock fromtokio::sync
)in this specific case the critical path is small so shouldn't matter but still if we are using tokio i think we should use their locking mechanism.
also i prefer we just call this variable
prices
orcached_prices
. the fact that it is a map is already in its typeThere 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.
make the name more descriptive.
cxl
?cancel
?cancel_token
? any of these works for me.