From 3b49e4d61e6f360f1c787aa048edf584aec55278 Mon Sep 17 00:00:00 2001 From: Mitar Date: Wed, 20 Oct 2021 22:41:38 +0200 Subject: [PATCH 1/3] fix: also retry HTTP-based transient errors --- gitlab/client.py | 36 +++++++++++++++++++++++++----------- 1 file changed, 25 insertions(+), 11 deletions(-) diff --git a/gitlab/client.py b/gitlab/client.py index 7e0a402ce..75765f755 100644 --- a/gitlab/client.py +++ b/gitlab/client.py @@ -675,19 +675,33 @@ def http_request( json, data, content_type = self._prepare_send_data(files, post_data, raw) opts["headers"]["Content-type"] = content_type + retry_transient_errors = kwargs.get( + "retry_transient_errors", self.retry_transient_errors + ) cur_retries = 0 while True: - result = self.session.request( - method=verb, - url=url, - json=json, - data=data, - params=params, - timeout=timeout, - verify=verify, - stream=streamed, - **opts, - ) + try: + result = self.session.request( + method=verb, + url=url, + json=json, + data=data, + params=params, + timeout=timeout, + verify=verify, + stream=streamed, + **opts, + ) + except requests.ConnectionError: + if retry_transient_errors and ( + max_retries == -1 or cur_retries < max_retries + ): + wait_time = 2 ** cur_retries * 0.1 + cur_retries += 1 + time.sleep(wait_time) + continue + + raise self._check_redirects(result) From c3ef1b5c1eaf1348a18d753dbf7bda3c129e3262 Mon Sep 17 00:00:00 2001 From: Clayton Walker Date: Wed, 2 Mar 2022 11:34:05 -0700 Subject: [PATCH 2/3] fix: add 52x range to retry transient failures and tests --- gitlab/client.py | 9 ++- tests/unit/test_gitlab_http_methods.py | 98 +++++++++++++++++++++++++- 2 files changed, 103 insertions(+), 4 deletions(-) diff --git a/gitlab/client.py b/gitlab/client.py index 75765f755..c6e9b96c1 100644 --- a/gitlab/client.py +++ b/gitlab/client.py @@ -35,6 +35,8 @@ "{source!r} to {target!r}" ) +RETRYABLE_TRANSIENT_ERROR_CODES = [500, 502, 503, 504] + list(range(520, 531)) + class Gitlab: """Represents a GitLab server connection. @@ -694,9 +696,9 @@ def http_request( ) except requests.ConnectionError: if retry_transient_errors and ( - max_retries == -1 or cur_retries < max_retries + max_retries == -1 or cur_retries < max_retries ): - wait_time = 2 ** cur_retries * 0.1 + wait_time = 2**cur_retries * 0.1 cur_retries += 1 time.sleep(wait_time) continue @@ -712,7 +714,8 @@ def http_request( "retry_transient_errors", self.retry_transient_errors ) if (429 == result.status_code and obey_rate_limit) or ( - result.status_code in [500, 502, 503, 504] and retry_transient_errors + result.status_code in RETRYABLE_TRANSIENT_ERROR_CODES + and retry_transient_errors ): # Response headers documentation: # https://docs.gitlab.com/ee/user/admin_area/settings/user_and_ip_rate_limits.html#response-headers diff --git a/tests/unit/test_gitlab_http_methods.py b/tests/unit/test_gitlab_http_methods.py index a65b53e61..ed962153b 100644 --- a/tests/unit/test_gitlab_http_methods.py +++ b/tests/unit/test_gitlab_http_methods.py @@ -3,6 +3,7 @@ import responses from gitlab import GitlabHttpError, GitlabList, GitlabParsingError, RedirectError +from gitlab.client import RETRYABLE_TRANSIENT_ERROR_CODES from tests.unit import helpers MATCH_EMPTY_QUERY_PARAMS = [responses.matchers.query_param_matcher({})] @@ -51,7 +52,7 @@ def test_http_request_404(gl): @responses.activate -@pytest.mark.parametrize("status_code", [500, 502, 503, 504]) +@pytest.mark.parametrize("status_code", RETRYABLE_TRANSIENT_ERROR_CODES) def test_http_request_with_only_failures(gl, status_code): url = "http://localhost/api/v4/projects" responses.add( @@ -97,6 +98,37 @@ def request_callback(request): assert len(responses.calls) == calls_before_success +@responses.activate +def test_http_request_with_retry_on_method_for_transient_network_failures(gl): + call_count = 0 + calls_before_success = 3 + + url = "http://localhost/api/v4/projects" + + def request_callback(request): + nonlocal call_count + call_count += 1 + status_code = 200 + headers = {} + body = "[]" + + if call_count >= calls_before_success: + return (status_code, headers, body) + raise requests.ConnectionError("Connection aborted.") + + responses.add_callback( + method=responses.GET, + url=url, + callback=request_callback, + content_type="application/json", + ) + + http_r = gl.http_request("get", "/projects", retry_transient_errors=True) + + assert http_r.status_code == 200 + assert len(responses.calls) == calls_before_success + + @responses.activate def test_http_request_with_retry_on_class_for_transient_failures(gl_retry): call_count = 0 @@ -126,6 +158,37 @@ def request_callback(request: requests.models.PreparedRequest): assert len(responses.calls) == calls_before_success +@responses.activate +def test_http_request_with_retry_on_class_for_transient_network_failures(gl_retry): + call_count = 0 + calls_before_success = 3 + + url = "http://localhost/api/v4/projects" + + def request_callback(request: requests.models.PreparedRequest): + nonlocal call_count + call_count += 1 + status_code = 200 + headers = {} + body = "[]" + + if call_count >= calls_before_success: + return (status_code, headers, body) + raise requests.ConnectionError("Connection aborted.") + + responses.add_callback( + method=responses.GET, + url=url, + callback=request_callback, + content_type="application/json", + ) + + http_r = gl_retry.http_request("get", "/projects", retry_transient_errors=True) + + assert http_r.status_code == 200 + assert len(responses.calls) == calls_before_success + + @responses.activate def test_http_request_with_retry_on_class_and_method_for_transient_failures(gl_retry): call_count = 0 @@ -155,6 +218,39 @@ def request_callback(request): assert len(responses.calls) == 1 +@responses.activate +def test_http_request_with_retry_on_class_and_method_for_transient_network_failures( + gl_retry, +): + call_count = 0 + calls_before_success = 3 + + url = "http://localhost/api/v4/projects" + + def request_callback(request): + nonlocal call_count + call_count += 1 + status_code = 200 + headers = {} + body = "[]" + + if call_count >= calls_before_success: + return (status_code, headers, body) + raise requests.ConnectionError("Connection aborted.") + + responses.add_callback( + method=responses.GET, + url=url, + callback=request_callback, + content_type="application/json", + ) + + with pytest.raises(requests.ConnectionError): + gl_retry.http_request("get", "/projects", retry_transient_errors=False) + + assert len(responses.calls) == 1 + + def create_redirect_response( *, response: requests.models.Response, http_method: str, api_path: str ) -> requests.models.Response: From 5cbbf26e6f6f3ce4e59cba735050e3b7f9328388 Mon Sep 17 00:00:00 2001 From: Nejc Habjan Date: Mon, 4 Apr 2022 23:34:11 +0200 Subject: [PATCH 3/3] chore(client): remove duplicate code --- gitlab/client.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/gitlab/client.py b/gitlab/client.py index c6e9b96c1..c6ac0d179 100644 --- a/gitlab/client.py +++ b/gitlab/client.py @@ -710,9 +710,6 @@ def http_request( if 200 <= result.status_code < 300: return result - retry_transient_errors = kwargs.get( - "retry_transient_errors", self.retry_transient_errors - ) if (429 == result.status_code and obey_rate_limit) or ( result.status_code in RETRYABLE_TRANSIENT_ERROR_CODES and retry_transient_errors