launchpad-reviewers team mailing list archive

[Tom Wardill <mp+406768@xxxxxxxxxxxxxxxxxx>]

Tom Wardill has proposed merging ~twom/launchpad:oci-retry-incomplete-read into launchpad:master.

Commit message:
Add IncompleteRead to list of retries for ociregistryclient

LP: #1938449

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1938449 in Launchpad itself: "OCI Registry Uploads failing"
  https://bugs.launchpad.net/launchpad/+bug/1938449

For more details, see:
https://code.launchpad.net/~twom/launchpad/+git/launchpad/+merge/406768
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~twom/launchpad:oci-retry-incomplete-read into launchpad:master.

diff --git a/lib/lp/oci/model/ociregistryclient.py b/lib/lp/oci/model/ociregistryclient.py
index 2c92c1b..649b89a 100644
--- a/lib/lp/oci/model/ociregistryclient.py
+++ b/lib/lp/oci/model/ociregistryclient.py
@@ -11,6 +11,7 @@ __all__ = [
 import base64
 from functools import partial
 import hashlib
+from http.client import IncompleteRead
 from io import BytesIO
 import json
 import logging
@@ -128,7 +129,8 @@ class OCIRegistryClient:
         wait=wait_fixed(3),
         before=before_log(log, logging.INFO),
         reraise=True,
-        retry=retry_if_exception_type(ConnectionError),
+        retry=(retry_if_exception_type(ConnectionError) |
+               retry_if_exception_type(IncompleteRead)),
         stop=stop_after_attempt(5))
     def _upload(cls, digest, push_rule, fileobj, length, http_client):
         """Upload a blob to the registry, using a given digest.
diff --git a/lib/lp/oci/tests/test_ociregistryclient.py b/lib/lp/oci/tests/test_ociregistryclient.py
index c5b1118..0634ac0 100644
--- a/lib/lp/oci/tests/test_ociregistryclient.py
+++ b/lib/lp/oci/tests/test_ociregistryclient.py
@@ -8,6 +8,7 @@ __metaclass__ = type
 import base64
 from datetime import timedelta
 from functools import partial
+from http.client import IncompleteRead
 import io
 import json
 import os
@@ -551,16 +552,10 @@ class TestOCIRegistryClient(OCIConfigHelperMixin, SpyProxyCallsMixin,
                 'Basic dXNlcjpwYXNzd29yZA==',
                 call.request.headers['Authorization'])
 
-    def test_upload_retries_exception(self):
-        # Use a separate counting mechanism so we're not entirely relying
-        # on tenacity to tell us that it has retried.
-        def count_retries(*args, **kwargs):
-            self.retry_count += 1
-            raise ConnectionError
-
+    def _check_retry_type(self, counting_method, retry_type):
         self.useFixture(MockPatch(
             'lp.oci.model.ociregistryclient.proxy_urlfetch',
-            side_effect=count_retries
+            side_effect=counting_method
         ))
         # Patch sleep so we don't need to change our arguments and the
         # test is instant
@@ -571,7 +566,7 @@ class TestOCIRegistryClient(OCIConfigHelperMixin, SpyProxyCallsMixin,
             self.client._upload(
                 "test-digest", push_rule,
                 None, 0, RegistryHTTPClient(push_rule))
-        except ConnectionError:
+        except retry_type:
             # Check that tenacity and our counting agree
             self.assertEqual(
                 5, self.client._upload.retry.statistics["attempt_number"])
@@ -580,6 +575,24 @@ class TestOCIRegistryClient(OCIConfigHelperMixin, SpyProxyCallsMixin,
             # We should see the original exception, not a RetryError
             raise
 
+    def test_upload_retries_exception(self):
+        # Use a separate counting mechanism so we're not entirely relying
+        # on tenacity to tell us that it has retried.
+        def count_retries(*args, **kwargs):
+            self.retry_count += 1
+            raise ConnectionError
+
+        self._check_retry_type(count_retries, ConnectionError)
+
+    def test_upload_retries_incomplete_read(self):
+        # Use a separate counting mechanism so we're not entirely relying
+        # on tenacity to tell us that it has retried.
+        def count_retries(*args, **kwargs):
+            self.retry_count += 1
+            raise IncompleteRead(b'', 20)
+
+        self._check_retry_type(count_retries, IncompleteRead)
+
     @responses.activate
     def test_upload_put_blob_raises_error(self):
         push_rule = self.build.recipe.push_rules[0]