Conversation
ureq backend for blitz-netureq 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 |
nicoburns
left a comment
There was a problem hiding this comment.
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
tokiofrom 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.
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.
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)