Skip to content

feat(client): automatically retry on HTTP 409 Resource lock #2326

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

Merged
nejch merged 2 commits into from
Dec 19, 2022
+81 −7
Merged

feat(client): automatically retry on HTTP 409 Resource lock #2326

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
8cfb877
feat(client): automatically retry on HTTP 409 Resource lock
pspacek Oct 18, 2022
1a37bab
chore: add test, docs, and helper for 409 retries
nejch Dec 5, 2022
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
13 changes: 10 additions & 3 deletions docs/api-usage-advanced.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,9 +123,16 @@ GitLab server can sometimes return a transient HTTP error.
python-gitlab can automatically retry in such case, when
``retry_transient_errors`` argument is set to ``True``. When enabled,
HTTP error codes 500 (Internal Server Error), 502 (502 Bad Gateway),
503 (Service Unavailable), and 504 (Gateway Timeout) are retried. It will retry until reaching
the ``max_retries`` value. By default, ``retry_transient_errors`` is set to ``False`` and an exception
is raised for these errors.
503 (Service Unavailable), 504 (Gateway Timeout), and Cloudflare
errors (520-530) are retried.

Additionally, HTTP error code 409 (Conflict) is retried if the reason
is a
`Resource lock <https://gitlab.com/gitlab-org/gitlab/-/blob/443c12cf3b238385db728f03b2cdbb4f17c70292/lib/api/api.rb#L111>`__.

It will retry until reaching the ``max_retries``
value. By default, ``retry_transient_errors`` is set to ``False`` and an
exception is raised for these errors.

.. code-block:: python

Expand Down
18 changes: 14 additions & 4 deletions gitlab/client.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -779,10 +779,20 @@ def http_request(
if 200 <= result.status_code < 300:
return result

if (429 == result.status_code and obey_rate_limit) or (
result.status_code in gitlab.const.RETRYABLE_TRANSIENT_ERROR_CODES
and retry_transient_errors
):
def should_retry() -> bool:
if result.status_code == 429 and obey_rate_limit:
return True

if not retry_transient_errors:
return False
if result.status_code in gitlab.const.RETRYABLE_TRANSIENT_ERROR_CODES:
return True
if result.status_code == 409 and "Resource lock" in result.reason:
Copy link

@rdesgroppes rdesgroppes Nov 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't seem to work for us - could it be because Resource lock is in the response body rather than the HTTP reason (Conflict)?

return True

return False

if should_retry():
# Response headers documentation:
# https://docs.gitlab.com/ee/user/admin_area/settings/user_and_ip_rate_limits.html#response-headers
if max_retries == -1 or cur_retries < max_retries:
Expand Down
57 changes: 57 additions & 0 deletions tests/unit/test_gitlab_http_methods.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -372,6 +372,63 @@ def response_callback(
assert "http://example.com/api/v4/user/status" in error_message


def test_http_request_on_409_resource_lock_retries(gl_retry):
url = "http://localhost/api/v4/user"
retried = False

def response_callback(
response: requests.models.Response,
) -> requests.models.Response:
"""We need a callback that adds a resource lock reason only on first call"""
nonlocal retried

if not retried:
response.reason = "Resource lock"

retried = True
return response
Comment on lines +379 to +389
Copy link
Member

@nejch nejch Dec 5, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We're mostly doing this because responses doesn't support mocking Response.reason as @pspacek already found out in #2325 (comment).


with responses.RequestsMock(response_callback=response_callback) as rsps:
rsps.add(
method=responses.GET,
url=url,
status=409,
match=helpers.MATCH_EMPTY_QUERY_PARAMS,
)
rsps.add(
method=responses.GET,
url=url,
status=200,
match=helpers.MATCH_EMPTY_QUERY_PARAMS,
)
response = gl_retry.http_request("get", "/user")

assert response.status_code == 200


def test_http_request_on_409_resource_lock_without_retry_raises(gl):
url = "http://localhost/api/v4/user"

def response_callback(
response: requests.models.Response,
) -> requests.models.Response:
"""Without retry, this will fail on the first call"""
response.reason = "Resource lock"
return response

with responses.RequestsMock(response_callback=response_callback) as req_mock:
req_mock.add(
method=responses.GET,
url=url,
status=409,
match=helpers.MATCH_EMPTY_QUERY_PARAMS,
)
with pytest.raises(GitlabHttpError) as excinfo:
gl.http_request("get", "/user")

assert excinfo.value.response_code == 409


@responses.activate
def test_get_request(gl):
url = "http://localhost/api/v4/projects"
Expand Down