cache result of fetch_files_from_pr function

easybuild/tools/github.py

@@ -33,6 +33,7 @@
 import copy
 import getpass
 import glob
+import functools
 import os
 import random
 import re
@@ -377,17 +378,37 @@ def download_repo(repo=GITHUB_EASYCONFIGS_REPO, branch='master', account=GITHUB_
     return extracted_path
 
 
-def fetch_easyblocks_from_pr(pr, path=None, github_user=None):
-    """Fetch patched easyconfig files for a particular PR."""
-    return fetch_files_from_pr(pr, path, github_user, github_repo=GITHUB_EASYBLOCKS_REPO)
+def pr_files_cache(func):
+    """
+    Decorator to cache result of fetch_files_from_pr.
+    """
+    cache = {}
+
+    @functools.wraps(func)
+    def cache_aware_func(pr, path=None, github_user=None, github_account=None, github_repo=None):
+        """Retrieve cached result, or fetch files from PR & cache result."""
+        # cache key is combination of all function arguments (incl. optional ones)
+        key = (pr, github_account, github_repo, path)
+
+        if key in cache and all(os.path.exists(x) for x in cache[key]):
+            _log.info("Using cached value for fetch_files_from_pr for PR #%s (account=%s, repo=%s, path=%s)",
+                      pr, github_account, github_repo, path)
+            return cache[key]
+        else:
+            res = func(pr, path=path, github_user=github_user, github_account=github_account, github_repo=github_repo)
+            cache[key] = res
+            return res
 
+    # expose clear/update methods of cache + cache itself to wrapped function
+    cache_aware_func._cache = cache  # useful in tests
+    cache_aware_func.clear_cache = cache.clear
+    cache_aware_func.update_cache = cache.update
 
-def fetch_easyconfigs_from_pr(pr, path=None, github_user=None):
-    """Fetch patched easyconfig files for a particular PR."""
-    return fetch_files_from_pr(pr, path, github_user, github_repo=GITHUB_EASYCONFIGS_REPO)
+    return cache_aware_func
 
 
-def fetch_files_from_pr(pr, path=None, github_user=None, github_repo=None):
+@pr_files_cache
+def fetch_files_from_pr(pr, path=None, github_user=None, github_account=None, github_repo=None):
     """Fetch patched files for a particular PR."""
 
     if github_user is None:
@@ -410,7 +431,8 @@ def fetch_files_from_pr(pr, path=None, github_user=None, github_repo=None):
         # make sure path exists, create it if necessary
         mkdir(path, parents=True)
 
-    github_account = build_option('pr_target_account')
+    if github_account is None:
+        github_account = build_option('pr_target_account')
 
     if github_repo == GITHUB_EASYCONFIGS_REPO:
         easyfiles = 'easyconfigs'
@@ -496,6 +518,16 @@ def fetch_files_from_pr(pr, path=None, github_user=None, github_repo=None):
     return files
 
 
+def fetch_easyblocks_from_pr(pr, path=None, github_user=None):
+    """Fetch patched easyconfig files for a particular PR."""
+    return fetch_files_from_pr(pr, path, github_user, github_repo=GITHUB_EASYBLOCKS_REPO)
+
+
+def fetch_easyconfigs_from_pr(pr, path=None, github_user=None):
+    """Fetch patched easyconfig files for a particular PR."""
+    return fetch_files_from_pr(pr, path, github_user, github_repo=GITHUB_EASYCONFIGS_REPO)
+
+
 def create_gist(txt, fn, descr=None, github_user=None, github_token=None):
     """Create a gist with the provided text."""
 
@@ -1283,7 +1315,7 @@ def merge_pr(pr):
     pr_target_account = build_option('pr_target_account')
     pr_target_repo = build_option('pr_target_repo') or GITHUB_EASYCONFIGS_REPO
 
-    pr_data, pr_url = fetch_pr_data(pr, pr_target_account, pr_target_repo, github_user, full=True)
+    pr_data, _ = fetch_pr_data(pr, pr_target_account, pr_target_repo, github_user, full=True)
 
     msg = "\n%s/%s PR #%s was submitted by %s, " % (pr_target_account, pr_target_repo, pr, pr_data['user']['login'])
     msg += "you are using GitHub account '%s'\n" % github_user

test/framework/github.py

@@ -126,13 +126,13 @@ def test_fetch_pr_data(self):
             print("Skipping test_fetch_pr_data, no GitHub token available?")
             return
 
-        pr_data, pr_url = gh.fetch_pr_data(1, GITHUB_USER, GITHUB_REPO, GITHUB_TEST_ACCOUNT)
+        pr_data, _ = gh.fetch_pr_data(1, GITHUB_USER, GITHUB_REPO, GITHUB_TEST_ACCOUNT)
 
         self.assertEqual(pr_data['number'], 1)
         self.assertEqual(pr_data['title'], "a pr")
         self.assertFalse(any(key in pr_data for key in ['issue_comments', 'review', 'status_last_commit']))
 
-        pr_data, pr_url = gh.fetch_pr_data(2, GITHUB_USER, GITHUB_REPO, GITHUB_TEST_ACCOUNT, full=True)
+        pr_data, _ = gh.fetch_pr_data(2, GITHUB_USER, GITHUB_REPO, GITHUB_TEST_ACCOUNT, full=True)
         self.assertEqual(pr_data['number'], 2)
         self.assertEqual(pr_data['title'], "an open pr (do not close this please)")
         self.assertTrue(pr_data['issue_comments'])
@@ -327,6 +327,67 @@ def test_fetch_easyconfigs_from_pr(self):
             except URLError as err:
                 print("Ignoring URLError '%s' in test_fetch_easyconfigs_from_pr" % err)
 
+    def test_fetch_files_from_pr_cache(self):
+        """Test caching for fetch_files_from_pr."""
+        if self.skip_github_tests:
+            print("Skipping test_fetch_files_from_pr_cache, no GitHub token available?")
+            return
+
+        init_config(build_options={
+            'pr_target_account': gh.GITHUB_EB_MAIN,
+        })
+
+        # clear cache first, to make sure we start with a clean slate
+        gh.fetch_files_from_pr.clear_cache()
+        self.assertFalse(gh.fetch_files_from_pr._cache)
+
+        pr7159_filenames = [
+            'DOLFIN-2018.1.0.post1-foss-2018a-Python-3.6.4.eb',
+            'OpenFOAM-5.0-20180108-foss-2018a.eb',
+            'OpenFOAM-5.0-20180108-intel-2018a.eb',
+            'OpenFOAM-6-foss-2018b.eb',
+            'OpenFOAM-6-intel-2018a.eb',
+            'OpenFOAM-v1806-foss-2018b.eb',
+            'PETSc-3.9.3-foss-2018a.eb',
+            'SCOTCH-6.0.6-foss-2018a.eb',
+            'SCOTCH-6.0.6-foss-2018b.eb',
+            'SCOTCH-6.0.6-intel-2018a.eb',
+            'Trilinos-12.12.1-foss-2018a-Python-3.6.4.eb'
+        ]
+        pr7159_files = gh.fetch_easyconfigs_from_pr(7159, path=self.test_prefix, github_user=GITHUB_TEST_ACCOUNT)
+        self.assertEqual(sorted(pr7159_filenames), sorted(os.path.basename(f) for f in pr7159_files))
+
+        # check that cache has been populated for PR 7159
+        self.assertEqual(len(gh.fetch_files_from_pr._cache.keys()), 1)
+
+        # github_account value is None (results in using default 'easybuilders')
+        cache_key = (7159, None, 'easybuild-easyconfigs', self.test_prefix)
+        self.assertTrue(cache_key in gh.fetch_files_from_pr._cache.keys())
+
+        cache_entry = gh.fetch_files_from_pr._cache[cache_key]
+        self.assertEqual(sorted([os.path.basename(f) for f in cache_entry]), sorted(pr7159_filenames))
+
+        # same query should return result from cache entry
+        res = gh.fetch_easyconfigs_from_pr(7159, path=self.test_prefix, github_user=GITHUB_TEST_ACCOUNT)
+        self.assertEqual(res, pr7159_files)
+
+        # inject entry in cache and check result of matching query
+        pr_id = 12345
+        tmpdir = os.path.join(self.test_prefix, 'easyblocks-pr-12345')
+        pr12345_files = [
+            os.path.join(tmpdir, 'foo.py'),
+            os.path.join(tmpdir, 'bar.py'),
+        ]
+        for fp in pr12345_files:
+            write_file(fp, '')
+
+        # github_account value is None (results in using default 'easybuilders')
+        cache_key = (pr_id, None, 'easybuild-easyblocks', tmpdir)
+        gh.fetch_files_from_pr.update_cache({cache_key: pr12345_files})
+
+        res = gh.fetch_easyblocks_from_pr(12345, tmpdir)
+        self.assertEqual(sorted(pr12345_files), sorted(res))
+
     def test_fetch_latest_commit_sha(self):
         """Test fetch_latest_commit_sha function."""
         if self.skip_github_tests: