archive: allow configurable retries

MarcKe · MarcKe · commit 8f083215e4c6 · 2025-03-28T09:35:18.000+01:00
some backends might be quite unreliable, so a single retry is not sufficient
making the retries configurable can mitigate this issue
diff --git a/doc/manual/configuration.rst b/doc/manual/configuration.rst
@@ -2290,10 +2290,12 @@ Type: Dictionary or list of dictionaries
 
 The ``archive`` key configures the default binary artifact server(s) that
 should be used. It is either directly an archive backend entry or a list of
-archive backends. For each entry at least the ``backend`` key must be
-specified. Optionally there can be a ``name`` key that is used in log output
-and a ``flags`` key that receives a list of various flags, in particular for
-what operations the backend might be used. See the following list for possible flags.
+archive backends. For each entry at least the ``backend`` key must be specified.
+Optionally there can be a ``name`` key that is used in log output, a ``retries`` key
+that determines the amount of retries for failed operations (default is 1,
+only used for `http` backend) and a ``flags`` key that receives a list of various
+flags, in particular for what operations the backend might be used. See the
+following list for possible flags.
 The default is ``[download, upload]``.
 
 ``download``
@@ -2372,6 +2374,8 @@ Example::
    archive:
       backend: http
       url: "http://localhost:8001/upload"
+      retries: 2
+      name: "http-backend"
 
 HTTP basic authentication is supported. The user name and password must be put
 in the URL. Be careful to escape special characters of the password with proper
diff --git a/pym/bob/archive.py b/pym/bob/archive.py
@@ -872,16 +872,17 @@ def __init__(self, spec):
         super().__init__(spec)
         self.__url = urllib.parse.urlparse(spec["url"])
         self._webdav = WebDav(self.__url, spec.get("sslVerify", True))
+        self._retries = spec.get("retries", 1)
 
     def __retry(self, request):
-        retry = True
+        retries = self._retries
         while True:
             try:
                 return (True, request())
             except (HTTPException, OSError) as e:
                 self._webdav._resetConnection()
-                if not retry: return (False, e)
-                retry = False
+                if retries == 0: return (False, e)
+                retries -= 1
 
     def _canManage(self):
         return True
diff --git a/pym/bob/input.py b/pym/bob/input.py
@@ -2999,6 +2999,7 @@ def __init__(self):
         httpArchive = baseArchive.copy()
         httpArchive["url"] = HttpUrlValidator()
         httpArchive[schema.Optional("sslVerify")] = bool
+        httpArchive[schema.Optional("retries")] = PositiveValidator()
         shellArchive = baseArchive.copy()
         shellArchive.update({
             schema.Optional('download') : str,
@@ -3065,6 +3066,14 @@ def validate(self, data):
             self.__schema.validate(data)
             return (data["name"], data.get("if"))
 
+class PositiveValidator:
+    def validate(self, data):
+        if not isinstance(data, int):
+            raise schema.SchemaError(None, "Value must be an integer")
+        if data < 0:
+            raise schema.SchemaError(None, "Int value must not be negative")
+        return data
+
 class RecipeSet:
     """The RecipeSet corresponds to the project root directory.
 
diff --git a/test/unit/test_archive.py b/test/unit/test_archive.py
@@ -18,6 +18,7 @@
 import threading
 import urllib.parse
 import sys
+from mocks.http_server import HttpServerMock
 
 from bob.archive import DummyArchive, HttpArchive, getArchiver
 from bob.errors import BuildError
@@ -60,7 +61,7 @@ def run(coro):
     with patch('bob.archive.signal.signal'):
         return runInEventLoop(coro)
 
-class BaseTester:
+class Base:
 
     def __createArtifact(self, bid, version="1"):
         bid = hexlify(bid).decode("ascii")
@@ -90,6 +91,23 @@ def __createBuildId(self, bid):
             f.write(b'\x00'*20)
         return name
 
+    def setUp(self):
+        # create repo
+        self.repo = TemporaryDirectory()
+
+        # add artifacts
+        self.dummyFileName = self.__createArtifact(DOWNLOAD_ARITFACT)
+        self.__createArtifact(WRONG_VERSION_ARTIFACT, "0")
+        self.__createBuildId(DOWNLOAD_ARITFACT)
+
+        self.executor = getProcessPoolExecutor()
+
+    def tearDown(self):
+        self.executor.shutdown()
+        self.repo.cleanup()
+
+class BaseTester(Base):
+
     def __testArtifact(self, bid):
         bid = hexlify(bid).decode("ascii")
         artifact = os.path.join(self.repo.name, bid[0:2], bid[2:4], bid[4:] + "-1.tgz")
@@ -152,13 +170,7 @@ def __getSingleArchiveInstance(self, spec):
         return getArchiver(recipes)
 
     def setUp(self):
-        # create repo
-        self.repo = TemporaryDirectory()
-
-        # add artifacts
-        self.dummyFileName = self.__createArtifact(DOWNLOAD_ARITFACT)
-        self.__createArtifact(WRONG_VERSION_ARTIFACT, "0")
-        self.__createBuildId(DOWNLOAD_ARITFACT)
+        super().setUp()
 
         # create ERROR_DOWNLOAD_ARTIFACT that is there but cannot be opened
         bid = hexlify(ERROR_DOWNLOAD_ARTIFACT).decode("ascii")
@@ -178,12 +190,6 @@ def setUp(self):
         with open(name, "wb") as f:
             f.write(b'\x00')
 
-        self.executor = getProcessPoolExecutor()
-
-    def tearDown(self):
-        self.executor.shutdown()
-        self.repo.cleanup()
-
     # standard tests for options
     def testOptions(self):
         """Test that wantDownload/wantUpload options work"""
@@ -672,3 +678,43 @@ def _setArchiveSpec(self, spec):
         spec["download"] = "cp {}/$BOB_REMOTE_ARTIFACT $BOB_LOCAL_ARTIFACT".format(self.repo.name)
         spec["upload"] = "mkdir -p {P}/${{BOB_REMOTE_ARTIFACT%/*}} && cp $BOB_LOCAL_ARTIFACT {P}/$BOB_REMOTE_ARTIFACT".format(P=self.repo.name)
 
+class TestHttpArchiveRetries(Base, TestCase):
+
+    def setUp(self):
+        super().setUp()
+        self.spec = {'backend' : 'http', 'name' : 'http-archive', 'flags' : ['download', 'upload', 'managed']}
+
+    def _getHttpArchiveInstance(self, port):
+        self.spec["url"] = "http://localhost:{}/".format(port)
+        recipes = DummyRecipeSet(self.spec)
+        return getArchiver(recipes)
+
+    def _testRetries(self, r):
+        self.spec['retries'] = r
+        # server will fail as often as retries in spec
+        with HttpServerMock(repoPath=self.repo.name, retries=r) as srv:
+            archive = self._getHttpArchiveInstance(srv.port)
+            archive.wantDownloadLocal(True)
+            with TemporaryDirectory() as tmp:
+                audit = os.path.join(tmp, "audit.json.gz")
+                content = os.path.join(tmp, "workspace")
+                self.assertTrue(run(archive.downloadPackage(DummyStep(), DOWNLOAD_ARITFACT, audit, content,
+                                                            executor=self.executor)))
+        # server fails one more time than retries in archive sepc -> fail
+        with HttpServerMock(repoPath=self.repo.name, retries=r+1) as srv:
+            archive = self._getHttpArchiveInstance(srv.port)
+            archive.wantDownloadLocal(True)
+            with TemporaryDirectory() as tmp:
+                audit = os.path.join(tmp, "audit.json.gz")
+                content = os.path.join(tmp, "workspace")
+                self.assertFalse(run(archive.downloadPackage(DummyStep(), DOWNLOAD_ARITFACT, audit, content,
+                                                             executor=self.executor)))
+
+    def testRetriesWithNoRetries(self):
+        self._testRetries(0)
+
+    def testRetriesWithOneRetry(self):
+        self._testRetries(1)
+
+    def testRetriesWithMultipleRetries(self):
+        self._testRetries(5)
diff --git a/test/unit/test_input_recipeset.py b/test/unit/test_input_recipeset.py
@@ -299,6 +299,53 @@ def testArchiveAppendPrepend(self):
                 },
             ], recipeSet.archiveSpec())
 
+    def testArchiveEntries(self):
+        # valid config
+        self.writeDefault(
+            {
+                "archive": {
+                    "name": "remote",
+                    "backend": "http",
+                    "url": "http://bob.test/main",
+                    "retries": 1
+                }
+            })
+        recipeSet = RecipeSet()
+        recipeSet.parse()
+        self.assertEqual(
+            [{
+                    "name": "remote",
+                    "backend": "http",
+                    "url": "http://bob.test/main",
+                    "retries": 1
+                }], recipeSet.archiveSpec())
+        # string not allowed
+        self.writeDefault(
+            {
+                "archive": {
+                    "name": "remote",
+                    "backend": "http",
+                    "url": "http://bob.test/main",
+                    "retries": "str"
+                }
+            })
+        recipeSet = RecipeSet()
+        with self.assertRaises(ParseError):
+            recipeSet.parse()
+        # negative integer not allowed
+        self.writeDefault(
+            {
+                "archive": {
+                    "name": "remote",
+                    "backend": "http",
+                    "url": "http://bob.test/main",
+                    "retries": -1
+                }
+            })
+        recipeSet = RecipeSet()
+        with self.assertRaises(ParseError):
+            recipeSet.parse()
+
     def testMirrorsAppendPrepend(self):
         """Test pre/fallbackMirrorAppend/Prepend keywords"""
         for prefix in ["pre", "fallback"]:
