Skip to content

Add stream timeout #348

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

Merged
merged 9 commits into from
Oct 28, 2019
Merged

Add stream timeout #348

merged 9 commits into from
Oct 28, 2019

Conversation

k-nasa
Copy link
Member

@k-nasa k-nasa commented Oct 16, 2019

close: #340

Currently, the process that times out cannot be tested.
Is there any stream processing that takes time easily?

#[cfg_attr(feature = "docs", doc(cfg(unstable)))]
#[cfg(any(feature = "unstable", feature = "docs"))]
#[derive(Clone, Copy, Debug, Eq, PartialEq)]
pub struct TimeoutError;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we export this too?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, this type should not be constructible by end-users. To do prevent users from being able to create instances of TimeoutError, do:

pub struct TimeoutError {
    _private: (),
}

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I fixed it. b6edd12

@k-nasa k-nasa force-pushed the add_stream_timeout branch from b6edd12 to 5ef5e5b Compare October 16, 2019 11:01
@k-nasa k-nasa force-pushed the add_stream_timeout branch from 5ef5e5b to 10f32ca Compare October 16, 2019 11:21
ghost
ghost previously requested changes Oct 16, 2019
"#]
#[cfg(any(feature = "unstable", feature = "docs"))]
#[cfg_attr(feature = "docs", doc(cfg(unstable)))]
fn timeout(self, dur: Duration) -> TimeoutStream<Self>
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't add the "Stream" suffix to stream types so this should be just Timeout<Self>. See other similar stream types for reference (e.g. Empty, Repeat, Chain).

@k-nasa
Copy link
Member Author

k-nasa commented Oct 17, 2019

I fixed it.
(The difference by the formatter was born.)

@k-nasa k-nasa requested review from yoshuawuyts and a user October 17, 2019 09:22
@k-nasa
Copy link
Member Author

k-nasa commented Oct 23, 2019

@yoshuawuyts @stjepang
Fixed it and rebased the master branch.
Can you review it again?

Copy link
Contributor

@yoshuawuyts yoshuawuyts left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is great. Thanks so much!

@yoshuawuyts yoshuawuyts merged commit fd940b8 into async-rs:master Oct 28, 2019
@k-nasa k-nasa deleted the add_stream_timeout branch October 28, 2019 12:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add Stream::timeout
2 participants