genouest / biomaj-download

Download microservice for BioMAJ
GNU Affero General Public License v3.0
1 stars 7 forks source link

Add a configurable mechanism to retry download when it fails #20

Closed duboism closed 4 years ago

duboism commented 4 years ago

This PR creates a configurable mechanism to retry a download if it fails.

Previously, only downloaders based on CurlDownloader had such a mechanism which was rather simple (try a fixed number of time without any pause between attempts).

The new mechanism is based on the tenacity module which is designed for such purpose. This allows fine-grained control over how to wait between retries (for example use an exponential backoff sleeping between attempts) and when to stop (for example: try a certain number of times without exceeding a given amount of time). The implementation is generic (i.e. most of the work is done in DownloaderInterface) thanks to previous refactoring efforts.

The PR introduces 2 new options for downloaders:

The configuration of the options is based on the simpleeval module which allows to parse strings like stop_after_attempt(3) | stop_after_delay(5) into tenacity objects. Default values for are added in 2 separate PR in the biomaj-core module: genouest/biomaj-core#6 and genouest/biomaj-core#7.

One of the side-effect of this is that we can factor the check of archives in InterfaceDownload so that all downloaders can use it (previously, only CurlDownloader did). See 098eb7c.

We considered being a bit smart on permanent errors i.e. don't retry when there is a way to check that it won't work for instance when the file doesn't exist or on FTP permanent errors. However, in our experience, so called permanent errors may well be temporary. Therefore downloaders always retry whatever the error. In some cases, this is a waste of time but generally this is worth it.

Status:

osallou commented 4 years ago

To ease following pr status, could you add [wip] in pr title?

duboism commented 4 years ago

To ease following pr status, could you add [wip] in pr title?

Done (I was not sure we could change it back).

duboism commented 4 years ago

I have merged #21 in this branch which was needed for IRODS tests. I also added some tests for iRODS and RSYNC.