Repository: pycontribs/jira

Issue:
Handle 429 rate limiting messages from Jira
**Is your feature request related to a problem? Please describe.**
 Atlassian recommends usage of rate limiting with Jira in this blog post: https://confluence.atlassian.com/adminjiraserver/improving-instance-stability-with-rate-limiting-983794911.html

This would result in getting a 429 status code.  [From what I can tell](https://github.com/pycontribs/jira/blob/91461fd736cd9a37cc136e07402c6f0b1e60170b/jira/resilientsession.py#L100) this library will never retry these requests.

**Describe the solution you'd like**
Use the existing exponential backoff logic to (eventually) retry a request that was ratelimited.

**Describe alternatives you've considered**
I have not really considered any alternative here. 

**Additional context**

Admittedly this may be handled elsewhere and I may have missed it,  I have not rolled out the ratelimiting yet so I do not have real world testing here. 

Discussion:

Patch:
diff --git a/jira/resilientsession.py b/jira/resilientsession.py
index dbd6b9c..ec01b6f 100644
--- a/jira/resilientsession.py
+++ b/jira/resilientsession.py
@@ -88,7 +88,7 @@ def raise_on_error(r: Optional[Response], verb="???", **kwargs):
 class ResilientSession(Session):
     """This class is supposed to retry requests that do return temporary errors.
 
-    At this moment it supports: 502, 503, 504
+    At this moment it supports: 429
     """
 
     def __init__(self, timeout=None):
@@ -113,11 +113,23 @@ class ResilientSession(Session):
                 f"Got ConnectionError [{response}] errno:{response.errno} on {request} {url}\n{vars(response)}\n{response.__dict__}"
             )
         if isinstance(response, Response):
-            if response.status_code in [502, 503, 504, 401]:
-                # 401 UNAUTHORIZED still randomly returned by Atlassian Cloud as of 2017-01-16
+            if response.status_code in [429]:
+                number_of_tokens_issued_per_interval = response.headers[
+                    "X-RateLimit-FillRate"
+                ]
+                token_issuing_rate_interval_seconds = response.headers[
+                    "X-RateLimit-Interval-Seconds"
+                ]
+                maximum_number_of_tokens = response.headers["X-RateLimit-Limit"]
+                retry_after = response.headers["retry-after"]
                 msg = f"{response.status_code} {response.reason}"
-                # 2019-07-25: Disabled recovery for codes above^
-                return False
+                logging.warning(
+                    f"Request rate limited by Jira: request should be retried after {retry_after} seconds.\n"
+                    + f"{number_of_tokens_issued_per_interval} tokens are issued every {token_issuing_rate_interval_seconds} seconds. "
+                    + f"You can accumulate up to {maximum_number_of_tokens} tokens.\n"
+                    + "Consider adding an exemption for the user as explained in: "
+                    + "https://confluence.atlassian.com/adminjiraserver/improving-instance-stability-with-rate-limiting-983794911.html"
+                )
             elif not (
                 response.status_code == 200
                 and len(response.content) == 0


Test patch:
diff --git a/tests/test_resilientsession.py b/tests/test_resilientsession.py
index 831321c..023fc5c 100644
--- a/tests/test_resilientsession.py
+++ b/tests/test_resilientsession.py
@@ -1,6 +1,11 @@
 import logging
+from unittest.mock import Mock, patch
+
+import pytest
+from requests import Response
 
 import jira.resilientsession
+from jira.exceptions import JIRAError
 from tests.conftest import JiraTestCase
 
 
@@ -53,3 +58,43 @@ class ResilientSessionLoggingConfidentialityTests(JiraTestCase):
     def tearDown(self):
         jira.resilientsession.logging.getLogger().removeHandler(self.loggingHandler)
         del self.loggingHandler
+
+
+status_codes_retries_test_data = [
+    (429, 4, 3),
+    (401, 1, 0),
+    (403, 1, 0),
+    (404, 1, 0),
+    (502, 1, 0),
+    (503, 1, 0),
+    (504, 1, 0),
+]
+
+
+@patch("requests.Session.get")
+@patch("time.sleep")
+@pytest.mark.parametrize(
+    "status_code,expected_number_of_retries,expected_number_of_sleep_invocations",
+    status_codes_retries_test_data,
+)
+def test_status_codes_retries(
+    mocked_sleep_method: Mock,
+    mocked_get_method: Mock,
+    status_code: int,
+    expected_number_of_retries: int,
+    expected_number_of_sleep_invocations: int,
+):
+    mocked_response: Response = Response()
+    mocked_response.status_code = status_code
+    mocked_response.headers["X-RateLimit-FillRate"] = "1"
+    mocked_response.headers["X-RateLimit-Interval-Seconds"] = "1"
+    mocked_response.headers["retry-after"] = "1"
+    mocked_response.headers["X-RateLimit-Limit"] = "1"
+    mocked_get_method.return_value = mocked_response
+    session: jira.resilientsession.ResilientSession = (
+        jira.resilientsession.ResilientSession()
+    )
+    with pytest.raises(JIRAError):
+        session.get("mocked_url")
+    assert mocked_get_method.call_count == expected_number_of_retries
+    assert mocked_sleep_method.call_count == expected_number_of_sleep_invocations