Skip to content
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

Upgrade to Tokio v1 #725

Closed
3 tasks done
Urhengulas opened this issue Oct 15, 2020 · 26 comments · Fixed by #753
Closed
3 tasks done

Upgrade to Tokio v1 #725

Urhengulas opened this issue Oct 15, 2020 · 26 comments · Fixed by #753
Labels
feature New feature or request

Comments

@Urhengulas
Copy link

Urhengulas commented Oct 15, 2020

Dependencies which need to upgrade

@seanmonstar
Copy link
Owner

Yep, needs to happen in hyper first: hyperium/hyper#2302

@Urhengulas
Copy link
Author

Yep, needs to happen in hyper first: hyperium/hyper#2302

Yes, also saw that :)
The patch besides the hyper changes is quite small anyways.

bump-warp.patch
diff --git a/Cargo.toml b/Cargo.toml
index b40c2b5..89f681e 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -31,7 +31,7 @@ scoped-tls = "1.0"
 serde = "1.0"
 serde_json = "1.0"
 serde_urlencoded = "0.7"
-tokio = { version = "0.2", features = ["fs", "stream", "sync", "time"] }
+tokio = { version = "0.3", features = ["fs", "stream", "sync", "time"] }
 tracing = { version = "0.1", default-features = false, features = ["log", "std"] }
 tracing-futures = { version = "0.2", default-features = false, features = ["std-future"] }
 tower-service = "0.3"
@@ -47,7 +47,7 @@ tracing-subscriber = "0.2.7"
 tracing-log = "0.1"
 serde_derive = "1.0"
 handlebars = "3.0.0"
-tokio = { version = "0.2", features = ["macros"] }
+tokio = { version = "0.3", features = ["full"] }
 listenfd = "0.3"
 
 [features]
diff --git a/src/filters/fs.rs b/src/filters/fs.rs
index 1f04f6b..d4b9fc8 100644
--- a/src/filters/fs.rs
+++ b/src/filters/fs.rs
@@ -22,7 +22,6 @@ use hyper::Body;
 use mime_guess;
 use percent_encoding::percent_decode_str;
 use tokio::fs::File as TkFile;
-use tokio::io::AsyncRead;
 
 use crate::filter::{Filter, FilterClone, One};
 use crate::reject::{self, Rejection};
diff --git a/src/filters/sse.rs b/src/filters/sse.rs
index 8c4eda5..9d58492 100644
--- a/src/filters/sse.rs
+++ b/src/filters/sse.rs
@@ -54,7 +54,7 @@ use hyper::Body;
 use pin_project::pin_project;
 use serde::Serialize;
 use serde_json;
-use tokio::time::{self, Delay};
+use tokio::time::{self, Sleep};
 
 use self::sealed::{
     BoxedServerSentEvent, EitherServerSentEvent, SseError, SseField, SseFormat, SseWrapper,
@@ -467,7 +467,7 @@ impl KeepAlive {
         S::Ok: ServerSentEvent + Send,
         S::Error: StdError + Send + Sync + 'static,
     {
-        let alive_timer = time::delay_for(self.max_interval);
+        let alive_timer = time::sleep(self.max_interval);
         SseKeepAlive {
             event_stream,
             comment_text: self.comment_text,
@@ -484,7 +484,7 @@ struct SseKeepAlive<S> {
     event_stream: S,
     comment_text: Cow<'static, str>,
     max_interval: Duration,
-    alive_timer: Delay,
+    alive_timer: Sleep,
 }
 
 #[doc(hidden)]
@@ -505,7 +505,7 @@ where
     let max_interval = keep_interval
         .into()
         .unwrap_or_else(|| Duration::from_secs(15));
-    let alive_timer = time::delay_for(max_interval);
+    let alive_timer = time::sleep(max_interval);
     SseKeepAlive {
         event_stream,
         comment_text: Cow::Borrowed(""),

@jxs
Copy link
Collaborator

jxs commented Oct 16, 2020

there's also the dependency on tokio-tungstenite and tokio-rustls.
tokio-rustls already has a PR upgrading it: tokio-rs/tls#29

@pomazanbohdan
Copy link

seanmonstar/reqwest#1060

@vn971
Copy link

vn971 commented Nov 10, 2020

Note that currently, warp can start with tokio being explicitly set to 0.3, and it will then fail at run-time. I wonder if we can/should tackle this problem so that it doesn't affect us in the future? E.g. so that the same run-time error cannot appear in future tokio releases (if applicable).

I'm really happy to see that the problem is being addressed in all the referenced projects though, thanks a lot for keeping it up-to-date!

@Urhengulas
Copy link
Author

Note that currently, warp can start with tokio being explicitly set to 0.3, and it will then fail at run-time. I wonder if we can/should tackle this problem so that it doesn't affect us in the future? E.g. so that the same run-time error cannot appear in future tokio releases (if applicable).

I'm really happy to see that the problem is being addressed in all the referenced projects though, thanks a lot for keeping it up-to-date!

@vn971 That is a very good idea. I had this problem in my project, that I thought I successfully upgraded to tokio 0.3, but then it actually failed on runtime, since dependencies are using tokio 0.2.

What would be an approach to avoid that for the future? Let tokio check the dependency tree on compile-time?

@MikailBag
Copy link

For example, warp can explicitly take tokio::runtime::Handle for server construction. That way it's always visible in user code which tokio version is used by warp.

@aknuds1
Copy link
Contributor

aknuds1 commented Nov 28, 2020

Is this being worked on in a branch?

@Urhengulas
Copy link
Author

Is this being worked on in a branch?

I started in #725, but feel free to take over :)

@aknuds1
Copy link
Contributor

aknuds1 commented Nov 29, 2020

@Urhengulas ok thanks, I will try.

@aknuds1
Copy link
Contributor

aknuds1 commented Nov 29, 2020

@Urhengulas I made my own PR, where I managed to get all tests to pass (cargo test --all-features): #753.

@phungleson
Copy link

Hey everyone, wonder if tokio 0.3.0 will be merged soon?

@novacrazy
Copy link

Likewise, just ran into issues with this that were mind-boggling until I realized the versions were different, and indeed a Tokio 0.2 reactor was not running, only 0.3

@xue35
Copy link

xue35 commented Dec 21, 2020

I just noticed that examples/hello.rs does not run with tokio 0.3. Switching back to tokio 0.2 runs perfectly.

@honsunrise
Copy link

Are we ready to upgrade to tokio 0.3?

@slyshykO
Copy link

@hosunrise Think update to tokio 1.0 more likely :)
If also have in mind hyperium/hyper#2369

@honsunrise
Copy link

honsunrise commented Dec 24, 2020

Wow tokio 1.0 is released 🚀🚀🚀🚀!!
So, are we ready to upgrade to tokio 1.0?

@songday
Copy link

songday commented Dec 24, 2020

@sinkuu Wow tokio 1.0 is released 🚀🚀🚀🚀!!
So, are we ready to upgrade to tokio 1.0?

Not yet, but I guess soon
Since hyper just released 0.14.1 which supports tokio 1.0 and tokio-rustls also upgraded
Only tokio-tungstenite left

@simonsan
Copy link

simonsan commented Jan 7, 2021

Would it make sense to widen the scope of this issue and jump to tokio-v1.0 directly?

@aknuds1
Copy link
Contributor

aknuds1 commented Jan 7, 2021

@simonsan My PR upgrades to tokio 1.0.

@simonsan
Copy link

simonsan commented Jan 7, 2021

@simonsan My PR upgrades to tokio 1.0.

Is there an issue to relate to for tracking?

@aknuds1
Copy link
Contributor

aknuds1 commented Jan 7, 2021

@simonsan This issue.

@Urhengulas Urhengulas changed the title Upgrade to Tokio v0.3 Upgrade to Tokio v1 Jan 7, 2021
@songday
Copy link

songday commented Jan 8, 2021

@sinkuu Wow tokio 1.0 is released 🚀🚀🚀🚀!!
So, are we ready to upgrade to tokio 1.0?

Not yet, but I guess soon
Since hyper just released 0.14.1 which supports tokio 1.0 and tokio-rustls also upgraded
Only tokio-tungstenite left

Currently, the last component tokio-tungstenite ( snapview/tokio-tungstenite#142 ) depends on tungstenite-rs ( snapview/tungstenite-rs#169 ) which depends on http
http v0.2.3 ( hyperium/http#463 ) will be released soon (I guess)

@mraerino
Copy link

mraerino commented Jan 9, 2021

tokio-tungstenite v0.13.0 has been released with the fix: snapview/tokio-tungstenite#142

@bkontur
Copy link

bkontur commented Jan 15, 2021

Hello, is there any date for release warp with new tokio?

@extrawurst
Copy link

It was merged!
Now we only need a release :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.