search

civisanalytics / civis-python

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

[APIC-79] - reconfigure retry logic using tenacity #401

Closed byndcivilization closed 4 years ago

byndcivilization commented 4 years ago

This reimplements retry logic using tenacity. Requirements are: [X] retry on is [429, 502, 503, and 504] [X] retry on ['HEAD', 'TRACE', 'GET', 'PUT', 'OPTIONS', 'DELETE'] [X] Retry-After header is present, we use that value for the retry interval. [X] Retry POSTs on 429s and 503s. [X] default max total time 10 minutes [X] default max tries = 10 [X] exponential backoff with full jitter

byndcivilization commented 4 years ago

Okay @skomanduri it looks like you were right on the urllib part. Bifurcation of different force lists for different verbs seems painful. This is at 60-70ish% so i wanted to check in on approach before i started on the respect_retry_after_header part, some issues with retries on the cli and tests. theres also still some tearout work and I'm not sure how you want me to handle file buffering retries. It seems like the best course here is to leave what we have in place and add some jitter to the back off calculation. I had a working backoff algo on a different branch that I can port back in.

byndcivilization commented 4 years ago

Okay @skomanduri how does this look. Tests still need work but otherwise i think this works. Theres some funniness with the interaction of retry conditions in that if we raise a connection error for status the codes will not be available in as part of the exception object. This retry after header class and only the status code check gets around this and makes the response headers available for inspection.

byndcivilization commented 4 years ago

Okay @skomanduri this should be ready for review

mheilman commented 4 years ago

just a minor thing: could you please add a changelog entry?

byndcivilization commented 4 years ago

@mheilman yeah for sure. this is probably a good time to ask about release process. do we just bump the version here and release or is there a cadence that we follow.

mheilman commented 4 years ago

do we just bump the version here and release or is there a cadence that we follow.

There isn't a set cadence for releases. The list of unreleased changes is getting pretty long right now, so I was thinking of making one after this and maybe one more PR, so you can just leave the version number as is.

byndcivilization commented 4 years ago

rad thanks