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

feat(tree-wide): migrate to Hyper 1.x #2013

Merged
merged 1 commit into from
Jan 26, 2024
Merged

Conversation

liushuyu
Copy link
Contributor

@liushuyu liushuyu commented Jan 6, 2024

This pull request migrates the hyper crate to version 1.1.

I have to admit the situation is a bit disastrous due to the massive amount of API changes they have accumulated.

The following is a summary of the changes in this pull request:

  • Due to hyper splitting the library. Some more crates were added to accommodate and retain the functionalities.
  • Since reqwest (as of writing) still uses hyper 0.14, dependencies can not be shared anymore.
  • hyper now requires the library user to provide the I/O interface, so you will find I have added an adapter/wrapper to avoid refactoring all the web-server-related functions to be async.
  • The make_service macro is removed because the make_service_fn function was deleted. Instead, I added a taping-over function to stitch the services together like before.
  • The shutdown handling is changed as hyper now delegates some event-loop responsibilities to the library user.

@Xuanwo
Copy link
Collaborator

Xuanwo commented Jan 6, 2024

The entire Rust ecosystem has undergone significant changes since that code was written. We're still using non-async crates like rouille and working directly with hyper, which increases our maintenance workload. Do you think it's a good opportunity for us to switch to axum or another framework?

@codecov-commenter
Copy link

codecov-commenter commented Jan 6, 2024

Codecov Report

Attention: 49 lines in your changes are missing coverage. Please review.

Comparison is base (11703e0) 31.02% compared to head (c8f9c8b) 30.65%.

Files Patch % Lines
src/dist/client_auth.rs 0.00% 49 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2013      +/-   ##
==========================================
- Coverage   31.02%   30.65%   -0.38%     
==========================================
  Files          51       51              
  Lines       19617    19637      +20     
  Branches     9418     9428      +10     
==========================================
- Hits         6087     6019      -68     
- Misses       7874     7911      +37     
- Partials     5656     5707      +51     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@liushuyu
Copy link
Contributor Author

liushuyu commented Jan 6, 2024

The entire Rust ecosystem has undergone significant changes since that code was written. We're still using non-async crates like rouille and working directly with hyper, which increases our maintenance workload. Do you think it's a good opportunity for us to switch to axum or another framework?

axum is currently unstable (in terms of API). Other service frameworks within the direct tokio ecosystem are still trying to migrate to hyper v1.

So, I am unsure which approach is better given the current situation. The main issue as of now is still the architectural support. Ironically, non-mainstream devices usually need the caching the most (due to limited computing power), but sccache can't provide service for them.

@liushuyu liushuyu force-pushed the hyper-1.x branch 2 times, most recently from 6397eb7 to c8f9c8b Compare January 11, 2024 18:31
@liushuyu liushuyu marked this pull request as ready for review January 18, 2024 18:11
Copy link
Collaborator

@Xuanwo Xuanwo left a comment

Choose a reason for hiding this comment

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

Thanks!

@sylvestre sylvestre merged commit d2a89d7 into mozilla:main Jan 26, 2024
49 checks passed
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.

4 participants