Move stream items into `tokio-stream` #3277

LucioFranco · 2020-12-14T21:11:32Z

This change removes all references to Stream from
within the tokio crate and moves them into a new
tokio-stream crate. Most types have had their
impl Stream removed as well in-favor of their
inherent methods.

Closes #2870

This change removes all references to `Stream` from within the `tokio` crate and moves them into a new `tokio-stream` crate. Most types have had their `impl Stream` removed as well in-favor of their inherent methods. Closes #2870

Darksonn · 2020-12-14T21:26:08Z

examples/chat.rs

-    rx: Rx,
+    rx: Pin<Box<dyn Stream<Item = String> + Send>>,


This raises a question. The Rx type is not used in a way that expects a Stream, rather just in a way that expects something with a poll_next method. Currently the impl Stream is just removed, and the private poll_recv method is not made public.

What approach should we be taking here? I kinda feel like we should be exposing the poll_recv method, as the type supports it perfectly fine (there is only one receiver, so there are no intrusiveness concerns), but then we have both poll_next and poll_recv when we add back the Stream impl in ≥ half a year.

(edit for future readers: a poll_recv method has since been added)

Does this example need to use poll_* APIs at all?

Yeah, I think we should rewrite it.

Darksonn · 2020-12-14T21:38:24Z

You're missing the Stream impl for ReadDir.

tokio-stream/src/iter.rs

Darksonn · 2020-12-14T21:49:22Z

tokio-stream/src/lib.rs

+    ///     // Convert the channels to a `Stream`.
+    ///     let rx1 = Box::pin(async_stream::stream! {
+    ///           while let Some(item) = rx1.recv().await {
+    ///               yield item;
+    ///           }
+    ///     }) as Pin<Box<dyn Stream<Item = usize> + Send>>;


I plan to submit a proposal for how to improve this later.

What would you change?

tokio-stream/src/stream_map.rs

tokio-stream/tests/CONTRIBUTING.md

tokio-stream/src/macros.rs

tokio/src/sync/broadcast.rs

Darksonn · 2020-12-14T22:30:07Z

tokio/tests/rt_basic.rs

+    // TODO: could probably avoid this, but why not.
+    let mut rx = Box::pin(rx);
+
    rt.spawn(async move { while rx.next().await.is_some() {} });


To avoid it, you could use tokio::pin!, but it would need to be inside the spawned task. Not that it matters much as it is a test.

tokio/tests/support/mpsc_stream.rs

Darksonn · 2020-12-14T22:36:03Z

The main thing missing before I approve this is a review of the generated rustdoc, but I can't do this yet as it does not currently compile.

tokio-stream/Cargo.toml

carllerche

Looking good. Nothing major stands out. Left a few comments inline. @Darksonn also covered stuff 👍

tokio-stream/Cargo.toml

tokio-stream/src/macros.rs

tokio-test/src/io.rs

carllerche · 2020-12-15T18:36:23Z

tokio/src/lib.rs

@@ -381,10 +381,6 @@ cfg_signal_internal! {
    pub(crate) mod signal;
 }

-cfg_stream! {
-    pub mod stream;


Question: Should we keep an empty module w/ documentation explaining the stream functionality will return once Stream stabilizes in 1.0 and tokio-stream is the current work around? This way, searching for stream in the API docs will find something.

That sounds like a good idea.

We can follow up.

tokio/tests/support/mpsc_stream.rs

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

tokio-stream/src/timeout.rs

Darksonn

We might want to coordinate between the text in the tokio::stream module and in tokio-stream/src/lib.rs. Doesn't have to be this PR though.

Darksonn · 2020-12-15T22:27:27Z

tokio-stream/src/lib.rs

-//! [`tokio::io`]: crate::io
-//! [`AsyncRead`]: crate::io::AsyncRead
-//! [`AsyncWrite`]: crate::io::AsyncWrite
+//! [tokio-util]: https://docs.rs/tokio-util/0.4/tokio_util/codec/index.html


The link is broken.

Suggested change

//! [tokio-util]: https://docs.rs/tokio-util/0.4/tokio_util/codec/index.html

//! [`tokio-util`]: https://docs.rs/tokio-util/0.4/tokio_util/codec/index.html

Darksonn · 2020-12-15T22:27:56Z

tokio-stream/src/lib.rs

+    attr(deny(warnings, rust_2018_idioms), allow(dead_code, unused_variables))
+))]
+#![cfg_attr(docsrs, feature(doc_cfg))]
+
 //! Stream utilities for Tokio.
 //!
 //! A `Stream` is an asynchronous sequence of values. It can be thought of as


Below this line (can't select further down), the crate calls itself a module.

Darksonn · 2020-12-15T22:29:14Z

tokio-stream/src/stream_map.rs

+/// [`mpsc`]: https://docs.rs/tokio/1.0/tokio/sync/mpsc/index.html
+/// [`pin!`]: https://docs.rs/tokio/1.0/tokio/macro.pin.html


These should work as intra-doc links, no? The tokio-stream crates does depend on tokio.

carllerche · 2020-12-16T04:24:21Z

@Darksonn had some additional comments, but they can be addressed later (even post 1.0). As this PR is blocking others, I am going to merge it 👍. Thanks!

jonhoo · 2021-01-05T22:14:05Z

I think @Darksonn's comment from #3277 (comment) was lost -- we're missing an implementation for fs::ReadDir.

Darksonn · 2021-01-05T23:08:45Z

Nah, it's there: https://github.com/tokio-rs/tokio/blob/master/tokio-stream/src/wrappers/read_dir.rs

The docs on docs.rs are just generated wrong in the current version.

Move stream items into tokio-stream

42af6ad

This change removes all references to `Stream` from within the `tokio` crate and moves them into a new `tokio-stream` crate. Most types have had their `impl Stream` removed as well in-favor of their inherent methods. Closes #2870

LucioFranco added this to the v1.0 milestone Dec 14, 2020

LucioFranco requested review from carllerche, Darksonn and a team December 14, 2020 21:11

Darksonn reviewed Dec 14, 2020

View reviewed changes

Darksonn added the A-tokio-stream Area: The tokio-stream crate label Dec 14, 2020

Darksonn reviewed Dec 14, 2020

View reviewed changes

Darksonn mentioned this pull request Dec 15, 2020

time: remove Box from Sleep #3278

Merged

carllerche reviewed Dec 15, 2020

View reviewed changes

tokio-stream/Cargo.toml Show resolved Hide resolved

carllerche reviewed Dec 15, 2020

View reviewed changes

carllerche mentioned this pull request Dec 15, 2020

io: remove Async{Read,Write}Ext #3005

Closed

LucioFranco mentioned this pull request Dec 15, 2020

stream: Add coop back to stream::iter #3280

Open

LucioFranco and others added 5 commits December 15, 2020 15:20

Apply suggestions from code review

9928f5e

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

Address comments

6169dab

Fix bench code

97f13c1

Remove signal stream windows impl

67e2d7f

Make windows happy

0574b0d

Darksonn reviewed Dec 15, 2020

View reviewed changes

tokio-stream/src/timeout.rs Show resolved Hide resolved

LucioFranco added 3 commits December 15, 2020 21:38

Fix docs

9f66ecf

fix rustfmt...

472f5a0

Add crate attributes to tokio-stream

23986d8

Darksonn reviewed Dec 15, 2020

View reviewed changes

Darksonn approved these changes Dec 15, 2020

View reviewed changes

carllerche merged commit 8efa620 into master Dec 16, 2020

carllerche deleted the lucio/remove-stream branch December 16, 2020 04:24

Darksonn mentioned this pull request Dec 17, 2020

Address comments from tokio-stream PR #3284

Closed

LucioFranco mentioned this pull request Dec 17, 2020

stream: Fix a few doc issues #3285

Merged

Darksonn mentioned this pull request Dec 22, 2020

Expose poll_next replacement on Split, Lines, DirEntry, Receiver and UnboundedReceiver #3308

Merged

Darksonn mentioned this pull request Mar 6, 2021

Rewrite chat example to not manually implement Stream #3586

Closed

brody4hire mentioned this pull request Feb 6, 2023

tokio::signal::unix::Signal and the Stream trait #5430

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Move stream items into `tokio-stream` #3277

Move stream items into `tokio-stream` #3277

LucioFranco commented Dec 14, 2020

Darksonn Dec 14, 2020 •

edited

Loading

carllerche Dec 15, 2020

LucioFranco Dec 15, 2020

Darksonn commented Dec 14, 2020

Darksonn Dec 14, 2020

LucioFranco Dec 15, 2020

Darksonn Dec 14, 2020

Darksonn commented Dec 14, 2020

carllerche left a comment

carllerche Dec 15, 2020

Darksonn Dec 15, 2020

carllerche Dec 16, 2020

Darksonn left a comment

Darksonn Dec 15, 2020

Darksonn Dec 15, 2020

Darksonn Dec 15, 2020

carllerche commented Dec 16, 2020

jonhoo commented Jan 5, 2021

Darksonn commented Jan 5, 2021

	//! [tokio-util]: https://docs.rs/tokio-util/0.4/tokio_util/codec/index.html
	//! [`tokio-util`]: https://docs.rs/tokio-util/0.4/tokio_util/codec/index.html

		/// [`mpsc`]: https://docs.rs/tokio/1.0/tokio/sync/mpsc/index.html
		/// [`pin!`]: https://docs.rs/tokio/1.0/tokio/macro.pin.html

Move stream items into tokio-stream #3277

Move stream items into tokio-stream #3277

Conversation

LucioFranco commented Dec 14, 2020

Darksonn Dec 14, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Darksonn commented Dec 14, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Darksonn commented Dec 14, 2020

carllerche left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Darksonn left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

carllerche commented Dec 16, 2020

jonhoo commented Jan 5, 2021

Darksonn commented Jan 5, 2021

Move stream items into `tokio-stream` #3277

Move stream items into `tokio-stream` #3277

Darksonn Dec 14, 2020 •

edited

Loading