launchpad-reviewers team mailing list archive

[Tom Wardill <tom.wardill@xxxxxxxxxxxxx>]

Tom Wardill has proposed merging ~twom/launchpad:oci-retry-registry-upload-job into launchpad:master.

Commit message:
Use tenacity to retry ConnectionError in OCIRegistryClient

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

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

diff --git a/constraints.txt b/constraints.txt
index ebe72da..bc3d495 100644
--- a/constraints.txt
+++ b/constraints.txt
@@ -241,6 +241,7 @@ meliae==0.2.0.final.0
 mistune==0.8.3
 mock==1.0.1
 mocker==1.1.1
+monotonic==1.5
 netaddr==0.7.19
 oauth==1.0
 oauthlib==3.1.0
@@ -299,6 +300,7 @@ soupsieve==1.9
 storm==0.23+lp415
 subprocess32==3.2.6
 subvertpy==0.9.1
+tenacity==6.1.0
 testresources==0.2.7
 testscenarios==0.4
 timeline==0.0.7
diff --git a/lib/lp/oci/model/ociregistryclient.py b/lib/lp/oci/model/ociregistryclient.py
index e88d0de..511a463 100644
--- a/lib/lp/oci/model/ociregistryclient.py
+++ b/lib/lp/oci/model/ociregistryclient.py
@@ -17,7 +17,17 @@ import json
 import logging
 import tarfile
 
-from requests.exceptions import HTTPError
+from requests.exceptions import (
+    HTTPError,
+    ConnectionError,
+    )
+from tenacity import (
+    before_log,
+    retry,
+    retry_if_exception_type,
+    stop_after_attempt,
+    wait_fixed,
+    )
 from zope.interface import implementer
 
 from lp.oci.interfaces.ociregistryclient import (
@@ -44,6 +54,11 @@ class OCIRegistryClient:
             reference.close()
 
     @classmethod
+    @retry(
+        wait=wait_fixed(30),
+        before=before_log(log, logging.WARNING),
+        retry=retry_if_exception_type(ConnectionError),
+        stop=stop_after_attempt(5))
     def _upload(cls, digest, push_rule, name, fileobj, auth):
         """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 9abf063..69aa4d9 100644
--- a/lib/lp/oci/tests/test_ociregistryclient.py
+++ b/lib/lp/oci/tests/test_ociregistryclient.py
@@ -10,8 +10,16 @@ __metaclass__ = type
 import json
 
 from fixtures import MockPatch
-from requests.exceptions import HTTPError
+from requests.exceptions import (
+    ConnectionError,
+    HTTPError,
+    )
 import responses
+from tenacity import (
+    stop_after_attempt,
+    wait_fixed,
+    RetryError,
+    )
 from testtools.matchers import (
     Equals,
     MatchesDict,
@@ -28,6 +36,7 @@ from lp.testing.layers import LaunchpadZopelessLayer
 class TestOCIRegistryClient(OCIConfigHelperMixin, TestCaseWithFactory):
 
     layer = LaunchpadZopelessLayer
+    retry_count = 0
 
     def setUp(self):
         super(TestOCIRegistryClient, self).setUp()
@@ -51,6 +60,7 @@ class TestOCIRegistryClient(OCIConfigHelperMixin, TestCaseWithFactory):
         self.build = self.factory.makeOCIRecipeBuild()
         self.push_rule = self.factory.makeOCIPushRule(recipe=self.build.recipe)
         self.client = OCIRegistryClient()
+        TestOCIRegistryClient.retry_count = 0
 
     def _makeFiles(self):
         self.factory.makeOCIFile(
@@ -247,3 +257,29 @@ class TestOCIRegistryClient(OCIConfigHelperMixin, TestCaseWithFactory):
             self.assertEqual(
                 'Basic dXNlcjpwYXNzd29yZA==',
                 call.request.headers['Authorization'])
+
+    def test_upload_retries_exception(self):
+        def count_retries(*args, **kwargs):
+            TestOCIRegistryClient.retry_count += 1
+            raise ConnectionError
+
+        self.useFixture(MockPatch(
+            'lp.oci.model.ociregistryclient.urlfetch',
+            side_effect=count_retries
+        ))
+
+        try:
+            # Call the method with slightly different retry arguments
+            # as otherwise we have to wait 5+ minutes...
+            self.client._upload.retry_with(
+                stop=stop_after_attempt(2),
+                wait=wait_fixed(1))(
+                OCIRegistryClient,
+                "test-digest", self.build.recipe.push_rules[0],
+                "test-name", None, ('user', 'password'))
+        except RetryError:
+            pass
+        # We should be able to assert against
+        # self.client._upload.retry.statistics
+        # But there's an interaction with class methods that means it's empty.
+        self.assertEqual(2, TestOCIRegistryClient.retry_count)
diff --git a/setup.py b/setup.py
index 565163b..acb19b7 100644
--- a/setup.py
+++ b/setup.py
@@ -225,6 +225,7 @@ setup(
         'Sphinx',
         'storm',
         'subvertpy',
+        'tenacity',
         'testscenarios',
         'testtools',
         'timeline',