search

replicate / replicate-go

Go client for Replicate
https://replicate.com
Apache License 2.0
65 stars 9 forks source link

Question about whether or not to add errorgroup #15

Closed lll-lll-lll-lll closed 1 year ago

lll-lll-lll-lll commented 1 year ago

I created #14 but close PR. because If an error occurs during the process, another thread will continue execution and execute the http.Get method.

now, I am trying to add the errrgroup pkg and write a process that will stop all threads if an error occurs. Is it ok to create a Download method with the errrgroup pkg added?

mattt commented 1 year ago

Hi @lll-lll-lll-lll. Thanks so much for your work on this library.

I recently implemented a download helper method for predictions in this PR to the replicate CLI.

I used errgroup there to parallelize the downloads, but other cases may have different requirements that warrant a different approach.

Although it could be helpful for this library to provide a built-in Download method, I'm not sure there's a way to do it that doesn't bake in a bunch of assumptions that wouldn't hold for some use cases. Whether to download in parallel, whether to download into a directory or a Zip archive, what to do for outputs that aren't an array of URLs... I think there are probably too many decisions than can fit neatly into a single API.

Instead of providing a Download method, what do you think about providing some example code in the docs?

lll-lll-lll-lll commented 1 year ago

@mattt thanks for your PR of cli.

That is certainly true. This Download method may be trying to handle too many different use cases.

That's a good idea! Can I create some example code with client and server? I would like to make the code as simple as possible.

Thanks for showing me how to do it !