-
Notifications
You must be signed in to change notification settings - Fork 57
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
Properly await for all futures in Future for TelemetryDriver #88
base: main
Are you sure you want to change the base?
Conversation
071b77c
to
a54bbb5
Compare
if let Poll::Ready(res) = Pin::new(server_fut).poll(cx) { | ||
ready_res.push(res.map_err(|err| anyhow!(err))); | ||
if let Poll::Ready(()) = Pin::new(server_fut).poll(cx)? { | ||
self.server_fut = None; |
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.
doesn't it mean we'll poll the server just once?
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.
No, this checks for Poll::Ready(())
, so it will be polled until it's ready.
a54bbb5
to
a8e7f01
Compare
a8e7f01
to
0a6d3ba
Compare
} | ||
} | ||
|
||
let tele_futures_poll = Pin::new(&mut self.tele_futures).poll_next(cx); | ||
ready!(Pin::new(&mut self.tele_futures).poll_next(cx)?); |
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.
technically we should call self.tele_futures.is_terminated()
first, but FuturesUnordered
would be hard to work with if you couldn't call poll_next
as many times as you like
No description provided.