camallo / dkregistry-rs

A pure-Rust asynchronous library for Docker Registry API v2
Apache License 2.0
62 stars 39 forks source link

v2/client: move to async reqwest #44

Closed lucab closed 5 years ago

lucab commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

@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 commented 6 years ago

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 commented 5 years ago

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.