Skip to content

DE-101 (Python Rewrite): Added retry functionality when fetching Avro schemas#33

Merged
fatimarahman merged 6 commits intomainfrom
de-101-2/add-retry
Jul 30, 2024
Merged

DE-101 (Python Rewrite): Added retry functionality when fetching Avro schemas#33
fatimarahman merged 6 commits intomainfrom
de-101-2/add-retry

Conversation

@fatimarahman
Copy link
Copy Markdown
Contributor

As per our offline discussion. This will be useful whenever the platform API is temporarily down. The retry functionality is inspired by location-hours-poller and patron-info-poller.


def __init__(self, platform_schema_url):
self.logger = create_log("avro_encoder")
retry_policy = Retry(total=2, backoff_factor=4,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does total=2 mean it will try twice or it will retry twice for a total of three requests? We want the second one, but I think we actually want 4 total tries so we can take advantage of the backoff. I think backoff_factor should be like 90 so that it waits 0 seconds between try 1 and try 2, 90 between 2 and 3, and then 180 between 3 and 4. I've always been a little confused about how this works, though, so correct me if that's not how the requests will be timed!

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It will retry twice for a total of three requests! I'll make that update so we get four total tries. Based on my understanding, if we set the backoff_factor to 90, I think it'll be 180 between 2 and 3, etc (since {backoff factor} * (2 ** ({number of previous retries}))). I'll set the backoff_factor to 45 to better suit our purposes (thus we get 90 between 2 and 3, 180 between 3 and 4).

Comment thread src/nypl_py_utils/classes/avro_client.py Outdated
Comment thread tests/test_avro_client.py
Comment thread src/nypl_py_utils/classes/avro_client.py Outdated
@fatimarahman fatimarahman merged commit 6168651 into main Jul 30, 2024
@fatimarahman fatimarahman deleted the de-101-2/add-retry branch July 30, 2024 18:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants