epwalsh / rust-cached-path

🦀 Rust utility for accessing both local and remote files through a unified interface
Apache License 2.0
29 stars 14 forks source link

refactor crate to use async/tokio over blocking calls #75

Open critocrito opened 9 months ago

critocrito commented 9 months ago

Thanks a lot for this crate. The current use of reqwest::blocking panics when used in an async environment. One workaround would be to use spawn_blocking but I thought this might create issues with large downloads.

This commit keeps all functionality as is but uses the Tokio runtime to make it work in an async environment. This means in turn that the current crate does not work in a sync environment. Omitted are any updates regarding documentation.

#[tokio::main]
async fn main() {
    let cache_dir = dirs::cache_dir().unwrap();
    let remote = "https://...";
    let local_path = "my/download/path";

    let cache = Cache::builder()
        .dir(cache_dir)
        .progress_bar(None)
        .build()
        .unwrap();

    let path = self
        .cache
        .cached_path_with_options(&remote, &Options::default().subdir(&local_path))
        .await
        .unwrap();

    println!("{:?}", path);
}

I don't know if you are interested at all in making rust-cached-path work on async. If you are, maybe I can improve the commit and make the crate dual use (sync and async). Not sure though what the best approach would be. In any case I wanted to share this commit with you.

epwalsh commented 9 months ago

Hey @critocrito, thanks for this!

I would LOVE to get this working with async, but also feel strongly about keeping backwards compat by continuing to support sync. Maybe we use a feature flag to separate the two?

critocrito commented 9 months ago

Oh, that's great, thanks. I'd be happy to create a proper pull request by the end of the week. A blocking feature could be the default, and a tokio feature could be opt-in. I'm unsure how to switch between async versions of the same function using feature flags without duplicating its function body. Would a crate like https://github.com/fMeow/maybe-async-rs maybe help? I have no prior experience with it. What do you think? The most significant change will be the trait implementation of Write/AsyncWrite for DownloadWrapper. But this would be simple to hide behind the feature flag.

epwalsh commented 9 months ago

maybe_async looks promising! But if that proves too complex, I'm fine with just duplicating functions for now. Obviously that's not a great way to write code in general, but we can worry about unifying the code later.