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

Add ureq backend for blitz-net #189

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

Conversation

SpikeHD
Copy link

@SpikeHD SpikeHD commented Feb 28, 2025

Addresses #181

TODO

(These are just copied from the above issue :P)

  • Be part of the existing blitz-net crate with feature flags for each backend
  • Use the new 3.x version of ureq
  • Split the reqwest and ureq implementations in separate files/modules
  • Use a threadpool to allow for multiple concurrent requests
  • Implement the blitz_net::get_text method as well as blitz_net::Provider

@SpikeHD SpikeHD marked this pull request as draft February 28, 2025 20:32
@SpikeHD SpikeHD changed the title DRAFT: ureq backend for blitz-net Add ureq backend for blitz-net Feb 28, 2025
@SpikeHD
Copy link
Author

SpikeHD commented Feb 28, 2025

To test this, set ureq as the default feature in blitz-net, then change packages/blitz/src/lib.rs@L36 like so:

- let html = rt.block_on(blitz_net::get_text(&url));
+ let html = blitz_net::get_text(&url));

@SpikeHD
Copy link
Author

SpikeHD commented Feb 28, 2025

@nicoburns I know you'll be gone, but once you're back, I have a couple architecture questions:

  • Is there a better way to handle the two Providers? Right now they are almost the same, except for the fact that fetch_inner is async on the reqwest backend. I'd like to ofc prevent repeated code but my little brain can't conceptualize how that might look
  • How should the other packages be modified to not use tokio (in the case someone is gonna use the ureq backend to avoid an async runtime, for example)? From what I can tell, three packages use it, blitz, blitz-shell, and dioxus-native. Or is that out of scope for this PR (I would be very okay with this :P)?

"Allow edits by maintainers" should be on if you feel inspired haha

@SpikeHD SpikeHD marked this pull request as ready for review March 8, 2025 04:50
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 this pull request may close these issues.

1 participant