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.

Sign up for GitHub

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 1.0 #142

Merged
merged 5 commits into from
Jan 9, 2021
Merged

Upgrade to tokio 1.0 #142

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
7e1ac6c
Upgrade to tokio 1.0
dnaka91 Dec 24, 2020
9c05f20
Fix formatting in handshake.rs
dnaka91 Dec 24, 2020
0f0665b
Update to tokio-native-tls 0.3.0
dnaka91 Dec 25, 2020
078a227
Update tungstenite dependency to main repo
dnaka91 Dec 31, 2020
c0e4979
Update to upstream version of tungstenite
daniel-abramov Jan 8, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,10 @@ stream = []
log = "0.4"
futures-util = { version = "0.3", default-features = false, features = ["async-await", "sink", "std"] }
pin-project = "1.0"
tokio = { version = "0.3", default-features = false, features = ["io-util"] }
tokio = { version = "1.0.0", default-features = false, features = ["io-util"] }
Copy link

Choose a reason for hiding this comment

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

Is there any reason for pinning to 1.0.0. Why can't it just be 1.0?

Copy link

Choose a reason for hiding this comment

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

If semantic versioning is used properly for this, which it should be, couldn't this even be "1" as tokio is stable on 1.x.x?

Copy link

Choose a reason for hiding this comment

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

That's a valid point. I mostly also specify the minor version in my apps so i didn't think of that. So, the tokio version here should just be 1

Copy link
Contributor

Choose a reason for hiding this comment

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

Right, per https://doc.rust-lang.org/cargo/reference/specifying-dependencies.html#caret-requirements 1, 1.0 and 1.0.0 are all equivalent.

Then it is a question of style/consistency. 1.0 would be most consistent locally (see line above). Is anybody aware of an ecosystem-wide style recommendation for equivalent version declarations in Cargo.toml?

Copy link

Choose a reason for hiding this comment

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

All I remember is Cargo endorses semantic versioning and the major version thats 1+.x.x won't have any breaking changes. It's fine to use 1.0 as well for consistency and in case of local bugfixes that get added/removed due to tokio in this repo as tokio is developed/patched

Copy link
Contributor Author

@dnaka91 dnaka91 Jan 8, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

As @strohel mentioned 1.0.0 is equivalent to 1.x.x. the only exception to this rule are 0.x.x releases. For example 0.1.0 would be equivalent to 0.1.x.

I personally prefer the full version as it kind of is a statement to what version the project was tested with by the authors and considered to be working. So that later when an error happens only on the user side and the user states that he is using for example the auto upgraded 1.5.3 instead of 1.0.0, then it's easy to derive that the error lies in some changes of the said dependency. Or at least would be likely and a good thing to look at first.

In the end it's up to you folks from snapview. I just wanted to explain my reasoning behind putting the full version in the Cargo.toml and don't mind changing to just 1.0 or 1.

Copy link

Choose a reason for hiding this comment

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

It's not pinned, this is equivalent to ^1.0.0 (docs) and will allow updates to any 1.x.x.

Copy link

Choose a reason for hiding this comment

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

It's not pinned, this is equivalent to ^1.0.0 (docs) and will allow updates to any 1.x.x.

TIL. Thanks for informing me


[dependencies.tungstenite]
version = "0.11.1"
version = "0.12.0"
default-features = false

[dependencies.native-tls]
Expand All @@ -33,10 +33,10 @@ version = "0.2.0"

[dependencies.tokio-native-tls]
optional = true
version = "0.2"
version = "0.3.0"

[dev-dependencies]
futures-channel = "0.3"
tokio = { version = "0.3", default-features = false, features = ["io-std", "macros", "rt-multi-thread", "stream", "time"] }
tokio = { version = "1.0.0", default-features = false, features = ["io-std", "macros", "rt-multi-thread", "time"] }
url = "2.0.0"
env_logger = "0.7"
23 changes: 7 additions & 16 deletions examples/interval-server.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,4 @@
use futures_util::{
future::{select, Either},
SinkExt, StreamExt,
};
use futures_util::{SinkExt, StreamExt};
use log::*;
use std::{net::SocketAddr, time::Duration};
use tokio::net::{TcpListener, TcpStream};
Expand All @@ -25,29 +22,23 @@ async fn handle_connection(peer: SocketAddr, stream: TcpStream) -> Result<()> {

// Echo incoming WebSocket messages and send a message periodically every second.

let mut msg_fut = ws_receiver.next();
let mut tick_fut = interval.next();
loop {
match select(msg_fut, tick_fut).await {
Either::Left((msg, tick_fut_continue)) => {
tokio::select! {
msg = ws_receiver.next() => {
match msg {
Some(msg) => {
let msg = msg?;
if msg.is_text() || msg.is_binary() {
if msg.is_text() ||msg.is_binary() {
ws_sender.send(msg).await?;
} else if msg.is_close() {
break;
}
tick_fut = tick_fut_continue; // Continue waiting for tick.
msg_fut = ws_receiver.next(); // Receive next WebSocket message.
}
None => break, // WebSocket stream terminated.
};
None => break,
}
}
Either::Right((_, msg_fut_continue)) => {
_ = interval.tick() => {
ws_sender.send(Message::Text("tick".to_owned())).await?;
msg_fut = msg_fut_continue; // Continue receiving the WebSocket message.
tick_fut = interval.next(); // Wait for next tick.
}
}
}
Expand Down
20 changes: 10 additions & 10 deletions src/handshake.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,11 +92,11 @@ pub(crate) async fn client_handshake<F, S>(
) -> Result<(WebSocketStream<S>, Response), Error<ClientHandshake<AllowStd<S>>>>
where
F: FnOnce(
AllowStd<S>,
) -> Result<
<ClientHandshake<AllowStd<S>> as HandshakeRole>::FinalResult,
Error<ClientHandshake<AllowStd<S>>>,
> + Unpin,
AllowStd<S>,
) -> Result<
<ClientHandshake<AllowStd<S>> as HandshakeRole>::FinalResult,
Error<ClientHandshake<AllowStd<S>>>,
> + Unpin,
S: AsyncRead + AsyncWrite + Unpin,
{
let result = handshake(stream, f).await?;
Expand All @@ -111,11 +111,11 @@ pub(crate) async fn server_handshake<C, F, S>(
where
C: Callback + Unpin,
F: FnOnce(
AllowStd<S>,
) -> Result<
<ServerHandshake<AllowStd<S>, C> as HandshakeRole>::FinalResult,
Error<ServerHandshake<AllowStd<S>, C>>,
> + Unpin,
AllowStd<S>,
) -> Result<
<ServerHandshake<AllowStd<S>, C> as HandshakeRole>::FinalResult,
Error<ServerHandshake<AllowStd<S>, C>>,
> + Unpin,
S: AsyncRead + AsyncWrite + Unpin,
{
let s: WebSocket<AllowStd<S>> = handshake(stream, f).await?;
Expand Down