Refactor Pooch.is_available to use downloaders #322
Conversation
The method checks if a file is available without downloading it. It used an implementation independent of the downloaders and only supported simple HTTP and FTP. To allow for more flexibility, allow the function to take a *downloader* argument. Downloaders had to be modified to include an optional check_only=False argument to do the checking and return a bool instead of None. Pooch.is_available raises an exception if the argument isn't supported by a given downloader, so we can retain backwards compatibility. Allowed refactoring the FTP is_available test to use the pytest-localftpserver instead of relying on a public server somewhere (the UNAVCO one we used was decommissioned).
|
@andersy005 would you have a bit of time to take a look at this PR? You were the last to work on the file availability checks. Thanks! |
|
Just have to resolve some linting issues and add a test for the failure condition. But otherwise, some quick feedback on this would be appreciated 🙂 |
|
cc @santisoler who implemented this originally as well |
There was a problem hiding this comment.
Thank you for your patience, @leouieda ( i was on vacation when you last pinged me).
This looks solid to me 👍🏽. i like the idea of off-loading the availability checks to the downloader instead of special casing this in core.
|
Thanks @andersy005! No problem and I hope I didn't intrude on your time off. I appreciate the feedback. Hopefully with this we can add extend support to the DOI downloader as well in the future. |
The method checks if a file is available without downloading it. It used
an implementation independent of the downloaders and only supported
simple HTTP and FTP. To allow for more flexibility, allow the function
to take a downloader argument. Downloaders had to be modified to
include an optional check_only=False argument to do the checking and
return a bool instead of None. Pooch.is_available raises an exception if
the argument isn't supported by a given downloader, so we can retain
backwards compatibility. Allowed refactoring the FTP is_available test
to use the pytest-localftpserver instead of relying on a public server
somewhere (the UNAVCO one we used was decommissioned).
Relevant issues/PRs:
Should fix the CI failures seen in #318