Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add retries for addtional response codes #106

Merged
merged 5 commits into from
Jun 10, 2020
Merged
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 35 additions & 2 deletions closeio_api/__init__.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import logging
import time

from random import randint
eengoron marked this conversation as resolved.
Show resolved Hide resolved

import requests

from closeio_api.utils import local_tz_offset
Expand All @@ -9,7 +11,7 @@

# To update the package version, change this variable. This variable is also
# read by setup.py when installing the package.
__version__ = '1.2'
__version__ = '1.3'

class APIError(Exception):
"""Raised when sending a request to the API failed."""
Expand Down Expand Up @@ -110,6 +112,21 @@ def _dispatch(self, method_name, endpoint, api_key=None, data=None,
logging.debug('Request was rate limited, sleeping %d seconds', sleep_time)
time.sleep(sleep_time)
continue

# Retry 503 errors or 502 or 504 erors on GET requests.
elif response.status_code == 503 or (
method_name == "get" and response.status_code in (502, 504)
eengoron marked this conversation as resolved.
Show resolved Hide resolved
):
sleep_time = self._get_randomized_sleep_time_for_error(
response.status_code, retry_count
)
logging.debug(
"Request hit a {}, sleeping for {} seconds".format(
eengoron marked this conversation as resolved.
Show resolved Hide resolved
response.status_code, sleep_time
)
)
time.sleep(sleep_time)
continue

# Break out of the retry loop if the request was successful.
break
Expand All @@ -122,14 +139,30 @@ def _dispatch(self, method_name, endpoint, api_key=None, data=None,
raise APIError(response)

def _get_rate_limit_sleep_time(self, response):
"""Get rate limit window expiration time from response."""
"""Get rate limit window expiration time from response if the response
status code is 429.
"""
try:
data = response.json()
return float(data['error']['rate_reset'])
except (AttributeError, KeyError, ValueError):
logging.exception('Error parsing rate limiting response')
return DEFAULT_RATE_LIMIT_DELAY

def _get_randomized_sleep_time_for_error(self, status_code, retries):
"""Get sleep time for a given status code before we can try the
request again.
"""
if status_code == 503:
return randint(2, 4)
Copy link
Member

Choose a reason for hiding this comment

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

This should also have a backoff.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done in bceed63. Note: I also didn't exponentially back off on this one either, because 503s should be able to be retried practically instantaneously, so it felt like overkill.

If you'd rather that, let me know.


# if it's a 502 or 504 error, we want back off more each time
# before we try again.
elif status_code in (502, 504):
return randint(60, 90) * (retries + 1)
eengoron marked this conversation as resolved.
Show resolved Hide resolved

return DEFAULT_RATE_LIMIT_DELAY

def get(self, endpoint, params=None, timeout=None, **kwargs):
"""Send a GET request to a given endpoint, for example:

Expand Down