civisanalytics / civis-python

Civis API Python Client
BSD 3-Clause "New" or "Revised" License
34 stars 26 forks source link

Retry on Intermittent S3 Errors #272

Closed waltaskew closed 5 years ago

waltaskew commented 5 years ago

Recently hit an error here: https://github.com/civisanalytics/civis-python/blob/7a08b8c238ba1a6f447ddee68e4ddfc78a4ea560/civis/io/_tables.py#L346 where S3 returned an intermittent 500 on our GET to retrieve an object, causing the read_civis_sql call to raise an exception. It'd be nice if we had a retry loop for S3 interactions to avoid these intermittent failures.

keithing commented 5 years ago

@stephen-hoover, did you run into this before?

mheilman commented 5 years ago

Should we do something like this, except without the Civis API-specific header and maybe just the regular Retry class?

mheilman commented 5 years ago

Oh, it looks like we already have a decorator for this.

stephen-hoover commented 5 years ago

There's a couple of functions in parallel which handle retries for files; see https://github.com/civisanalytics/civis-python/blob/c220a5244573e37f5687138059029f8c0bc7b957/civis/parallel.py#L470L574 . I'm not sure why the download retries weren't built into the get file functionality itself. I think that file uploads and downloads have gone through some changes since we put in the code in parallel.