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

v2/client: move to async reqwest #44

Closed
9 tasks done
lucab opened this issue Oct 12, 2018 · 4 comments · Fixed by #91
Closed
9 tasks done

v2/client: move to async reqwest #44

lucab opened this issue Oct 12, 2018 · 4 comments · Fixed by #91

Comments

@lucab
Copy link
Member

lucab commented Oct 12, 2018

Now that reqwest has an async module, we should use that instead of plain hyper. Unfortunately, this means switching from rustls to native-tls, thus getting a FFI dependency to openssl (or similar).


Progress

@steveej
Copy link
Contributor

steveej commented Oct 15, 2018

Could you describe the benefits of switching to reqwest over using hyper? Do you expect the reqwest API to be more stable than the hyper one?

@lucab
Copy link
Member Author

lucab commented Oct 16, 2018

@steveej both are still in development and will likely see a major rework once futures land in stdlib. However hyper is the wrong layer to use when implementing an HTTP client, which means that we poorly re-implement here some middleware logic (e.g. following redirections).

This crate status quo is is purely a result of historical timing, as at that point there was no higher-level async HTTP client.

@steveej
Copy link
Contributor

steveej commented Oct 16, 2018

However hyper is the wrong layer to use when implementing an HTTP client, which means that we poorly re-implement here some middleware logic (e.g. following redirections).

This crate status quo is is purely a result of historical timing, as at that point there was no higher-level async HTTP client.

I understand your concern. If we need to extend the functionality of dkregistry we can refactor the code to use reqwest along that. Personally I don't have the urge to do it earlier, but of course I'm not stopping you from doing it ;-)

@steveej
Copy link
Contributor

steveej commented Feb 7, 2019

The close was unintentional via a stale Fixes .. in a PR.
Marking this as a bug because it causes the system certificate store to be ignored, which is the default behavior of hyper-rustls.

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