implement exponential backoff in download_file + allow max. 6 attempts to download PR diff in fetch_files_from_pr and fetch_files_from_commit

easybuild/tools/filetools.py

@@ -777,8 +777,17 @@ def det_file_size(http_header):
     return res
 
 
-def download_file(filename, url, path, forced=False, trace=True):
-    """Download a file from the given URL, to the specified path."""
+def download_file(filename, url, path, forced=False, trace=True, max_attempts=3):
+    """
+    Download a file from the given URL, to the specified path.
+
+    :param filename: name of file to download
+    :param url: URL of file to download
+    :param path: path to download file to
+    :param forced: boolean to indicate whether force should be used to write the file
+    :param trace: boolean to indicate whether trace output should be printed
+    :param max_attempts: max. number of attempts to try downloading
+    """
 
     insecure = build_option('insecure_download')
 
@@ -802,7 +811,6 @@ def download_file(filename, url, path, forced=False, trace=True):
 
     # try downloading, three times max.
     downloaded = False
-    max_attempts = 3
     attempt_cnt = 0
 
     # use custom HTTP header
@@ -823,6 +831,8 @@ def download_file(filename, url, path, forced=False, trace=True):
     used_urllib = std_urllib
     switch_to_requests = False
 
+    wait_time_secs = 1
+
     while not downloaded and attempt_cnt < max_attempts:
         attempt_cnt += 1
         try:
@@ -861,6 +871,8 @@ def download_file(filename, url, path, forced=False, trace=True):
                 status_code = err.code
             if status_code == 403 and attempt_cnt == 1:
                 switch_to_requests = True
+            elif status_code == 429:  # too many requests
+                _log.warning(f"Downloading of {url} failed with HTTP status code 429 (Too many requests)")
             elif 400 <= status_code <= 499:
                 _log.warning("URL %s was not found (HTTP response code %s), not trying again" % (url, status_code))
                 break
@@ -887,6 +899,11 @@ def download_file(filename, url, path, forced=False, trace=True):
                 _log.info("Downloading using requests package instead of urllib2")
                 used_urllib = requests
 
+            # exponential backoff
+            wait_time_secs *= 2
+            _log.info(f"Waiting for {wait_time_secs} seconds before trying download of {url} again...")
+            time.sleep(wait_time_secs)
+
     if downloaded:
         _log.info("Successful download of file %s from url %s to path %s" % (filename, url, path))
         if trace:

easybuild/tools/github.py

@@ -546,9 +546,14 @@ def fetch_files_from_pr(pr, path=None, github_user=None, github_account=None, gi
                                        github_user=github_user)
 
     # determine list of changed files via diff
-    diff_fn = os.path.basename(pr_data['diff_url'])
+    diff_url = pr_data['diff_url']
+    diff_fn = os.path.basename(diff_url)
     diff_filepath = os.path.join(path, diff_fn)
-    download_file(diff_fn, pr_data['diff_url'], diff_filepath, forced=True, trace=False)
+    # max. 8 attempts -> max. 2^8 = 128 secs of waiting time in download_file
+    max_attempts = 8
+    download_file(diff_fn, diff_url, diff_filepath, forced=True, trace=False, max_attempts=max_attempts)
+    if not os.path.exists(diff_filepath):
+        raise EasyBuildError(f"Failed to download {diff_url}, even after {max_attempts} attempts and being patient...")
     diff_txt = read_file(diff_filepath)
     _log.debug("Diff for PR #%s:\n%s", pr, diff_txt)
 

test/framework/filetools.py

@@ -557,6 +557,21 @@ def test_download_file(self):
         downloads = glob.glob(target_location + '*')
         self.assertEqual(len(downloads), 1)
 
+        ft.remove_file(target_location)
+
+        # with max attempts set to 0, nothing gets downloaded
+        with self.mocked_stdout_stderr():
+            res = ft.download_file(fn, source_url, target_location, max_attempts=0)
+        self.assertEqual(res, None)
+        downloads = glob.glob(target_location + '*')
+        self.assertEqual(len(downloads), 0)
+
+        with self.mocked_stdout_stderr():
+            res = ft.download_file(fn, source_url, target_location, max_attempts=3)
+        self.assertEqual(res, target_location, "'download' of local file works")
+        downloads = glob.glob(target_location + '*')
+        self.assertEqual(len(downloads), 1)
+
         # non-existing files result in None return value
         with self.mocked_stdout_stderr():
             self.assertEqual(ft.download_file(fn, 'file://%s/nosuchfile' % test_dir, target_location), None)