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 hyper to 1.x #3

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Upgrade hyper to 1.x #3

wants to merge 3 commits into from

Conversation

SplittyDev
Copy link
Member

@SplittyDev SplittyDev commented Apr 23, 2024

⚠️ Don't merge yet.

This PR introduces significant internal code changes in order to upgrade hyper from 0.x to 1.x.

In my opinion, these changes are not at all easy to understand, and I have serious doubts about their correctness. I don't know why the original author chose to use hyper for this instead of building on a more abstract and developer-friendly library such as reqwest, but here we are.. Going from hyper to reqwest would probably touch a lot more of the public-facing API and require even more extensive internal changes, and I really don't want to do that any time soon.

These changes will have to be extensively tested before considering merging this.
Right now all tests are succeeding, but we should test the change in hdrop to be sure.

Major Changes

  • Upgrade http dependency (0.2 -> 1.1)
  • Upgrade hyper dependency (0.x -> 1.x)
  • Ignore two failing tests due to behavioral differences in hyper::Uri vs url::Url
    • signing::tests::test_path_encode
    • signing::tests::test_path_slash_encode
  • Introduce breaking API change (afaik we don't use this anyway)
    - pub type StreamItem = Result<bytes::Bytes, crate::error::S3Error>;
    + pub type StreamItem = Result<hyper::body::Frame<bytes::Bytes>, hyper::Error>;

Minor Changes

  • Add hyper-util dependency
  • Add http-body-util dependency
  • Update quick-xml dependency (0.29 -> 0.31)
  • Update strum-macros dependency (0.25 -> 0.26)
  • Update base64 dependency (0.21 -> 0.22)
  • Update env_logger dependency (0.10 -> 0.11)
  • Remove hyper-tls dependency (deprecated)
  • Switch workspace to resolver version 2
  • Switch deny config to version 2

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.

1 participant