@@ -123,9 +123,14 @@ GitLab server can sometimes return a transient HTTP error. | |||
| 123 | 123 | python-gitlab can automatically retry in such case, when | |
| 124 | 124 | ``retry_transient_errors`` argument is set to ``True``. When enabled, | |
| 125 | 125 | HTTP error codes 500 (Internal Server Error), 502 (502 Bad Gateway), | |
| 126 | - 503 (Service Unavailable), and 504 (Gateway Timeout) are retried. | ||
| 127 | - Additionally the HTTP error code 409 (Conflict) is retried if the text message | ||
| 128 | - mentions "Resource lock". It will retry until reaching the ``max_retries`` | ||
| 126 | + 503 (Service Unavailable), 504 (Gateway Timeout), and Cloudflare | ||
| 127 | + errors (520-530) are retried. | ||
| 128 | + | ||
| 129 | + Additionally, HTTP error code 409 (Conflict) is retried if the reason | ||
| 130 | + is a | ||
| 131 | + `Resource lock <https://gitlab.com/gitlab-org/gitlab/-/blob/443c12cf3b238385db728f03b2cdbb4f17c70292/lib/api/api.rb#L111>`__. | ||
| 132 | + | ||
| 133 | + It will retry until reaching the ``max_retries`` | ||
| 129 | 134 | value. By default, ``retry_transient_errors`` is set to ``False`` and an | |
| 130 | 135 | exception is raised for these errors. | |
| 131 | 136 | ||
@@ -751,13 +751,20 @@ def http_request( | |||
| 751 | 751 | if 200 <= result.status_code < 300: | |
| 752 | 752 | return result.response | |
| 753 | 753 | ||
| 754 | - if (429 == result.status_code and obey_rate_limit) or ( | ||
| 755 | - ( | ||
| 756 | - result.status_code in gitlab.const.RETRYABLE_TRANSIENT_ERROR_CODES | ||
| 757 | - or (result.status_code == 409 and "Resource lock" in result.reason) | ||
| 758 | - ) | ||
| 759 | - and retry_transient_errors | ||
| 760 | - ): | ||
| 754 | + def should_retry() -> bool: | ||
| 755 | + if result.status_code == 429 and obey_rate_limit: | ||
| 756 | + return True | ||
| 757 | + | ||
| 758 | + if not retry_transient_errors: | ||
| 759 | + return False | ||
| 760 | + if result.status_code in gitlab.const.RETRYABLE_TRANSIENT_ERROR_CODES: | ||
| 761 | + return True | ||
| 762 | + if result.status_code == 409 and "Resource lock" in result.reason: | ||
| 763 | + return True | ||
| 764 | + | ||
| 765 | + return False | ||
| 766 | + | ||
| 767 | + if should_retry(): | ||
| 761 | 768 | # Response headers documentation: | |
| 762 | 769 | # https://docs.gitlab.com/ee/user/admin_area/settings/user_and_ip_rate_limits.html#response-headers | |
| 763 | 770 | if max_retries == -1 or cur_retries < max_retries: | |
@@ -372,6 +372,63 @@ def response_callback( | |||
| 372 | 372 | assert "http://example.com/api/v4/user/status" in error_message | |
| 373 | 373 | ||
| 374 | 374 | ||
| 375 | + def test_http_request_on_409_resource_lock_retries(gl_retry): | ||
| 376 | + url = "http://localhost/api/v4/user" | ||
| 377 | + retried = False | ||
| 378 | + | ||
| 379 | + def response_callback( | ||
| 380 | + response: requests.models.Response, | ||
| 381 | + ) -> requests.models.Response: | ||
| 382 | + """We need a callback that adds a resource lock reason only on first call""" | ||
| 383 | + nonlocal retried | ||
| 384 | + | ||
| 385 | + if not retried: | ||
| 386 | + response.reason = "Resource lock" | ||
| 387 | + | ||
| 388 | + retried = True | ||
| 389 | + return response | ||
| 390 | + | ||
| 391 | + with responses.RequestsMock(response_callback=response_callback) as rsps: | ||
| 392 | + rsps.add( | ||
| 393 | + method=responses.GET, | ||
| 394 | + url=url, | ||
| 395 | + status=409, | ||
| 396 | + match=helpers.MATCH_EMPTY_QUERY_PARAMS, | ||
| 397 | + ) | ||
| 398 | + rsps.add( | ||
| 399 | + method=responses.GET, | ||
| 400 | + url=url, | ||
| 401 | + status=200, | ||
| 402 | + match=helpers.MATCH_EMPTY_QUERY_PARAMS, | ||
| 403 | + ) | ||
| 404 | + response = gl_retry.http_request("get", "/user") | ||
| 405 | + | ||
| 406 | + assert response.status_code == 200 | ||
| 407 | + | ||
| 408 | + | ||
| 409 | + def test_http_request_on_409_resource_lock_without_retry_raises(gl): | ||
| 410 | + url = "http://localhost/api/v4/user" | ||
| 411 | + | ||
| 412 | + def response_callback( | ||
| 413 | + response: requests.models.Response, | ||
| 414 | + ) -> requests.models.Response: | ||
| 415 | + """Without retry, this will fail on the first call""" | ||
| 416 | + response.reason = "Resource lock" | ||
| 417 | + return response | ||
| 418 | + | ||
| 419 | + with responses.RequestsMock(response_callback=response_callback) as req_mock: | ||
| 420 | + req_mock.add( | ||
| 421 | + method=responses.GET, | ||
| 422 | + url=url, | ||
| 423 | + status=409, | ||
| 424 | + match=helpers.MATCH_EMPTY_QUERY_PARAMS, | ||
| 425 | + ) | ||
| 426 | + with pytest.raises(GitlabHttpError) as excinfo: | ||
| 427 | + gl.http_request("get", "/user") | ||
| 428 | + | ||
| 429 | + assert excinfo.value.response_code == 409 | ||
| 430 | + | ||
| 431 | + | ||
| 375 | 432 | @responses.activate | |
| 376 | 433 | def test_get_request(gl): | |
| 377 | 434 | url = "http://localhost/api/v4/projects" | |
0 commit comments