Replace reqwest with ureq #417
Conversation
ureq is a blocking HTTP client. ureq is simpler than reqwest. This brings: - smaller binary size - cleaner configuration interfaces
src/cache.rs
Outdated
| if let Ok(ref host) = env::var("HTTP_PROXY") { | ||
| return Ok(Some(Proxy::new(host)?)); | ||
| } | ||
| if let Ok(ref host) = env::var("HTTPS_PROXY") { | ||
| return Ok(Some(Proxy::new(host)?)); | ||
| } | ||
|
|
||
| Ok(None) |
There was a problem hiding this comment.
This is different from what we did before. The code before uses HTTP_PROXY as a Proxy::http and HTTPS_PROXY as a Proxy::https, which means that they will be used for http and https traffic respectively (see https://docs.rs/reqwest/latest/reqwest/struct.Proxy.html). Now, this uses HTTP_PROXY also for https traffic if present and ignores HTTP_PROXY.
If we want to be conservative, we could only check HTTPS_PROXY. Alternatively, there is https://docs.rs/ureq/latest/ureq/struct.Proxy.html#method.try_from_env, which would prefer HTTPS_PROXY, but still use HTTP_PROXY if HTTPS_PROXY is unset. I am not sure which approach is best
There was a problem hiding this comment.
My experience is to set both HTTP_PROXY/HTTPS_PROXY. Otherwise, I will go with SOCKS_PROXY. The default preference looks okay. HTTP_PROXY is also not going to affect tldr given the source is usually GitHub. And GitHub uses https.
Let's go with the default which will use Proxy::try_from_env.
There was a problem hiding this comment.
Alright. We should add a test at some point, see #418
ureq is a blocking HTTP client. ureq is simpler than reqwest. This brings:
Fixes #415