-
Notifications
You must be signed in to change notification settings - Fork 120
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
Use broadcast::Receiver::recv
instead of next
#2933
Conversation
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.
broadcast::Receiver::recv
exists in tokio 0.3.7:
https://docs.rs/tokio/0.3.7/tokio/sync/broadcast/struct.Receiver.html#method.recv
So we can merge this low-risk PR before the beta release if you want.
On newer versions of Tokio the `Receiver` doesn't implement `Stream`.
9da067d
to
ce54247
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.
Approving on behalf of @teor2345 as per #2933 (review)
I just moved this to draft since from the description it seems it can't be merged by itself. @jvff is this correct? |
Ah sorry, I just saw @teor2345 's comment 😅 |
Motivation
This is part of the update to use Tokio version 1 (#2200), and must be merged together with the other PRs.
On newer versions of Tokio the
broadcast::Receiver
doesn't implementStream
, so theStreamExt::next
method used intower-batch
is no longer available.Solution
Instead of wrapping the
broadcast::Receiver
in atokio_stream::wrappers::BroadcastStream
like in #2922, in this case a simpler solution was used, simply replacing the usage ofStreamExt::next()
withbroadcast::Receiver::recv()
.Review
Anyone from the team can review this, but it shouldn't be merged yet.
Reviewer Checklist
Follow Up Work