Retry the validation as well as the fetch

apps/cli/executables/pexable/productfetcher/productfetcher/exceptions.py

@@ -27,7 +27,7 @@ import traceback
 from pathlib import Path
 
 
-class FetchError(Exception):
+class FetchError(BaseException):
     """
     Some sort of problem has occurred, and the program must end.
     """
@@ -54,7 +54,7 @@ class RetryableFetchError(FetchError):
         super().terminate_fetch()
 
 
-class FileValidationFault(FetchError):
+class FileValidationFault(RetryableFetchError):
     """
     A file has failed to validate for some reason. This is not a retryable error.
     """

apps/cli/executables/pexable/productfetcher/productfetcher/fetchers.py

@@ -41,6 +41,7 @@ from .exceptions import (
 )
 from .interfaces import FetchProgressReporter, FileFetcher, LocatedFile
 from .locations import NgasFile, OracleXml
+from .retry import retry
 
 # pylint: disable=E0401, E0402, W0221
 
@@ -245,18 +246,8 @@ class RetryableFileFetcher(FileFetcherDecorator):
         super().__init__(underlying)
         self.retries = retries
 
-    def do_fetch(self, attempt=1) -> Path:
-        try:
-            return self.underlying.do_fetch()
-        except RetryableFetchError as r_err:
-            if attempt < self.retries:
-                # sleep for 2, 4, 8 seconds between attempts
-                time.sleep(2**attempt)
-                return self.do_fetch(attempt + 1)
-            else:
-                # let's annotate the error with how many times we tried
-                r_err.retries = attempt
-                raise r_err
+    def do_fetch(self) -> Path:
+        return retry(self.underlying.do_fetch, [2**attempt for attempt in range(1, self.retries)])
 
     def __str__(self):
         return f"{self.underlying} (with up to {self.retries} retries)"

apps/cli/executables/pexable/productfetcher/productfetcher/interfaces.py

@@ -42,6 +42,8 @@ from abc import ABC, abstractmethod
 from pathlib import Path
 from typing import List, NamedTuple, Union
 
+from .retry import retry
+
 
 class LocatedFile(abc.ABC):
     """
@@ -170,7 +172,7 @@ class FileFetcher(ABC):
         # now validate with our validators, if we fetched a file
         if result is not None:
             for validator in self.validators:
-                validator.validate(result)
+                retry(lambda: validator.validate(result), [1, 4, 9])
 
         # if we made it here, we have a result to report
         reporter.complete(self.file, result)

apps/cli/executables/pexable/productfetcher/productfetcher/retry.py

+#
+# Copyright (C) 2021 Associated Universities, Inc. Washington DC, USA.
+#
+# This file is part of NRAO Workspaces.
+#
+# Workspaces is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+#
+# Workspaces is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with Workspaces.  If not, see <https://www.gnu.org/licenses/>.
+import time
+from typing import Callable, List, TypeVar
+
+from .exceptions import RetryableFetchError
+
+T = TypeVar("T")
+
+
+def retry(f: Callable[[], T], backoff_times: List[int] = None, retry_number=1) -> T:
+    """
+    Calls f() and returns its result. However, if f() throws an exception, we will retry.
+    Each entry in backoff_times is a length of time (in seconds) to sleep using `time.sleep`.
+    So if the backoff_times = [1, 4, 9], we will sleep 1 second, then 4 seconds, then 9 seconds between retries.
+
+    Also, if an exception should be raised on the last attempt, we will attempt to add a "retries" attribute to
+    it with the number of retries that were attempted.
+
+    :param f: the function to call
+    :param backoff_times: a list of times to wait
+    :param retry_number: the current "retry" (used only for the retries attribute)
+    :return: whatever the result of `f` is
+    """
+    exception = None
+    try:
+        return f()
+    except RetryableFetchError as ex:
+        if backoff_times:
+            # if we have, say, [1, 4, 9] in our list of backoff times,
+            # we will delay for 1 second and then have [4, 9] remaining backoff times
+            delay, *remaining_backoff_times = backoff_times
+
+            # sleep
+            time.sleep(delay)
+
+            return retry(f, remaining_backoff_times, retry_number + 1)
+        else:
+            # if this exception has a slot for "retry count" we should set it
+            if hasattr(ex, "retries"):
+                ex.retries = retry_number
+
+            # now raise the exception
+            raise ex

apps/cli/executables/pexable/productfetcher/tests/test_fetchers.py

@@ -187,8 +187,8 @@ def test_retrying_succeeds(capsys):
 
 def test_retrying_fails(capsys):
     # three failures is too many
-    fail_twice = MagicMock(wraps=FailNTimesFileFetcher(3))
+    fail_thrice = MagicMock(wraps=FailNTimesFileFetcher(3))
     with patch("time.sleep"):
         with pytest.raises(RetryableFetchError):
-            RetryableFileFetcher(fail_twice).do_fetch()
+            RetryableFileFetcher(fail_thrice).do_fetch()
     capsys.readouterr()

apps/cli/executables/pexable/productfetcher/tests/test_retry.py

+#
+# Copyright (C) 2021 Associated Universities, Inc. Washington DC, USA.
+#
+# This file is part of NRAO Workspaces.
+#
+# Workspaces is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+#
+# Workspaces is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with Workspaces.  If not, see <https://www.gnu.org/licenses/>.
+from unittest.mock import MagicMock, call, patch
+
+import pytest
+from productfetcher.exceptions import RetryableFetchError
+from productfetcher.retry import retry
+
+
+class FailNTimes:
+    def __init__(self, n):
+        self.total = n
+        self.current = 1
+
+    def __call__(self, *args, **kwargs):
+        if self.current <= self.total:
+            self.current += 1
+            raise RetryableFetchError(f"Failure #{self.current-1}")
+
+
+def test_fail_n_times():
+    x = FailNTimes(2)
+    with pytest.raises(RetryableFetchError):
+        x()
+    with pytest.raises(RetryableFetchError):
+        x()
+    x()
+
+
+def test_retrying_succeeds(capsys):
+    # two failures is OK
+    with patch("time.sleep") as sleep:
+        fail_twice = MagicMock(wraps=FailNTimes(2))
+        retry(fail_twice, [3, 7])
+        assert sleep.call_count == 2
+        assert sleep.call_args_list == [call(3), call(7)]
+    capsys.readouterr()
+
+
+def test_retrying_fails(capsys):
+    # three failures is too many
+    with patch("time.sleep"):
+        fail_thrice = MagicMock(wraps=FailNTimes(3))
+        with pytest.raises(RetryableFetchError):
+            retry(fail_thrice, [3, 7])
+    capsys.readouterr()