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

pspacek · 2022-10-18T12:58:39Z

lmilbaum · 2022-10-18T13:40:04Z

Unit tests might increase the confidence that the code is working as expected. Would you mind adding unit tests?

nejch

@pspacek thanks for the quick work here! Just a few nits.

Regarding unit tests and your question in #2325 (comment).

We have some vendored code from httmock for a similar reason (redirects). I think you might be able to reuse it here?
https://github.com/python-gitlab/python-gitlab/blob/main/tests/unit/helpers.py

See how it's used in the redirect tests:
https://github.com/python-gitlab/python-gitlab/blob/main/tests/unit/test_gitlab_http_methods.py#L310-L317

Let us know if you need help with that :) That's a shame about responses though.

docs/api-usage-advanced.rst

gitlab/client.py

nejch · 2022-10-20T18:24:53Z

And @pspacek we're also ok to go ahead without the unit tests if it's too complicated and we open a follow-up issue to resolve on our end :)

nejch · 2022-12-04T10:09:25Z

Hi @pspacek! Let us know if you'd like to finish this PR, or if we should push to your branch to get this merged (either way you'll be credited as the commit author). Thanks again for the contribution!

pspacek · 2022-12-05T17:02:49Z

@nejch I'm sorry, I have no time to work on this. Feel free to do whatever you like.

nejch · 2022-12-05T20:55:44Z

tests/unit/test_gitlab_http_methods.py

+    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


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

nejch

This LGTM but I added a commit in 25446cf to finish the PR so will need someone else to do the merge.

Thanks again @pspacek!

codecov-commenter · 2022-12-05T21:01:17Z

Codecov Report

Merging #2326 (1a37bab) into main (c7cf0d1) will increase coverage by 0.00%.
The diff coverage is 90.90%.

@@           Coverage Diff           @@
##             main    #2326   +/-   ##
=======================================
  Coverage   96.13%   96.13%           
=======================================
  Files          84       85    +1     
  Lines        5534     5569   +35     
=======================================
+ Hits         5320     5354   +34     
- Misses        214      215    +1

Flag	Coverage Δ
api_func_v4	`82.58% <54.54%> (+<0.01%)`	⬆️
cli_func_v4	`82.83% <45.45%> (-0.06%)`	⬇️
unit	`87.48% <81.81%> (+0.04%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
gitlab/client.py	`98.38% <90.90%> (-0.18%)`	⬇️
gitlab/v4/objects/__init__.py	`100.00% <0.00%> (ø)`
gitlab/v4/objects/resource_groups.py	`100.00% <0.00%> (ø)`
gitlab/v4/objects/projects.py	`98.86% <0.00%> (+<0.01%)`	⬆️

Fixes: python-gitlab#2325

nejch · 2022-12-19T11:21:59Z

Unit tests might increase the confidence that the code is working as expected. Would you mind adding unit tests?

@lmilbaum I've added a commit on top of the author's to rework and add tests (1a37bab). Maybe you can take a look and we can merge before the next release.

tests/unit/test_gitlab_http_methods.py

gitlab/client.py

tests/unit/test_gitlab_http_methods.py

lmilbaum

LGTM

rdesgroppes · 2025-11-28T17:08:21Z

gitlab/client.py

+                    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:


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

…3648) ### Motivation The python-gitlab fix (python-gitlab/python-gitlab#2326) for 409 Resource lock retries checks `result.reason`, but GitLab sends `Conflict` (standard HTTP reason phrase) while `Resource lock` is in the response body. The check never matches, so retries never happen: ``` $ dda inv -- pipeline.auto-cancel-previous-pipelines [...] gitlab.exceptions.GitlabHttpError: 409: 409 Conflict: Resource lock ^ reason ^ response body ``` Example: https://gitlab.ddbuild.io/DataDog/datadog-agent/-/jobs/1262109099 ### What does this PR do? This patch adds HTTP `409` to GitLab's API retriable transient error codes as a lightweight workaround, allowing all 409 errors to be retried when `retry_transient_errors=True` (our case). ### Additional Notes Will file an issue upstream. Co-authored-by: regis.desgroppes <regis.desgroppes@datadoghq.com>

…taDog#43648) ### Motivation The python-gitlab fix (python-gitlab/python-gitlab#2326) for 409 Resource lock retries checks `result.reason`, but GitLab sends `Conflict` (standard HTTP reason phrase) while `Resource lock` is in the response body. The check never matches, so retries never happen: ``` $ dda inv -- pipeline.auto-cancel-previous-pipelines [...] gitlab.exceptions.GitlabHttpError: 409: 409 Conflict: Resource lock ^ reason ^ response body ``` Example: https://gitlab.ddbuild.io/DataDog/datadog-agent/-/jobs/1262109099 ### What does this PR do? This patch adds HTTP `409` to GitLab's API retriable transient error codes as a lightweight workaround, allowing all 409 errors to be retried when `retry_transient_errors=True` (our case). ### Additional Notes Will file an issue upstream. Co-authored-by: regis.desgroppes <regis.desgroppes@datadoghq.com> fcb5852

nejch requested changes Oct 18, 2022

View reviewed changes

docs/api-usage-advanced.rst Outdated Show resolved Hide resolved

gitlab/client.py Outdated Show resolved Hide resolved

nejch force-pushed the http-409-lock-retry branch from fdb1d2c to 25446cf Compare December 5, 2022 20:48

nejch reviewed Dec 5, 2022

View reviewed changes

nejch changed the title ~~Draft: feat(client): automatically retry on HTTP 409 Resource lock~~ feat(client): automatically retry on HTTP 409 Resource lock Dec 5, 2022

nejch approved these changes Dec 5, 2022

View reviewed changes

nejch assigned JohnVillalovos and max-wittig Dec 5, 2022

pspacek and others added 2 commits December 18, 2022 12:40

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

8cfb877

Fixes: python-gitlab#2325

chore: add test, docs, and helper for 409 retries

1a37bab

nejch force-pushed the http-409-lock-retry branch from 25446cf to 1a37bab Compare December 18, 2022 11:47

lmilbaum self-requested a review December 19, 2022 20:02

lmilbaum suggested changes Dec 19, 2022

View reviewed changes

tests/unit/test_gitlab_http_methods.py Show resolved Hide resolved

gitlab/client.py Show resolved Hide resolved

tests/unit/test_gitlab_http_methods.py Show resolved Hide resolved

lmilbaum approved these changes Dec 19, 2022

View reviewed changes

nejch merged commit 3e1c625 into python-gitlab:main Dec 19, 2022

HonkingGoose mentioned this pull request May 11, 2024

Retry GitLab API requests on HTTP 409 Resource lock errors renovatebot/renovate#29006

Closed

rdesgroppes reviewed Nov 28, 2025

View reviewed changes

rdesgroppes mentioned this pull request Nov 28, 2025

cancel-prev-pipelines: do retry on HTTP 409 (Conflict) errors DataDog/datadog-agent#43648

Merged

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

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

Conversation

pspacek commented Oct 18, 2022

Uh oh!

lmilbaum commented Oct 18, 2022

Uh oh!

nejch left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

nejch commented Oct 20, 2022

Uh oh!

nejch commented Dec 4, 2022

Uh oh!

pspacek commented Dec 5, 2022

Uh oh!

nejch Dec 5, 2022

Choose a reason for hiding this comment

Uh oh!

nejch left a comment

Choose a reason for hiding this comment

Uh oh!

codecov-commenter commented Dec 5, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

nejch commented Dec 19, 2022

Uh oh!

Uh oh!

Uh oh!

Uh oh!

lmilbaum left a comment

Choose a reason for hiding this comment

Uh oh!

rdesgroppes Nov 28, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

codecov-commenter commented Dec 5, 2022 •

edited

Loading