-
Notifications
You must be signed in to change notification settings - Fork 94
Add ureq
backend for blitz-net
#189
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
base: main
Are you sure you want to change the base?
Conversation
ureq
backend for blitz-net
ureq
backend for blitz-net
To test this, set - let html = rt.block_on(blitz_net::get_text(&url));
+ let html = blitz_net::get_text(&url)); |
@nicoburns I know you'll be gone, but once you're back, I have a couple architecture questions:
"Allow edits by maintainers" should be on if you feel inspired haha |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for taking so long to review this. I've requested a couple of specific changes.
In regard to your questions:
- I think you should be able to unify the interface of the backends by using a callback-based interface which should work for both async and threadpool+channel based backends.
- Removing
tokio
from other crates has now happened :)
I think I'm probably going to try to land #188 before this as I suspect it will be easier to rework this one than that one.
self.client | ||
.run(<blitz_traits::net::Request as Into<http::Request<()>>>::into(request))? | ||
} else { | ||
self.client.run(<blitz_traits::net::Request as Into< | ||
http::Request<Vec<u8>>, | ||
>>::into(request))? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These type assertions are kinda horrible. I think you could make this much nicer by splitting the conversion onto a separate line:
let request : http::Request<()> = request.into();
self.client.run(request)?
I also wonder how useful the ()
path is here. Could we just always use the Vec<u8>
version and pass an empty Vec
if there's no data?
@@ -0,0 +1,16 @@ | |||
use blitz_traits::net::NetCallback; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we move the callbacks into the "backend" modules (so all the code each backend is in the same place) please?
Addresses #181
TODO
(These are just copied from the above issue :P)