refactor: Remove tokio dependency for blocking ureq and curl transports #422
Conversation
These transport backends don't need the overhead of a tokio runtime and all the dependency crates that come with it, when they are already blocking on their own. A simple Rust thread and mpsc channel is enough. The transport thread implementation is duplicated between a tokio and non-tokio module that are used by reqwest/surf, and curl/ureq respectively. See also #419 (comment) where this was briefly discussed.
Don't worry too much about surf, certainly not in this PR. It's fine to leave this just about async/sync transports. The issue with surf is that is supports both async-std as well as tokio, so really the async version of the thread should have an implementations for both these runtimes. |
|
thanks for this! I agree with @Swatinem that this is fine as is as well without further refactoring. so if you are fine with that as well we can merge as-is. |
|
@flub yes please, all set for merging! It's probably already considered, but I'd love a new release with these changes 😬 |
|
An unrelated comment to this PR: When cloning all branches from this repo are adopted with it, though most appear to be stale remnants from closed PRs: https://github.com/getsentry/sentry-rust/branches/stale - would you mind deleting them for cleanliness? |
what happens to closed PRs when you delete their branch? do they keep giving useful info? seems a bit of lost history if they'd become useless. I'm kind of used to having many useless branches in git, it's a side-effect of how git works and it doesn't cost much i think, they're mostly just a ref afaik. |
History should still be accessible though the GitHub UI and fetchable from
Branches are cheap but do cause the commits they reference to be downloaded too, in a non-shallow clone. It's not computing/storage cost as the changes are generally tiny, but the mental overhead in seeing a bunch of unused, "meaningless" branches lingering around. I'm more used to clean upstream repos and PRs from forks (every branch has meaning, such as "stable" branches for older tags), though Fine to leave it as it is if that's your explicit preference, wasn't sure if these were left sitting around by accident. |
|
Most of those branches are mine ;-) And they contain a few ideas that I was kicking around very early, I would rather keep those if I ever come around to revisit that. But sure, some are also useless by now, but I don’t really want to look at each and every one in detail. I guess thats more of an argument to just get rid of them, but meh. |
These transport backends don't need the overhead of a tokio runtime and all the dependency crates that come with it, when they are already blocking on their own. A simple Rust thread and mpsc channel is enough. The transport thread implementation is duplicated between a tokio and non-tokio module that are used by reqwest/surf, and curl/ureq respectively.
See also #419 (comment) where this was briefly discussed.
TODO
fn new()signature and the contents of the thread closure...surf, which has been mentioned multiple times in not needingtokio?