move-coop / parsons

A python library of connectors for the progressive community.
https://www.parsonsproject.org/
Other
261 stars 132 forks source link

APIConnector should handle retry logic in a standard way. #750

Closed alxmrs closed 1 year ago

alxmrs commented 2 years ago

Requests dispatched with APIConnector should centrally handle retries with exponential backoff and jitter.

Detailed Description

As described in this comment (https://github.com/move-coop/parsons/pull/670#discussion_r973633500), retrying request errors is non-trivial. Further, this is a concern for every single API request (that would be nice to not have to put back on to the user). Since the APIConnector class is pending a refactor #736, I recommend that retry logic be included.

Context

I noticed the second example of a error-prone retry pattern in the mysql_to_googlesheets.py script: https://github.com/move-coop/parsons/blob/1411927f818133e485e6f9571f1ef0fabb8ca9ff/useful_resources/sample_code/mysql_to_googlesheets.py#L43

Ideally, we'd include a 3p dependency to handle retries so we wouldn't need to reinvent the wheel (pun intended). Users could add a decorator to their functions to add retry logic throughout the codebase, but especially on the APIConnector.

Possible Implementation

This implementation looks good to me: https://pypi.org/project/backoff/

Priority

medium

shaunagm commented 1 year ago

I'm going to close this issue in favor of #736 which should include the retry logic standardization in addition to other improvements.