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.

Sign up for GitHub

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

Improve handling of Jira's retry-after handling #1825

Merged
merged 2 commits into from
Mar 21, 2024
Merged

Improve handling of Jira's retry-after handling #1825

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5a14fda
Respect the "Retry-After" times requested by Jira
matthias-bach-by Feb 26, 2024
c7e5b53
Also retry requests where Jira specifies a Retry-after of 0 seconds
matthias-bach-by Feb 26, 2024
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
8 changes: 6 additions & 2 deletions jira/resilientsession.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -315,7 +315,9 @@ def __recoverable(
if response.status_code in recoverable_error_codes:
retry_after = response.headers.get("Retry-After")
if retry_after:
suggested_delay = int(retry_after) # Do as told
suggested_delay = 2 * max(
int(retry_after), 1
) # Do as told but always wait at least a little
elif response.status_code == HTTPStatus.TOO_MANY_REQUESTS:
suggested_delay = 10 * 2**counter # Exponential backoff

Expand All @@ -326,7 +328,9 @@ def __recoverable(
is_recoverable = suggested_delay > 0
if is_recoverable:
# Apply jitter to prevent thundering herd
delay = min(self.max_retry_delay, suggested_delay) * random.random()
delay = min(self.max_retry_delay, suggested_delay) * random.uniform(
0.5, 1.0
)
LOG.warning(
f"Got recoverable error from {request_method} {url}, will retry [{counter}/{self.max_retries}] in {delay}s. Err: {msg}" # type: ignore[str-bytes-safe]
)
Expand Down
20 changes: 15 additions & 5 deletions tests/test_resilientsession.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,13 +65,17 @@ def tearDown(self):


# Retry test data tuples: (status_code, with_rate_limit_header, with_retry_after_header, retry_expected)
with_rate_limit = with_retry_after = True
without_rate_limit = without_retry_after = False
with_rate_limit = True
with_retry_after = 1
without_rate_limit = False
without_retry_after = None
status_codes_retries_test_data = [
# Always retry 429 responses
(HTTPStatus.TOO_MANY_REQUESTS, with_rate_limit, with_retry_after, True),
(HTTPStatus.TOO_MANY_REQUESTS, with_rate_limit, 0, True),
(HTTPStatus.TOO_MANY_REQUESTS, with_rate_limit, without_retry_after, True),
(HTTPStatus.TOO_MANY_REQUESTS, without_rate_limit, with_retry_after, True),
(HTTPStatus.TOO_MANY_REQUESTS, without_rate_limit, 0, True),
(HTTPStatus.TOO_MANY_REQUESTS, without_rate_limit, without_retry_after, True),
# Retry 503 responses only when 'Retry-After' in headers
(HTTPStatus.SERVICE_UNAVAILABLE, with_rate_limit, with_retry_after, True),
Expand Down Expand Up @@ -103,10 +107,11 @@ def test_status_codes_retries(
mocked_request_method: Mock,
status_code: int,
with_rate_limit_header: bool,
with_retry_after_header: bool,
with_retry_after_header: int | None,
retry_expected: bool,
):
RETRY_AFTER_HEADER = {"Retry-After": "1"}
RETRY_AFTER_SECONDS = with_retry_after_header or 0
RETRY_AFTER_HEADER = {"Retry-After": f"{RETRY_AFTER_SECONDS}"}
RATE_LIMIT_HEADERS = {
"X-RateLimit-FillRate": "1",
"X-RateLimit-Interval-Seconds": "1",
Expand All @@ -124,7 +129,7 @@ def test_status_codes_retries(

mocked_response: Response = Response()
mocked_response.status_code = status_code
if with_retry_after_header:
if with_retry_after_header is not None:
mocked_response.headers.update(RETRY_AFTER_HEADER)
if with_rate_limit_header:
mocked_response.headers.update(RATE_LIMIT_HEADERS)
Expand All @@ -141,6 +146,11 @@ def test_status_codes_retries(
assert mocked_request_method.call_count == expected_number_of_requests
assert mocked_sleep_method.call_count == expected_number_of_sleep_invocations

for actual_sleep in (
call_args.args[0] for call_args in mocked_sleep_method.call_args_list
):
assert actual_sleep >= RETRY_AFTER_SECONDS


errors_parsing_test_data = [
(403, {"x-authentication-denied-reason": "err1"}, "", ["err1"]),
Expand Down
Loading