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 Tokio to version 0.3.0 #681

Merged
merged 9 commits into from
Nov 19, 2020
Merged

Upgrade Tokio to version 0.3.0 #681

merged 9 commits into from
Nov 19, 2020

Conversation

romac
Copy link
Member

@romac romac commented Nov 17, 2020

Close: #683

Target: v0.17.0-rc4

  • Upgrade Tokio to version 0.3.
  • Upgrade all dependencies which depends on Tokio 0.2 to newer versions for Tokio 0.3 support.
    • async-tungstenite: 0.8 => 0.10
    • hyper: 0.13 => 0.14.0-dev (development version)

  • Referenced an issue explaining the need for the change
  • Updated all relevant documentation in docs
  • Updated all code comments where relevant
  • Wrote tests
  • Updated CHANGELOG.md

@informalsystems informalsystems deleted a comment from codecov-io Nov 17, 2020
@tony-iqlusion
Copy link
Collaborator

tony-iqlusion commented Nov 17, 2020

It'd be really helpful if we could get another -rc release with #679 fixed before this lands.

It's a breaking change to TMKMS to upgrade Tokio and we haven't done it yet, and we need to be able to test Stargate upgrades with production keys ASAP.

@thanethomson
Copy link
Contributor

It'd be really helpful if we could get another -rc release with #679 fixed before this lands.

It's a breaking change to TMKMS to upgrade Tokio and we haven't done it yet, and we need to be able to test Stargate upgrades with production keys ASAP.

Sure - I'd say we should target v0.17.0-rc4 to upgrade Tokio 👍 (rc3 will release the fix for #679)

@informalsystems informalsystems deleted a comment from codecov-io Nov 18, 2020
@romac romac marked this pull request as ready for review November 18, 2020 09:04
@codecov-io
Copy link

codecov-io commented Nov 18, 2020

Codecov Report

Merging #681 (53ae372) into master (91bc1f6) will increase coverage by 0.0%.
The diff coverage is 29.4%.

Impacted file tree graph

@@          Coverage Diff           @@
##           master    #681   +/-   ##
======================================
  Coverage    39.4%   39.5%           
======================================
  Files         191     191           
  Lines       12595   12592    -3     
  Branches     3234    3229    -5     
======================================
+ Hits         4974    4981    +7     
+ Misses       7284    7274   -10     
  Partials      337     337           
Impacted Files Coverage Δ
light-client/src/components/io.rs 0.0% <ø> (ø)
light-client/src/utils/block_on.rs 0.0% <0.0%> (ø)
light-node/src/application.rs 0.0% <0.0%> (ø)
light-node/src/commands/start.rs 0.0% <0.0%> (ø)
tendermint/tests/integration.rs 0.4% <0.0%> (ø)
rpc/src/client/subscription.rs 74.0% <100.0%> (+1.4%) ⬆️
rpc/src/client/sync.rs 57.1% <100.0%> (+7.1%) ⬆️
rpc/src/client/transport/websocket.rs 73.0% <100.0%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 91bc1f6...53ae372. Read the comment docs.

melekes
melekes previously approved these changes Nov 19, 2020
Copy link
Contributor

@melekes melekes left a comment

Choose a reason for hiding this comment

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

👍

@@ -53,14 +54,19 @@ pub trait SubscriptionClient {
/// ```
///
/// [`Event`]: ./event/struct.Event.html
#[pin_project]
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for this! 👍 I was wondering when this was going to become necessary 😁

Copy link
Member Author

Choose a reason for hiding this comment

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

FWIW, I initially did it the unsafe and manual way, but that unsafe block kept me most of the following night, so I figured I'd rather trust a real Rust dev and take the (admittedly very small) compile-time + code size penalty with pin-project.

Copy link
Member Author

Choose a reason for hiding this comment

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

Before/after: 7c8f444

Copy link
Contributor

@thanethomson thanethomson left a comment

Choose a reason for hiding this comment

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

Looks awesome! 👍

@thanethomson thanethomson merged commit 0fc7f40 into master Nov 19, 2020
@thanethomson thanethomson deleted the romac/upgrade-tokio branch November 19, 2020 17:23
@adizere adizere restored the romac/upgrade-tokio branch November 20, 2020 10:27
@adizere
Copy link
Member

adizere commented Nov 20, 2020

As a temporary workaround, the PR informalsystems/hermes#364 depends on branch romac/upgrade-tokio here. We'll delete this branch later once it's not needed anymore.

@romac romac deleted the romac/upgrade-tokio branch January 12, 2021 14:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Upgrade Tokio to version 0.3.0
6 participants