Skip to content

Conversation

@critocrito
Copy link

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.

The current use of `reqwest::blocking` panics when used in an async environment. 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);
}
```
@epwalsh
Copy link
Owner

epwalsh commented Jan 9, 2024

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
Copy link
Author

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
Copy link
Owner

epwalsh commented Jan 10, 2024

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.

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.

2 participants