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

uses multiple http client libraries, none of them much used #374

Closed
jonassmedegaard opened this issue Apr 7, 2022 · 11 comments · Fixed by #375
Closed

uses multiple http client libraries, none of them much used #374

jonassmedegaard opened this issue Apr 7, 2022 · 11 comments · Fixed by #375

Comments

@jonassmedegaard
Copy link

atomic_lib depends on ureq which (is publicly exposed but internally) seems used only for `validate which seems deprecated (see #61).

atomic_server depends on awcwhich seems used only in relation to acme_lib (which itself depends on ureq so it seems odd a) that extra client calls are even necessary, and b) a different client library is used).

It seems sensible to flag the dependency on awc as optional, tied to feature https.

It seems sensible to use only one http library.
Right now simplest is to drop awc since ureq is also used by acme_lib.
Smarter might be to drop both awcand ureq and instead use generic-async-http-client which is also used by async-acme (see #192). If I understand correctly, using that with feature tokio-rustls reuses most of the underlying libraries.

If keeping ureq then please consider disabling feature cookies as (to the best of my knowledge) cookies are not being used neither for accessing atomic servers nor to access Letsencrypt. Then crate cookie_storage should only get pulled on with feature https (because apparently acme_lib has that feature always enabled).

@joepio
Copy link
Member

joepio commented Apr 8, 2022

Thanks for taking the time to help keep these crates minimal!

atomic_lib depends on ureq which (is publicly exposed but internally) seems used only for validate

ureq is also used in client, but I should definitely try to remove either awc or ureq - no reason to have both.

@jonassmedegaard
Copy link
Author

ureq is also used in client

Really? Where?

@jonassmedegaard
Copy link
Author

ah, you mean indirectly via the public interface to atomic_lib.

@joepio
Copy link
Member

joepio commented Apr 8, 2022

In atomic_lib::client::fetch_body

@jonassmedegaard
Copy link
Author

Right: in atomic_lib::client::fetch_body :-)

@jonassmedegaard
Copy link
Author

Ohhh, now I get your point: in atomic_lib not only validate but also client calls ureq.

@jonassmedegaard
Copy link
Author

I missed that because I only looked for use ureq statements - I was unaware that you could load it deeper in the code.

@jonassmedegaard
Copy link
Author

hm, no - scratch that - I don't make sense

joepio added a commit that referenced this issue Apr 9, 2022
@joepio joepio mentioned this issue Apr 9, 2022
4 tasks
@jonassmedegaard
Copy link
Author

was it deliberate that you pushed to a branch for #331 seemingly unrelated to this issue?

@joepio
Copy link
Member

joepio commented Apr 9, 2022

deliberate!

joepio added a commit that referenced this issue Apr 9, 2022
@joepio
Copy link
Member

joepio commented Apr 9, 2022

If keeping ureq then please consider disabling feature cookies as (to the best of my knowledge) cookies are not being used neither for accessing atomic servers nor to access Letsencrypt. Then crate cookie_storage should only get pulled on with feature https (because apparently acme_lib has that feature always enabled).

I still use ureq, but the cookies feature is not enabled as far as I can see. Except that acme_lib does depend on it (as well as an older ureq version.

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 a pull request may close this issue.

2 participants