Replace reqwest with bitreq #20
Conversation
TheBlueMatt
force-pushed
the
main
branch
2 times, most recently
from
ccfa0dd to
5c4b437
Compare
`reqwest` has an absolutely insane dependency tree, with anti-features (like insanely-fast-unsafe-IDN parsing) baked in. Instead, the rust-bitcoin ecosystem is starting to move to `bitreq` which is relatively simple, only depends (even optionally) on `tokio` and `rustls`, and doesn't bother with crap like IDNs. Here we switch over, maintaining the ability to proxy connections for those that need that.
Sadly, we'd exposed `reqwest` in our public API, so we have to bump the version for the swap to `bitreq`.
|
Looks like |
TheBlueMatt
force-pushed
the
main
branch
2 times, most recently
from
1daa77b to
ec0b296
Compare
tnull
left a comment
tnull
left a comment
There was a problem hiding this comment.
LGTM
Makes sense to switch this crate to bitreq already, for other applications I'm still a bit sceptical if its feature set is fully ready yet.
Confirmed failing tests pass locally.
|
|
||
| impl Default for HTTPHrnResolver { | ||
| fn default() -> Self { | ||
| HTTPHrnResolver { client: reqwest::Client::new() } |
There was a problem hiding this comment.
Hmm, seems bitreq has no way to reuse a connection. That might be fine here, but for other applications it seems that might introduce a bunch of unnecessary latency overhead (cf rust-bitcoin/corepc#442).
There was a problem hiding this comment.
Yea, for syncing it likely does.
| if let Some(proxy) = &self.proxy { | ||
| req = req.with_proxy(proxy.clone()) | ||
| } | ||
| let resp = req.send_async().await.map_err(|_| ())?; |
There was a problem hiding this comment.
Turns out that this is not actually async but currently just spawns a blocking task (i.e., potentially a thread) per request (cf. rust-bitcoin/corepc#441).
There was a problem hiding this comment.
Oof, yea, that's pretty gnarly...maybe we wait for that to be fixed....let me see if claude can do it.
There was a problem hiding this comment.
Okay, got a PR up for it. Gonna go ahead and land this but we should wait for rust-bitcoin/corepc#448 to release.
2 participants
reqwesthas an absolutely insane dependency tree, withanti-features (like insanely-fast-unsafe-IDN parsing) baked in.
Instead, the rust-bitcoin ecosystem is starting to move to
bitreqwhich is relatively simple, only depends (even optionally) on
tokioandrustls, and doesn't bother with crap like IDNs.Here we switch over, maintaining the ability to proxy connections
for those that need that.