stream: add `WatchStream::from_changes`, add testing, etc. #5432

brodycj · 2023-02-06T19:06:43Z

other minor updates:

add a test case for existing WatchStream::from
update existing WatchStream test function name
apply minor documentation updates

resolves #5420

Motivation

It looks like WatchStream::from was not covered by documentation or integration test cases, adding one here as well.

Quick disclaimer that I am still quite new with Rust in contrast to my years in quite a few other major systems programming languages.

Solution

added WatchStream::from_changes, with implementation as suggested in WatchStream: Add additional constructor which causes stream to only yield when value changes #5420, along with a test case and minor documentation updates

add a test case for existing `WatchStream::from` update existing WatchStream test function name apply minor documentation updates

brodycj · 2023-02-06T19:15:45Z

tokio-stream/src/wrappers/watch.rs

@@ -66,6 +67,13 @@ impl<T: 'static + Clone + Send + Sync> WatchStream<T> {
            inner: ReusableBoxFuture::new(async move { (Ok(()), rx) }),
        }
    }
+
+    /// Create a new `WatchStream` that waits for the value to be changed.
+    pub fn new_on_changed(rx: Receiver<T>) -> Self {


I would be open to changing this function name. I tried to come up with something consistent with the Receiver::changed function that we await for, but doesn't feel as pretty as I would like.

I thought of from_changes and new_from_changes, but do not feel as consistent.

How about new_skip_unchanged? Or new_skip_until_changed?

I think new_on_changed is fine.

tokio-stream/tests/watch.rs

…atch-stream-on-changed

tokio-stream/tests/watch.rs

tokio-stream/src/wrappers/watch.rs

…atch-stream-on-changed

tokio-stream/src/wrappers/watch.rs

brodycj · 2023-02-13T04:56:01Z

I have simplified the test with help from the tokio-test utility and have added an example to the documentation.

I have also aded a test to the documentation to assert that the watch stream is still pending until new data is available. If we keep this, it may be enough overlap to remove the test case I added.

…atch-stream-on-changed

brodycj · 2023-02-14T20:51:24Z

I think this should be better now, please let me know if anything else is needed from my end to get this accepted and merged.

I think there is an overlap between the watch_stream_new_on_change test case and the documentation example that I have added with a poll test. I would prefer to keep the documentation with the poll test that I made but can remove the poll test from the documentation if needed.

…atch-stream-on-changed

Darksonn

Other than this comment, it looks good to me.

tokio-stream/src/wrappers/watch.rs

Co-authored-by: Alice Ryhl <[email protected]>

…atch-stream-on-changed

Darksonn · 2023-02-19T13:18:47Z

tokio-stream/Cargo.toml

+futures-test = "0.3.26"
+futures-util = "0.3.26"


I don't think we need these. We already import futures.

Note that FutureExt is available from the futures crate.

I removed futures-test. ~~Unfortunately I still need futures-util to import FutureExt as I needed to get now_or_never() to work.~~

I noticed that tokio/src/sync/watch.rs is using recv.changed().now_or_never() from crate::sync::watch::channel(0i32) in some tests. I would like to investigate this a little further. Any pointers would still be highly appreciated as before.

futures::future::FutureExt worked for me thanks!

…atch-stream-on-changed

Darksonn

Thanks.

brodycj added 2 commits February 6, 2023 13:50

stream: add WatchStream::new_on_changed, add testing, etc.

afc73fb

add a test case for existing `WatchStream::from` update existing WatchStream test function name apply minor documentation updates

FIXUP: remove extra parens in test case

83888af

brodycj commented Feb 6, 2023

View reviewed changes

brodycj mentioned this pull request Feb 6, 2023

WatchStream: Add additional constructor which causes stream to only yield when value changes #5420

Closed

Darksonn added A-tokio Area: The main tokio crate M-sync Module: tokio/sync labels Feb 8, 2023

Darksonn reviewed Feb 9, 2023

View reviewed changes

tokio-stream/tests/watch.rs Outdated Show resolved Hide resolved

brodycj marked this pull request as draft February 9, 2023 20:22

brodycj added 5 commits February 9, 2023 15:58

Merge branch 'master' of https://github.com/tokio-rs/tokio into new-w…

f988a46

…atch-stream-on-changed

Merge branch 'master' of https://github.com/tokio-rs/tokio into new-w…

a9482cd

…atch-stream-on-changed

rework test

bdc09c0

fix imports & fmt

88d2feb

fix an import again

48bbfd4

brodycj commented Feb 10, 2023

View reviewed changes

tokio-stream/tests/watch.rs Outdated Show resolved Hide resolved

brodycj marked this pull request as ready for review February 10, 2023 18:12

Darksonn reviewed Feb 11, 2023

View reviewed changes

tokio-stream/src/wrappers/watch.rs Outdated Show resolved Hide resolved

Merge branch 'master' of https://github.com/tokio-rs/tokio into new-w…

048bd05

…atch-stream-on-changed

brodycj marked this pull request as draft February 12, 2023 21:48

brodycj added 3 commits February 12, 2023 16:50

simplify test with utility from tokio-test

b8c31ea

add example with new_on_changed & add comment to another example

6ca8990

add another test to the example

2db6ccb

brodycj commented Feb 13, 2023

View reviewed changes

tokio-stream/src/wrappers/watch.rs Outdated Show resolved Hide resolved

brodycj marked this pull request as ready for review February 13, 2023 04:52

brodycj added 2 commits February 14, 2023 15:40

Merge branch 'master' of https://github.com/tokio-rs/tokio into new-w…

edc4fde

…atch-stream-on-changed

update sample comment & first_poll variable name

1ce7bbb

brodycj added 2 commits February 15, 2023 18:53

Merge branch 'master' of https://github.com/tokio-rs/tokio into new-w…

eef3db9

…atch-stream-on-changed

sample poll test down to 1 line & update comment

95870cd

Darksonn reviewed Feb 16, 2023

View reviewed changes

tokio-stream/src/wrappers/watch.rs Outdated Show resolved Hide resolved

use now_or_never (needs futures_util::future::FutureExt)

63e276a

Co-authored-by: Alice Ryhl <[email protected]>

brodycj marked this pull request as draft February 17, 2023 16:47

brodycj added 2 commits February 17, 2023 11:49

update imports

f413e2a

rename function to from_changes

67d4c87

brodycj changed the title ~~stream: add WatchStream::new_on_changed, add testing, etc.~~ stream: add WatchStream:: from_changes, add testing, etc. Feb 17, 2023

brodycj changed the title ~~stream: add WatchStream:: from_changes, add testing, etc.~~ stream: add WatchStream::from_changes, add testing, etc. Feb 17, 2023

Merge branch 'master' of https://github.com/tokio-rs/tokio into new-w…

28c2040

…atch-stream-on-changed

brodycj marked this pull request as ready for review February 17, 2023 17:26

Darksonn reviewed Feb 19, 2023

View reviewed changes

brodycj added 2 commits February 19, 2023 08:34

Merge branch 'master' of https://github.com/tokio-rs/tokio into new-w…

0da26da

…atch-stream-on-changed

remove futures-test not needed from dev dependencies

f1ddac9

brodycj marked this pull request as draft February 19, 2023 13:48

fix sample import & remove futures-util from dev dependencies

7d0f3c5

brodycj marked this pull request as ready for review February 19, 2023 14:27

Darksonn approved these changes Feb 19, 2023

View reviewed changes

Darksonn merged commit d07027f into tokio-rs:master Feb 19, 2023

brodycj deleted the new-watch-stream-on-changed branch February 19, 2023 15:25

satakuma mentioned this pull request Feb 20, 2023

stream: fix changelog date #5488

Merged

amab8901 pushed a commit to amab8901/tokio that referenced this pull request Feb 27, 2023

sync: add WatchStream::from_changes (tokio-rs#5432)

552fa7e

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

stream: add `WatchStream::from_changes`, add testing, etc. #5432

stream: add `WatchStream::from_changes`, add testing, etc. #5432

brodycj commented Feb 6, 2023 •

edited

Loading

brodycj Feb 6, 2023

extremeandy Feb 11, 2023

Darksonn Feb 11, 2023

brodycj commented Feb 13, 2023

brodycj commented Feb 14, 2023

Darksonn left a comment

Darksonn Feb 19, 2023

Darksonn Feb 19, 2023

brodycj Feb 19, 2023 •

edited

Loading

brodycj Feb 19, 2023

Darksonn left a comment

stream: add WatchStream::from_changes, add testing, etc. #5432

stream: add WatchStream::from_changes, add testing, etc. #5432

Conversation

brodycj commented Feb 6, 2023 • edited Loading

Motivation

Solution

brodycj Feb 6, 2023

Choose a reason for hiding this comment

extremeandy Feb 11, 2023

Choose a reason for hiding this comment

Darksonn Feb 11, 2023

Choose a reason for hiding this comment

brodycj commented Feb 13, 2023

brodycj commented Feb 14, 2023

Darksonn left a comment

Choose a reason for hiding this comment

Darksonn Feb 19, 2023

Choose a reason for hiding this comment

Darksonn Feb 19, 2023

Choose a reason for hiding this comment

brodycj Feb 19, 2023 • edited Loading

Choose a reason for hiding this comment

brodycj Feb 19, 2023

Choose a reason for hiding this comment

Darksonn left a comment

Choose a reason for hiding this comment

stream: add `WatchStream::from_changes`, add testing, etc. #5432

stream: add `WatchStream::from_changes`, add testing, etc. #5432

brodycj commented Feb 6, 2023 •

edited

Loading

brodycj Feb 19, 2023 •

edited

Loading