Dec 19, 2022 · Oct 18, 2022 · Dec 5, 2022 · Dec 5, 2022
diff --git a/docs/api-usage-advanced.rst b/docs/api-usage-advanced.rst
 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

diff --git a/gitlab/client.py b/gitlab/client.py
            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:
                    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:
diff --git a/tests/unit/test_gitlab_http_methods.py b/tests/unit/test_gitlab_http_methods.py
    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

    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"
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
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:
nejch marked this conversation as resolved. Show resolvedHide resolved
		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:
		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
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,
nejch marked this conversation as resolved. Show resolvedHide resolved
		) -> 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 nejchDec 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(
nejch marked this conversation as resolved. Show resolvedHide resolved
		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