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

Update hyper to 0.14 and tokio/bytes to 1.0 #511

Merged
merged 4 commits into from
Jan 19, 2021

Conversation

msrd0
Copy link
Member

@msrd0 msrd0 commented Dec 24, 2020

This PR updates hyper to 0.14 and tokio and bytes to 1.0.

The websocket example has been disabled as tokio-tungstenite has not yet been updated (PR: snapview/tokio-tungstenite#142). The websocket example has also been updated. Unfortunately, hyper no longer accepts upgrades on the Body type but requires OnUpgrade from the http requests extensions, so I put that type into the state whenever it appears in the request.

Note: hyper split up their code behind several feature flags. As of right now, gotham basically needs all features, since we have built-in test functionality that also depends on the client side of hyper. We might want to also expose feature flags that only depend on the hyper client code if we enabled our test code.

@msrd0 msrd0 added this to the 0.6 milestone Dec 24, 2020
@msrd0 msrd0 force-pushed the update-hyper-to-0.14 branch from 6ed2b9b to 669dd4c Compare December 24, 2020 01:17
@codecov
Copy link

codecov bot commented Dec 24, 2020

Codecov Report

Merging #511 (12a75e0) into master (89d73f6) will increase coverage by 0.00%.
The diff coverage is 83.58%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #511   +/-   ##
=======================================
  Coverage   85.54%   85.54%           
=======================================
  Files         109      109           
  Lines        5569     5577    +8     
=======================================
+ Hits         4764     4771    +7     
- Misses        805      806    +1     
Impacted Files Coverage Δ
gotham/src/lib.rs 83.33% <0.00%> (ø)
gotham/src/plain.rs 45.45% <0.00%> (ø)
gotham/src/tls.rs 23.52% <0.00%> (ø)
middleware/diesel/src/repo.rs 76.19% <ø> (ø)
gotham/src/test.rs 72.13% <40.00%> (-3.31%) ⬇️
examples/websocket/src/main.rs 85.57% <75.00%> (+0.14%) ⬆️
examples/diesel/src/main.rs 89.28% <100.00%> (ø)
...xamples/handlers/simple_async_handlers/src/main.rs 91.66% <100.00%> (-0.53%) ⬇️
...s/handlers/simple_async_handlers_await/src/main.rs 89.09% <100.00%> (-0.57%) ⬇️
examples/hello_world_until/src/main.rs 100.00% <100.00%> (ø)
... and 6 more

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 89d73f6...12a75e0. Read the comment docs.

@msrd0 msrd0 force-pushed the update-hyper-to-0.14 branch from 669dd4c to 02ec42e Compare December 24, 2020 14:15
@msrd0 msrd0 changed the title WIP: Update hyper to 0.14 and tokio/bytes to 1.0 Update hyper to 0.14 and tokio/bytes to 1.0 Dec 24, 2020
@msrd0 msrd0 marked this pull request as ready for review December 24, 2020 14:23
Copy link
Contributor

@nyarly nyarly left a comment

Choose a reason for hiding this comment

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

Yay websockets

@msrd0 msrd0 merged commit 9f10935 into gotham-rs:master Jan 19, 2021
@msrd0 msrd0 deleted the update-hyper-to-0.14 branch January 19, 2021 23:10
@krallin
Copy link
Contributor

krallin commented Feb 23, 2021

Would it make sense to just expose the Extensions on the state directly, given that this is what Gotham does for the other parts that come from Hyper?

For context, the reason I'm asking is because I'd like to send a PR to expose those, but if I'm exposing extensions, it's a little awkward if this one isn't there and has to be obtained from the State instead. Given this version hasn't been released yet, maybe there's time to reconsider this decision? :)

@msrd0
Copy link
Member Author

msrd0 commented Feb 23, 2021

We could just put the extensions into the state directly. Since there seems to be no way of getting an iterator over all extensions, this might be the only way for now, especially for non-standard extensions that gotham can't know about. However, it feels a bit weird since both State and Extensions are basically wrappers over HashMap<TypeId, Box<dyn Any>>.

The only alternative I can think of is providing a method that allows the user to process extensions and put them into the state - which I'm not a fan of since it'd involve a second type of middleware that'd probably just make stuff more complicated.

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

Successfully merging this pull request may close these issues.

3 participants