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@curii.com>

diff --git a/sdk/python/arvados/retry.py b/sdk/python/arvados/retry.py
index e93624a5d110fa8f935dde6adc83dca477bd9341..2168146a4bc2de6f935a1d628bd4153a0a30f12f 100644 (file)
--- 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 d8136f4ac568079a0906b5a89587698e7e3e6f4c..e54f387b3951a944cef30375cafc671502d2366c 100644 (file)
--- 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 2d020059374b3dc3f42fd95010ebd675605ec929..bcf784d13003a1de826e87d10f04d0a2f4a8d7d5 100644 (file)
--- 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)))