Upgrade to Tokio 1.0 #753

aknuds1 · 2020-11-29T15:17:40Z

Similar to @Urhengulas' PR #741, although more complete, this PR aims to upgrade to Tokio v1.0.

I can successfully run all tests with all features enabled: cargo test --all-features.

This is my first time trying to contribute to warp, and I'm not super familiar with the internals, so please consider it a best effort :)

Fixes #725.

Signed-off-by: Arve Knudsen <arve.knudsen@gmail.com>

Urhengulas

Good job! Only two remarks :D

Cargo.toml

src/filters/fs.rs

src/filters/ws.rs

…tokio Signed-off-by: Arve Knudsen <arve.knudsen@gmail.com>

Cargo.toml

Signed-off-by: Arve Knudsen <arve.knudsen@gmail.com>

jhpratt · 2020-12-24T01:03:26Z

Tokio 1.0 has been released, so this PR should probably be updated to depend on that.

aknuds1 · 2020-12-24T09:43:15Z

I'm trying to upgrade to Tokio 1.0.

Signed-off-by: Arve Knudsen <arve.knudsen@gmail.com>

aknuds1 · 2020-12-24T10:31:20Z

The remaining issue is tokio-tungstenite. Trying to upgrade it to tokio 1.0, but it's easier said than done.

Signed-off-by: Arve Knudsen <arve.knudsen@gmail.com>

aknuds1 · 2020-12-24T10:37:26Z

Forked tokio-tungstenite, to upgrade it to tokio 1, and depending on it, but build still fails.

paolobarbolini · 2020-12-24T10:45:16Z

async-compression using bytes 1.0 should come out soon Nullus157/async-compression#111 (comment)

but build still fails

Sleep is now !Unpin
bytes_mut is now chunk_mut on BytesMut
bytes::buf::{Buf, BufMut} -> bytes::{Buf, BufMut} (it's top-level now)

aknuds1 · 2020-12-24T11:24:47Z

If anyone has tips wrt. fixing the build failures, I'm all ears :)

Signed-off-by: Arve Knudsen <arve.knudsen@gmail.com>

Cargo.toml

Signed-off-by: Arve Knudsen <arve.knudsen@gmail.com>

src/filters/body.rs

Co-authored-by: Paolo Barbolini <paolo@paolo565.org>

Signed-off-by: Arve Knudsen <arve.knudsen@gmail.com>

jxs · 2021-01-09T18:36:43Z

src/filters/compression.rs

@@ -56,7 +58,10 @@ pub struct Compression<F> {
 /// ```
 pub fn gzip() -> Compression<impl Fn(CompressionProps) -> Response + Copy> {
    let func = move |mut props: CompressionProps| {
-        let body = Body::wrap_stream(GzipEncoder::new(props.body));
+        let body = Body::wrap_stream(FramedRead::new(


why did you prefer FramedRead over ReaderStream?

@jxs To be honest, I don't remember exactly, but it was probably from an example. Do you think ReaderStream is preferable?

It was inspired from the reqwest implementation https://github.com/seanmonstar/reqwest/blob/a19eb3419637856d6d42f55dff6e4790613b2933/src/async_impl/decoder.rs#L47

Thanks @paolobarbolini, I can't bring myself to remember where I found this solution.

thanks @paolobarbolini. @aknuds1 I asked because I was not sure either, Previously FrameReader was the way to achieve AsyncRead to Stream conversion, but since ReaderStream was introduced I would maybe suggest ReaderStream as it's simpler, and recommended for this cases on tokio-util doc

@seanmonstar Do you have any opinion on this?

I converted from FramedRead into ReaderStream, ass @jxs recommended. I don't really know personally if there are any practical differences, but I can tell that the API is simpler (a second argument to FramedRead::new can be dropped). I trust @jxs knows more about this than I do, and I'll let @seanmonstar be the judge.

jxs

LGTM thanks! 🎆

Test (uds, --features tokio/uds) also needs to be renamed on the repository settings
should the tokio_util dependent filters (fs, compression) also be featured gated?

Signed-off-by: Arve Knudsen <arve.knudsen@gmail.com>

seanmonstar

Excellent work! I believe this is very close.

.github/workflows/ci.yml

Cargo.toml

seanmonstar · 2021-01-12T01:17:52Z

src/filters/ws.rs

-            .on_upgrade()
+        let mut resp = Response::new(self.ws.body);
+        if let Some(on_upgrade) = self.ws.on_upgrade {
+            resp.extensions_mut().insert(on_upgrade);


If you already have the extension, you don't need to insert it here to remove it again. (on_upgrade is the same as fut).

@seanmonstar I think I see what you mean, and try to simplify it to the best of my ability. Please have a look.

…tokio Signed-off-by: Arve Knudsen <arve.knudsen@gmail.com>

Signed-off-by: Arve Knudsen <arve.knudsen@gmail.com>

koivunej · 2021-01-12T14:35:01Z

Should the return type of warp::body::stream() now have a stream of Bytes instead of impl Buf in: https://github.com/aknuds1/warp/blob/bb2f511fc8785784089ebdb12861bbc32e4cef5d/src/filters/body.rs#L77-L78

This following ... Bytes change tokio-rs/bytes#439, which removed Buf::to_bytes and added Buf::copy_to_bytes. Well, I guess the buf.copy_to_bytes(buf.remaining()) should work just as well.

EDIT: found the change

paolobarbolini · 2021-01-12T14:38:47Z

This following ... some Bytes change I cannot track down, which removed Buf::to_bytes.

It was replaced by Buf::copy_to_bytes @koivunej

Kiskae · 2021-01-12T15:02:25Z

It appears 657f1c6 broke the build since warp::test::handshake uses hyper::Client:

   Compiling warp v0.2.5 (https://github.com/aknuds1/warp?branch=chore/upgrade-tokio#bb2f511f)
error[E0433]: failed to resolve: could not find `Client` in `hyper`
   --> /home/x/.cargo/git/checkouts/warp-2008e9c17b453427/bb2f511/src/test.rs:518:36
    |
518 |             let upgrade = ::hyper::Client::builder()
    |                                    ^^^^^^ could not find `Client` in `hyper`

error: aborting due to previous error

aknuds1 · 2021-01-12T15:04:24Z

It appears 657f1c6 broke the build since warp::test::handshake uses hyper::Client:

   Compiling warp v0.2.5 (https://github.com/aknuds1/warp?branch=chore/upgrade-tokio#bb2f511f)
error[E0433]: failed to resolve: could not find `Client` in `hyper`
   --> /home/x/.cargo/git/checkouts/warp-2008e9c17b453427/bb2f511/src/test.rs:518:36
    |
518 |             let upgrade = ::hyper::Client::builder()
    |                                    ^^^^^^ could not find `Client` in `hyper`

error: aborting due to previous error

@Kiskae What's the command to reproduce your failure?

Kiskae · 2021-01-12T15:09:05Z

@aknuds1 Calling cargo build with the branch as a dependency seems to trigger it:

[package]
name = "repro"
version = "0.1.0"
edition = "2018"

[dependencies]
warp = {git = "https://github.com/aknuds1/warp", branch = "chore/upgrade-tokio"}

aknuds1 · 2021-01-12T15:21:33Z

@Kiskae Aha, I see the problem, it's because src/test.rs contains public utilities and depends on hyper::Client. Guess I'll have to reintroduce the dependency on hyper's client feature @seanmonstar.

Signed-off-by: Arve Knudsen <arve.knudsen@gmail.com>

nickelc · 2021-01-12T15:25:38Z

@Kiskae Aha, I see the problem, it's because src/test.rs contains public utilities and depends on hyper::Client. Guess I'll have to reintroduce the dependency on hyper's client feature @seanmonstar.

i think another option could be to make it an optional test feature that requires hyper/client

…tokio

aknuds1 · 2021-01-15T18:01:09Z

Thanks @seanmonstar! 🎉

Co-authored-by: janpetschexain <58227040+janpetschexain@users.noreply.github.com> Co-authored-by: João Oliveira <hello@jxs.pt> Co-authored-by: Paolo Barbolini <paolo@paolo565.org> Co-authored-by: Muhammad Hamza <muhammadhamza1311@gmail.com> Co-authored-by: teenjuna <53595243+teenjuna@users.noreply.github.com>

aknuds1 force-pushed the chore/upgrade-tokio branch from 5d2937c to cec8c56 Compare November 29, 2020 15:18

Upgrade to Tokio 0.3

e3c4c70

Signed-off-by: Arve Knudsen <arve.knudsen@gmail.com>

aknuds1 force-pushed the chore/upgrade-tokio branch from cec8c56 to e3c4c70 Compare November 29, 2020 15:19

aknuds1 mentioned this pull request Nov 29, 2020

Upgrade to Tokio v1 #725

Closed

3 tasks

Fix uds test

fc71e96

Signed-off-by: Arve Knudsen <arve.knudsen@gmail.com>

Urhengulas reviewed Nov 30, 2020

View reviewed changes

Cargo.toml Outdated Show resolved Hide resolved

src/filters/fs.rs Outdated Show resolved Hide resolved

blacktemplar mentioned this pull request Dec 1, 2020

Out-of-date dependencies sigp/lighthouse#1712

Closed

fundon reviewed Dec 6, 2020

View reviewed changes

src/filters/ws.rs Outdated Show resolved Hide resolved

Merge remote-tracking branch 'seanmonstar/master' into chore/upgrade-…

610f3dc

…tokio Signed-off-by: Arve Knudsen <arve.knudsen@gmail.com>

Urhengulas mentioned this pull request Dec 9, 2020

Bump tokio from 0.2 to 0.3 #741

Closed

5 tasks

Merge branch 'master' into chore/upgrade-tokio

d4c9344

jxs reviewed Dec 10, 2020

View reviewed changes

Cargo.toml Outdated Show resolved Hide resolved

Switch to tokio-tungstenite v0.12

6308e90

Signed-off-by: Arve Knudsen <arve.knudsen@gmail.com>

ljedrz mentioned this pull request Dec 11, 2020

Update libp2p and tokio rs-ipfs/rust-ipfs#431

Closed

phlip9 mentioned this pull request Dec 15, 2020

Bump tokio-util from 0.3.1 to 0.5.0 diem/diem#6680

Closed

Upgrade to tokio 1.0

2de7e08

Signed-off-by: Arve Knudsen <arve.knudsen@gmail.com>

aknuds1 changed the title ~~Upgrade to Tokio 0.3~~ Upgrade to Tokio 1.0 Dec 24, 2020

Depend on forked tokio-tungstenite

305c888

Signed-off-by: Arve Knudsen <arve.knudsen@gmail.com>

Upgrade tokio-tungstenite

b476d81

Signed-off-by: Arve Knudsen <arve.knudsen@gmail.com>

paolobarbolini reviewed Dec 24, 2020

View reviewed changes

Cargo.toml Outdated Show resolved Hide resolved

Fix build failures

fa8a9e8

Signed-off-by: Arve Knudsen <arve.knudsen@gmail.com>

paolobarbolini reviewed Dec 24, 2020

View reviewed changes

src/filters/body.rs Outdated Show resolved Hide resolved

aknuds1 and others added 2 commits December 24, 2020 17:42

Update src/filters/body.rs

5d068ae

Co-authored-by: Paolo Barbolini <paolo@paolo565.org>

Fix build failures

f242193

Signed-off-by: Arve Knudsen <arve.knudsen@gmail.com>

jxs reviewed Jan 9, 2021

View reviewed changes

jxs approved these changes Jan 9, 2021

View reviewed changes

aknuds1 added 2 commits January 10, 2021 15:07

Use ReaderStream instead of FramedRead

e50c375

Signed-off-by: Arve Knudsen <arve.knudsen@gmail.com>

Fix style

d8bfb02

Signed-off-by: Arve Knudsen <arve.knudsen@gmail.com>

thomaseizinger mentioned this pull request Jan 10, 2021

Upgrade to tokio 1.0 comit-network/waves#82

Merged

4 tasks

yotamofek mentioned this pull request Jan 11, 2021

Upgrade to Tokio 1.0 etcdv3/etcd-client#16

Merged

2 tasks

seanmonstar requested changes Jan 12, 2021

View reviewed changes

kflansburg mentioned this pull request Jan 12, 2021

Tokio 1.0.0 krustlet/krustlet#480

Merged

aknuds1 added 4 commits January 12, 2021 08:03

Merge remote-tracking branch 'seanmonstar/master' into chore/upgrade-…

e989869

…tokio Signed-off-by: Arve Knudsen <arve.knudsen@gmail.com>

CI: Remove net job

4d92fa8

Signed-off-by: Arve Knudsen <arve.knudsen@gmail.com>

Remove client feature for non-dev hyper dependency

657f1c6

Signed-off-by: Arve Knudsen <arve.knudsen@gmail.com>

Simplify websocket filter

bb2f511

Signed-off-by: Arve Knudsen <arve.knudsen@gmail.com>

aknuds1 requested a review from seanmonstar January 12, 2021 08:47

amousset mentioned this pull request Jan 12, 2021

Fixes #18818: Update relayd to tokio 1.0 Normation/rudder#3467

Merged

Reintroduce dependency on hyper's client feature

5ac7a62

Signed-off-by: Arve Knudsen <arve.knudsen@gmail.com>

jxs mentioned this pull request Jan 12, 2021

tracking 0.3 breaking changes #641

Closed

4 tasks

ChristianBeilschmidt mentioned this pull request Jan 13, 2021

Update tokio requirement from 0.2 to 1.0 geo-engine/geoengine#114

Closed

Merge remote-tracking branch 'seanmonstar/master' into chore/upgrade-…

b095a6d

…tokio

seanmonstar approved these changes Jan 15, 2021

View reviewed changes

seanmonstar merged commit ffefea0 into seanmonstar:master Jan 15, 2021

aknuds1 deleted the chore/upgrade-tokio branch January 15, 2021 18:00

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Upgrade to Tokio 1.0 #753

Upgrade to Tokio 1.0 #753

aknuds1 commented Nov 29, 2020 •

edited

Loading

Urhengulas left a comment

jhpratt commented Dec 24, 2020

aknuds1 commented Dec 24, 2020

aknuds1 commented Dec 24, 2020

aknuds1 commented Dec 24, 2020

paolobarbolini commented Dec 24, 2020 •

edited

Loading

aknuds1 commented Dec 24, 2020

jxs Jan 9, 2021

aknuds1 Jan 9, 2021

paolobarbolini Jan 9, 2021

aknuds1 Jan 9, 2021

jxs Jan 9, 2021

aknuds1 Jan 10, 2021

aknuds1 Jan 10, 2021

jxs left a comment

seanmonstar left a comment

seanmonstar Jan 12, 2021

aknuds1 Jan 12, 2021

koivunej commented Jan 12, 2021 •

edited

Loading

paolobarbolini commented Jan 12, 2021

Kiskae commented Jan 12, 2021

aknuds1 commented Jan 12, 2021

Kiskae commented Jan 12, 2021

aknuds1 commented Jan 12, 2021 •

edited

Loading

nickelc commented Jan 12, 2021 •

edited

Loading

aknuds1 commented Jan 15, 2021

Upgrade to Tokio 1.0 #753

Upgrade to Tokio 1.0 #753

Conversation

aknuds1 commented Nov 29, 2020 • edited Loading

Urhengulas left a comment

Choose a reason for hiding this comment

jhpratt commented Dec 24, 2020

aknuds1 commented Dec 24, 2020

aknuds1 commented Dec 24, 2020

aknuds1 commented Dec 24, 2020

paolobarbolini commented Dec 24, 2020 • edited Loading

aknuds1 commented Dec 24, 2020

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jxs left a comment

Choose a reason for hiding this comment

seanmonstar left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

koivunej commented Jan 12, 2021 • edited Loading

paolobarbolini commented Jan 12, 2021

Kiskae commented Jan 12, 2021

aknuds1 commented Jan 12, 2021

Kiskae commented Jan 12, 2021

aknuds1 commented Jan 12, 2021 • edited Loading

nickelc commented Jan 12, 2021 • edited Loading

aknuds1 commented Jan 15, 2021

aknuds1 commented Nov 29, 2020 •

edited

Loading

paolobarbolini commented Dec 24, 2020 •

edited

Loading

koivunej commented Jan 12, 2021 •

edited

Loading

aknuds1 commented Jan 12, 2021 •

edited

Loading

nickelc commented Jan 12, 2021 •

edited

Loading