[arvados] updated: 2.6.0-121-g5d0b20514

git repository hosting git at public.arvados.org
Thu May 18 12:57:36 UTC 2023


Summary of changes:
 sdk/python/arvados/retry.py    | 6 +-----
 sdk/python/tests/test_api.py   | 7 +++++--
 sdk/python/tests/test_retry.py | 4 ++--
 3 files changed, 8 insertions(+), 9 deletions(-)

       via  5d0b205148026b44c7f1bd30448c5429d0918d16 (commit)
       via  11cf3f097fa7d9f10c0131791249c56aab6839a6 (commit)
      from  e3962b41ad01780c1b173892f0a5c947449a0ed8 (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.


commit 5d0b205148026b44c7f1bd30448c5429d0918d16
Author: Brett Smith <brett.smith at curii.com>
Date:   Thu May 18 08:48:13 2023 -0400

    12684: Test that plain 403 responses are not retried
    
    Requested in review.
    
    Arvados-DCO-1.1-Signed-off-by: Brett Smith <brett.smith at curii.com>

diff --git a/sdk/python/tests/test_api.py b/sdk/python/tests/test_api.py
index e54f387b3..8667d5160 100644
--- a/sdk/python/tests/test_api.py
+++ b/sdk/python/tests/test_api.py
@@ -168,7 +168,10 @@ class ArvadosApiTest(run_test_server.TestCaseWithServers):
 
     def test_4xx_not_retried(self):
         client = arvados.api('v1', num_retries=3)
-        for code in [400, 401, 404, 422]:
+        # Note that googleapiclient does retry 403 *if* the response JSON
+        # includes flags that say the request was denied by rate limiting.
+        # An empty JSON response like we use here should not be retried.
+        for code in [400, 401, 403, 404, 422]:
             with self.subTest(f'error {code}'), mock.patch('time.sleep'):
                 with mock_api_responses(
                         client,

commit 11cf3f097fa7d9f10c0131791249c56aab6839a6
Author: Brett Smith <brett.smith at curii.com>
Date:   Thu May 18 08:36:56 2023 -0400

    12684: Stop retrying 422 responses in PySDK
    
    The original motivation for this was to retry when the API server was
    having database connectivity problems. The feeling eight years later is
    that things have changed enough that, on balance, this isn't worth
    retrying anymore.
    
    I don't think this will have any real impact on current Arvados
    software. In the main branch as I write this,
    `check_http_response_status` only gets called in five places. Three of
    those are in the main `arvados` module for job and task utilities, which
    presumably nobody is using anymore. The other two talk to Keep, which
    only returns 422 for hash mismatches, where a retry will definitely
    never succeed.
    
    Arvados-DCO-1.1-Signed-off-by: Brett Smith <brett.smith at curii.com>

diff --git a/sdk/python/arvados/retry.py b/sdk/python/arvados/retry.py
index e93624a5d..2168146a4 100644
--- a/sdk/python/arvados/retry.py
+++ b/sdk/python/arvados/retry.py
@@ -27,7 +27,7 @@ from collections import deque
 import arvados.errors
 
 _HTTP_SUCCESSES = set(range(200, 300))
-_HTTP_CAN_RETRY = set([408, 409, 422, 423, 500, 502, 503, 504])
+_HTTP_CAN_RETRY = set([408, 409, 423, 500, 502, 503, 504])
 
 class RetryLoop(object):
     """Coordinate limited retries of code.
@@ -200,10 +200,6 @@ def check_http_response_success(status_code):
     * Any 2xx result returns `True`.
 
     * A select few status codes, or any malformed responses, return `None`.
-      422 Unprocessable Entity is in this category.  This may not meet the
-      letter of the HTTP specification, but the Arvados API server will
-      use it for various server-side problems like database connection
-      errors.
 
     * Everything else returns `False`.  Note that this includes 1xx and
       3xx status codes.  They don't indicate success, and you can't
diff --git a/sdk/python/tests/test_api.py b/sdk/python/tests/test_api.py
index d8136f4ac..e54f387b3 100644
--- a/sdk/python/tests/test_api.py
+++ b/sdk/python/tests/test_api.py
@@ -38,7 +38,7 @@ if not mimetypes.inited:
 class ArvadosApiTest(run_test_server.TestCaseWithServers):
     MAIN_SERVER = {}
     ERROR_HEADERS = {'Content-Type': mimetypes.types_map['.json']}
-    RETRIED_4XX = frozenset([408, 409, 422, 423])
+    RETRIED_4XX = frozenset([408, 409, 423])
 
     def api_error_response(self, code, *errors):
         return (fake_httplib2_response(code, **self.ERROR_HEADERS),
@@ -168,7 +168,7 @@ class ArvadosApiTest(run_test_server.TestCaseWithServers):
 
     def test_4xx_not_retried(self):
         client = arvados.api('v1', num_retries=3)
-        for code in [400, 401, 404]:
+        for code in [400, 401, 404, 422]:
             with self.subTest(f'error {code}'), mock.patch('time.sleep'):
                 with mock_api_responses(
                         client,
diff --git a/sdk/python/tests/test_retry.py b/sdk/python/tests/test_retry.py
index 2d0200593..bcf784d13 100644
--- a/sdk/python/tests/test_retry.py
+++ b/sdk/python/tests/test_retry.py
@@ -174,14 +174,14 @@ class CheckHTTPResponseSuccessTestCase(unittest.TestCase):
         self.check_is(True, *list(range(200, 207)))
 
     def test_obvious_stops(self):
-        self.check_is(False, 424, 426, 428, 431,
+        self.check_is(False, 422, 424, 426, 428, 431,
                       *list(range(400, 408)) + list(range(410, 420)))
 
     def test_obvious_retries(self):
         self.check_is(None, 500, 502, 503, 504)
 
     def test_4xx_retries(self):
-        self.check_is(None, 408, 409, 422, 423)
+        self.check_is(None, 408, 409, 423)
 
     def test_5xx_failures(self):
         self.check_is(False, 501, *list(range(505, 512)))

-----------------------------------------------------------------------


hooks/post-receive
-- 




More information about the arvados-commits mailing list