Skip to content

Commit d98d948

Browse files
nejchnejch
authored
Merge pull request #1565 from javatarz/master
feat: allow global retry_transient_errors
2 parents a00ec87 + 3b1d3a4 commit d98d948

File tree

6 files changed

+206
-11
lines changed

6 files changed

+206
-11
lines changed

‎docs/api-usage.rst‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -391,6 +391,17 @@ default an exception is raised for these errors.
391391
gl = gitlab.gitlab(url, token, api_version=4)
392392
gl.projects.list(all=True, retry_transient_errors=True)
393393
394+
The default ``retry_transient_errors`` can also be set on the ``Gitlab`` object
395+
and overridden by individual API calls.
396+
397+
.. code-block:: python
398+
399+
import gitlab
400+
import requests
401+
gl = gitlab.gitlab(url, token, api_version=4, retry_transient_errors=True)
402+
gl.projects.list(all=True) # retries due to default value
403+
gl.projects.list(all=True, retry_transient_errors=False) # does not retry
404+
394405
Timeout
395406
-------
396407

‎gitlab/client.py‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,8 @@ class Gitlab(object):
5252
pagination (str): Can be set to 'keyset' to use keyset pagination
5353
order_by (str): Set order_by globally
5454
user_agent (str): A custom user agent to use for making HTTP requests.
55+
retry_transient_errors (bool): Whether to retry after 500, 502, 503, or
56+
504 responses. Defaults to False.
5557
"""
5658

5759
def __init__(
@@ -70,6 +72,7 @@ def __init__(
7072
pagination: Optional[str] = None,
7173
order_by: Optional[str] = None,
7274
user_agent: str = gitlab.const.USER_AGENT,
75+
retry_transient_errors: bool = False,
7376
) -> None:
7477

7578
self._api_version = str(api_version)
@@ -79,6 +82,7 @@ def __init__(
7982
self._url = "%s/api/v%s" % (self._base_url, api_version)
8083
#: Timeout to use for requests to gitlab server
8184
self.timeout = timeout
85+
self.retry_transient_errors = retry_transient_errors
8286
#: Headers that will be used in request to GitLab
8387
self.headers = {"User-Agent": user_agent}
8488

@@ -246,6 +250,7 @@ def from_config(
246250
pagination=config.pagination,
247251
order_by=config.order_by,
248252
user_agent=config.user_agent,
253+
retry_transient_errors=config.retry_transient_errors,
249254
)
250255

251256
def auth(self) -> None:
@@ -511,7 +516,6 @@ def http_request(
511516
files: Optional[Dict[str, Any]] = None,
512517
timeout: Optional[float] = None,
513518
obey_rate_limit: bool = True,
514-
retry_transient_errors: bool = False,
515519
max_retries: int = 10,
516520
**kwargs: Any,
517521
) -> requests.Response:
@@ -531,9 +535,6 @@ def http_request(
531535
timeout (float): The timeout, in seconds, for the request
532536
obey_rate_limit (bool): Whether to obey 429 Too Many Request
533537
responses. Defaults to True.
534-
retry_transient_errors (bool): Whether to retry after 500, 502,
535-
503, or 504 responses. Defaults
536-
to False.
537538
max_retries (int): Max retries after 429 or transient errors,
538539
set to -1 to retry forever. Defaults to 10.
539540
**kwargs: Extra options to send to the server (e.g. sudo)
@@ -598,6 +599,9 @@ def http_request(
598599
if 200 <= result.status_code < 300:
599600
return result
600601

602+
retry_transient_errors = kwargs.get(
603+
"retry_transient_errors", self.retry_transient_errors
604+
)
601605
if (429 == result.status_code and obey_rate_limit) or (
602606
result.status_code in [500, 502, 503, 504] and retry_transient_errors
603607
):

‎gitlab/config.py‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -206,6 +206,20 @@ def __init__(
206206
except Exception:
207207
pass
208208

209+
self.retry_transient_errors = False
210+
try:
211+
self.retry_transient_errors = self._config.getboolean(
212+
"global", "retry_transient_errors"
213+
)
214+
except Exception:
215+
pass
216+
try:
217+
self.retry_transient_errors = self._config.getboolean(
218+
self.gitlab_id, "retry_transient_errors"
219+
)
220+
except Exception:
221+
pass
222+
209223
def _get_values_from_helper(self) -> None:
210224
"""Update attributes that may get values from an external helper program"""
211225
for attr in HELPER_ATTRIBUTES:

‎tests/unit/conftest.py‎

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,15 +9,26 @@ def gl():
99
"http://localhost",
1010
private_token="private_token",
1111
ssl_verify=True,
12-
api_version=4,
12+
api_version="4",
13+
)
14+
15+
16+
@pytest.fixture
17+
def gl_retry():
18+
return gitlab.Gitlab(
19+
"http://localhost",
20+
private_token="private_token",
21+
ssl_verify=True,
22+
api_version="4",
23+
retry_transient_errors=True,
1324
)
1425

1526

1627
# Todo: parametrize, but check what tests it's really useful for
1728
@pytest.fixture
1829
def gl_trailing():
1930
return gitlab.Gitlab(
20-
"http://localhost/", private_token="private_token", api_version=4
31+
"http://localhost/", private_token="private_token", api_version="4"
2132
)
2233

2334

‎tests/unit/test_config.py‎

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,31 @@
8686
"""
8787

8888

89+
def global_retry_transient_errors(value: bool) -> str:
90+
return u"""[global]
91+
default = one
92+
retry_transient_errors={}
93+
[one]
94+
url = http://one.url
95+
private_token = ABCDEF""".format(
96+
value
97+
)
98+
99+
100+
def global_and_gitlab_retry_transient_errors(
101+
global_value: bool, gitlab_value: bool
102+
) -> str:
103+
return u"""[global]
104+
default = one
105+
retry_transient_errors={global_value}
106+
[one]
107+
url = http://one.url
108+
private_token = ABCDEF
109+
retry_transient_errors={gitlab_value}""".format(
110+
global_value=global_value, gitlab_value=gitlab_value
111+
)
112+
113+
89114
@mock.patch.dict(os.environ, {"PYTHON_GITLAB_CFG": "/some/path"})
90115
def test_env_config_present():
91116
assert ["/some/path"] == config._env_config()
@@ -245,3 +270,48 @@ def test_config_user_agent(m_open, path_exists, config_string, expected_agent):
245270

246271
cp = config.GitlabConfigParser()
247272
assert cp.user_agent == expected_agent
273+
274+
275+
@mock.patch("os.path.exists")
276+
@mock.patch("builtins.open")
277+
@pytest.mark.parametrize(
278+
"config_string,expected",
279+
[
280+
pytest.param(valid_config, False, id="default_value"),
281+
pytest.param(
282+
global_retry_transient_errors(True), True, id="global_config_true"
283+
),
284+
pytest.param(
285+
global_retry_transient_errors(False), False, id="global_config_false"
286+
),
287+
pytest.param(
288+
global_and_gitlab_retry_transient_errors(False, True),
289+
True,
290+
id="gitlab_overrides_global_true",
291+
),
292+
pytest.param(
293+
global_and_gitlab_retry_transient_errors(True, False),
294+
False,
295+
id="gitlab_overrides_global_false",
296+
),
297+
pytest.param(
298+
global_and_gitlab_retry_transient_errors(True, True),
299+
True,
300+
id="gitlab_equals_global_true",
301+
),
302+
pytest.param(
303+
global_and_gitlab_retry_transient_errors(False, False),
304+
False,
305+
id="gitlab_equals_global_false",
306+
),
307+
],
308+
)
309+
def test_config_retry_transient_errors_when_global_config_is_set(
310+
m_open, path_exists, config_string, expected
311+
):
312+
fd = io.StringIO(config_string)
313+
fd.close = mock.Mock(return_value=None)
314+
m_open.return_value = fd
315+
316+
cp = config.GitlabConfigParser()
317+
assert cp.retry_transient_errors == expected

‎tests/unit/test_gitlab_http_methods.py‎

Lines changed: 90 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -30,14 +30,99 @@ def resp_cont(url, request):
3030
def test_http_request_404(gl):
3131
@urlmatch(scheme="http", netloc="localhost", path="/api/v4/not_there", method="get")
3232
def resp_cont(url, request):
33-
content = {"Here is wh it failed"}
33+
content = {"Here is why it failed"}
3434
return response(404, content, {}, None, 5, request)
3535

3636
with HTTMock(resp_cont):
3737
with pytest.raises(GitlabHttpError):
3838
gl.http_request("get", "/not_there")
3939

4040

41+
@pytest.mark.parametrize("status_code", [500, 502, 503, 504])
42+
def test_http_request_with_only_failures(gl, status_code):
43+
call_count = 0
44+
45+
@urlmatch(scheme="http", netloc="localhost", path="/api/v4/projects", method="get")
46+
def resp_cont(url, request):
47+
nonlocal call_count
48+
call_count += 1
49+
return response(status_code, {"Here is why it failed"}, {}, None, 5, request)
50+
51+
with HTTMock(resp_cont):
52+
with pytest.raises(GitlabHttpError):
53+
gl.http_request("get", "/projects")
54+
55+
assert call_count == 1
56+
57+
58+
def test_http_request_with_retry_on_method_for_transient_failures(gl):
59+
call_count = 0
60+
calls_before_success = 3
61+
62+
@urlmatch(scheme="http", netloc="localhost", path="/api/v4/projects", method="get")
63+
def resp_cont(url, request):
64+
nonlocal call_count
65+
call_count += 1
66+
status_code = 200 if call_count == calls_before_success else 500
67+
return response(
68+
status_code,
69+
{"Failure is the stepping stone to success"},
70+
{},
71+
None,
72+
5,
73+
request,
74+
)
75+
76+
with HTTMock(resp_cont):
77+
http_r = gl.http_request("get", "/projects", retry_transient_errors=True)
78+
79+
assert http_r.status_code == 200
80+
assert call_count == calls_before_success
81+
82+
83+
def test_http_request_with_retry_on_class_for_transient_failures(gl_retry):
84+
call_count = 0
85+
calls_before_success = 3
86+
87+
@urlmatch(scheme="http", netloc="localhost", path="/api/v4/projects", method="get")
88+
def resp_cont(url, request):
89+
nonlocal call_count
90+
call_count += 1
91+
status_code = 200 if call_count == calls_before_success else 500
92+
return response(
93+
status_code,
94+
{"Failure is the stepping stone to success"},
95+
{},
96+
None,
97+
5,
98+
request,
99+
)
100+
101+
with HTTMock(resp_cont):
102+
http_r = gl_retry.http_request("get", "/projects")
103+
104+
assert http_r.status_code == 200
105+
assert call_count == calls_before_success
106+
107+
108+
def test_http_request_with_retry_on_class_and_method_for_transient_failures(gl_retry):
109+
call_count = 0
110+
calls_before_success = 3
111+
112+
@urlmatch(scheme="http", netloc="localhost", path="/api/v4/projects", method="get")
113+
def resp_cont(url, request):
114+
nonlocal call_count
115+
call_count += 1
116+
status_code = 200 if call_count == calls_before_success else 500
117+
return response(status_code, {"Here is why it failed"}, {}, None, 5, request)
118+
119+
with HTTMock(resp_cont):
120+
with pytest.raises(GitlabHttpError):
121+
gl_retry.http_request("get", "/projects", retry_transient_errors=False)
122+
123+
assert call_count == 1
124+
125+
41126
def test_get_request(gl):
42127
@urlmatch(scheme="http", netloc="localhost", path="/api/v4/projects", method="get")
43128
def resp_cont(url, request):
@@ -66,7 +151,7 @@ def resp_cont(url, request):
66151
def test_get_request_404(gl):
67152
@urlmatch(scheme="http", netloc="localhost", path="/api/v4/not_there", method="get")
68153
def resp_cont(url, request):
69-
content = {"Here is wh it failed"}
154+
content = {"Here is why it failed"}
70155
return response(404, content, {}, None, 5, request)
71156

72157
with HTTMock(resp_cont):
@@ -150,7 +235,7 @@ def test_post_request_404(gl):
150235
scheme="http", netloc="localhost", path="/api/v4/not_there", method="post"
151236
)
152237
def resp_cont(url, request):
153-
content = {"Here is wh it failed"}
238+
content = {"Here is why it failed"}
154239
return response(404, content, {}, None, 5, request)
155240

156241
with HTTMock(resp_cont):
@@ -186,7 +271,7 @@ def resp_cont(url, request):
186271
def test_put_request_404(gl):
187272
@urlmatch(scheme="http", netloc="localhost", path="/api/v4/not_there", method="put")
188273
def resp_cont(url, request):
189-
content = {"Here is wh it failed"}
274+
content = {"Here is why it failed"}
190275
return response(404, content, {}, None, 5, request)
191276

192277
with HTTMock(resp_cont):
@@ -226,7 +311,7 @@ def test_delete_request_404(gl):
226311
scheme="http", netloc="localhost", path="/api/v4/not_there", method="delete"
227312
)
228313
def resp_cont(url, request):
229-
content = {"Here is wh it failed"}
314+
content = {"Here is why it failed"}
230315
return response(404, content, {}, None, 5, request)
231316

232317
with HTTMock(resp_cont):

0 commit comments

Comments
 (0)