8000 refactor(client): move retry logic into utility · python-gitlab/python-gitlab@3235c48 · GitHub
[go: up one dir, main page]
Skip to content

Commit 3235c48

Browse files
nejchmax-wittig
nejch
authored and
max-wittig
committed
refactor(client): move retry logic into utility
1 parent fe5e608 commit 3235c48

File tree

3 files changed

+127
-34
lines changed

3 files changed

+127
-34
lines changed

‎gitlab/client.py

Lines changed: 9 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22

33
import os
44
import re
5-
import time
65
from typing import (
76
Any,
87
BinaryIO,
@@ -718,7 +717,12 @@ def http_request(
718717
send_data = self._backend.prepare_send_data(files, post_data, raw)
719718
opts["headers"]["Content-type"] = send_data.content_type
720719

721-
cur_retries = 0
720+
retry = utils.Retry(
721+
max_retries=max_retries,
722+
obey_rate_limit=obey_rate_limit,
723+
retry_transient_errors=retry_transient_errors,
724+
)
725+
722726
while True:
723727
try:
724728
result = self._backend.http_request(
@@ -733,46 +737,17 @@ def http_request(
733737
**opts,
734738
)
735739
except (requests.ConnectionError, requests.exceptions.ChunkedEncodingError):
736-
if retry_transient_errors and (
737-
max_retries == -1 or cur_retries < max_retries
738-
):
739-
wait_time = 2**cur_retries * 0.1
740-
cur_retries += 1
741-
time.sleep(wait_time)
740+
if retry.handle_retry():
742741
continue
743-
744742
raise
745743

746744
self._check_redirects(result.response)
747745

748746
if 200 <= result.status_code < 300:
749747
return result.response
750748

751-
def should_retry() -> bool:
752-
if result.status_code == 429 and obey_rate_limit:
753-
return True
754-
755-
if not retry_transient_errors:
756-
return False
757-
if result.status_code in gitlab.const.RETRYABLE_TRANSIENT_ERROR_CODES:
758-
return True
759-
if result.status_code == 409 and "Resource lock" in result.reason:
760-
return True
761-
762-
return False
763-
764-
if should_retry():
765-
# Response headers documentation:
766-
# https://docs.gitlab.com/ee/user/admin_area/settings/user_and_ip_rate_limits.html#response-headers
767-
if max_retries == -1 or cur_retries < max_retries:
768-
wait_time = 2**cur_retries * 0.1
769-
if "Retry-After" in result.headers:
770-
wait_time = int(result.headers["Retry-After"])
771-
elif "RateLimit-Reset" in result.headers:
772-
wait_time = int(result.headers["RateLimit-Reset"]) - time.time()
773-
cur_retries += 1
774-
time.sleep(wait_time)
775-
continue
749+
if retry.handle_retry_on_status(result):
750+
continue
776751

777752
error_message = result.content
778753
try:

‎gitlab/utils.py

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
import email.message
33
import logging
44
import pathlib
5+
import time
56
import traceback
67
import urllib.parse
78
import warnings
@@ -10,6 +11,7 @@
1011
import requests
1112

1213
from gitlab import const, types
14+
from gitlab._backends import requests_backend
1315

1416

1517
class _StdoutStream:
@@ -85,6 +87,64 @@ def response_content(
8587
return None
8688

8789

90+
class Retry:
91+
def __init__(
92+
self,
93+
max_retries: int,
94+
obey_rate_limit: Optional[bool] = True,
95+
retry_transient_errors: Optional[bool] = False,
96+
) -> None:
97+
self.cur_retries = 0
98+
self.max_retries = max_retries
99+
self.obey_rate_limit = obey_rate_limit
100+
self.retry_transient_errors = retry_transient_errors
101+
102+
def _retryable_status_code(
103+
self,
104+
result: requests_backend.RequestsResponse,
105+
) -> bool:
106+
if result.status_code == 429 and self.obey_rate_limit:
107+
return True
108+
109+
if not self.retry_transient_errors:
110+
return False
111+
if result.status_code in const.RETRYABLE_TRANSIENT_ERROR_CODES:
112+
return True
113+
if result.status_code == 409 and "Resource lock" in result.reason:
114+
return True
115+
116+
return False
117+
118+
def handle_retry_on_status(self, result: requests_backend.RequestsResponse) -> bool:
119+
if not self._retryable_status_code(result):
120+
return False
121+
122+
# Response headers documentation:
123+
# https://docs.gitlab.com/ee/user/admin_area/settings/user_and_ip_rate_limits.html#response-headers
124+
if self.max_retries == -1 or self.cur_retries < self.max_retries:
125+
wait_time = 2**self.cur_retries * 0.1
126+
if "Retry-After" in result.headers:
127+
wait_time = int(result.headers["Retry-After"])
128+
elif "RateLimit-Reset" in result.headers:
129+
wait_time = int(result.headers["RateLimit-Reset"]) - time.time()
130+
self.cur_retries += 1
131+
time.sleep(wait_time)
132+
return True
133+
134+
return False
135+
136+
def handle_retry(self) -> bool:
137+
if self.retry_transient_errors and (
138+
self.max_retries == -1 or self.cur_retries < self.max_retries
139+
):
140+
wait_time = 2**self.cur_retries * 0.1
141+
self.cur_retries += 1
142+
time.sleep(wait_time)
143+
return True
144+
145+
return False
146+
147+
88148
def _transform_types(
89149
data: Dict[str, Any],
90150
custom_types: Dict[str, Any],

‎tests/unit/test_retry.py

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
import time
2+
from unittest import mock
3+
4+
import pytest
5+
import requests
6+
7+
from gitlab import utils
8+
from gitlab._backends import requests_backend
9+
10+
11+
def test_handle_retry_on_status_ignores_unknown_status_code():
12+
retry = utils.Retry(max_retries=1, retry_transient_errors=True)
13+
response = requests.Response()
14+
response.status_code = 418
15+
backend_response = requests_backend.RequestsResponse(response)
16+
17+
assert retry.handle_retry_on_status(backend_response) is False
18+
19+
20+
def test_handle_retry_on_status_accepts_retry_after_header(
21+
monkeypatch: pytest.MonkeyPatch,
22+
):
23+
mock_sleep = mock.Mock()
24+
monkeypatch.setattr(time, "sleep", mock_sleep)
25+
26+
retry = utils.Retry(max_retries=1)
27+
response = requests.Response()
28+
response.status_code = 429
29+
response.headers["Retry-After"] = "1"
30+
backend_response = requests_backend.RequestsResponse(response)
31+
32+
assert retry.handle_retry_on_status(backend_response) is True
33+
assert isinstance(mock_sleep.call_args[0][0], int)
34+
35+
36+
def test_handle_retry_on_status_accepts_ratelimit_reset_header(
37+
monkeypatch: pytest.MonkeyPatch,
38+
):
39+
mock_sleep = mock.Mock()
40+
monkeypatch.setattr(time, "sleep", mock_sleep)
41+
42+
retry = utils.Retry(max_retries=1)
43+
response = requests.Response()
44+
response.status_code = 429
45+
response.headers["RateLimit-Reset"] = str(int(time.time() + 1))
46+
backend_response = requests_backend.RequestsResponse(response)
47+
48+
assert retry.handle_retry_on_status(backend_response) is True
49+
assert isinstance(mock_sleep.call_args[0][0], float)
50+
51+
52+
def test_handle_retry_on_status_returns_false_when_max_retries_reached():
53+
retry = utils.Retry(max_retries=0)
54+
response = requests.Response()
55+
response.status_code = 429
56+
backend_response = requests_backend.RequestsResponse(response)
57+
58+ assert retry.handle_retry_on_status(backend_response) is False

0 commit comments

Comments
 (0)
0