feat(github-apps): Do not fail immediately on missing GitHub contexts

freight/checks/github_apps.py

@@ -4,12 +4,18 @@
 from flask import current_app
 
 from freight import http
+from freight.config import redis
+from freight.models import App
 from freight.exceptions import CheckFailed, CheckPending
 
 from .base import Check
 
 ERR_CHECK = "[{status}] {name}: {description} ({details_url})"
 ERR_MISSING_CONTEXT = "{} context was not found"
+ERR_NO_CONTEXT_ON_FIRST_RUN = "No GitHub contexts found (yet)"
+ERR_NO_CONTEXT = "No contexts were present on GitHub"
+
+FIRST_CHECK_TIMEOUT = 120
 
 
 class GitHubAppsContextCheck(Check):
@@ -21,7 +27,31 @@ def get_options(self):
             "repo": {"required": True},
         }
 
-    def check(self, app, sha, config):
+    def _retry_on_first_run(self, condition: bool, app: App, sha: str, reason: str,
+                            msg_pending: str, msg_failed: str):
+        """
+        Retry the check later if the given condition is True and it's the first run
+
+        We need to pass the condition and clean up if it's False because we have no
+        information about deploy ID here. We rely on the fact that only one deploy and check
+        can be executed for the same app at the given moment.
+        """
+        cache_key = f"github_apps:{app.name}:{sha}:{reason}"
+        exists = b'1'
+
+        if condition:
+            if redis.get(cache_key) == exists:
+                # Not the first run => probably a configuration error
+                redis.delete(cache_key)
+                raise CheckFailed(msg_failed)
+            else:
+                # First run => retry
+                redis.set(cache_key, exists, ex=FIRST_CHECK_TIMEOUT)
+                raise CheckPending(msg_pending)
+        else:
+            redis.delete(cache_key)
+
+    def check(self, app: App, sha: str, config: dict):
         token = current_app.config["GITHUB_TOKEN"]
         if not token:
             raise CheckFailed("GITHUB_TOKEN is not set")
@@ -44,8 +74,12 @@ def check(self, app, sha, config):
         resp = http.get(url, headers=headers)
 
         check_runs_dict = resp.json()
-        if not check_runs_dict or not check_runs_dict.get("total_count"):
-            raise CheckFailed("No contexts were present in GitHub")
+        # GitHub is sometimes slow to update commit context, so we can
+        # tolerate empty context list on the first run
+        self._retry_on_first_run(condition=(not check_runs_dict or not check_runs_dict.get("total_count")),
+                                 app=app, sha=sha, reason="empty_context",
+                                 msg_pending=ERR_NO_CONTEXT_ON_FIRST_RUN,
+                                 msg_failed=ERR_NO_CONTEXT)
 
         valid_contexts = set()
         for check_run in check_runs_dict.get("check_runs", []):
@@ -70,5 +104,9 @@ def check(self, app, sha, config):
                 raise CheckFailed(ERR_CHECK.format(**check_run))
             contexts.remove(check_name)
 
-        if contexts:
-            raise CheckFailed(ERR_MISSING_CONTEXT.format(next(iter(contexts))))
+        # Retry on the first iteration, since GitHub might need some time
+        first_context = next(iter(contexts)) if contexts else None
+        self._retry_on_first_run(condition=bool(contexts),
+                                 app=app, sha=sha, reason="missing_context",
+                                 msg_pending=ERR_NO_CONTEXT_ON_FIRST_RUN,
+                                 msg_failed=ERR_MISSING_CONTEXT.format(first_context))

tests/checks/test_github_apps.py

@@ -97,6 +97,8 @@ def test_missing_required_context(self):
 
         config = {"contexts": ["other"], "repo": "getsentry/freight"}
 
+        with pytest.raises(CheckPending):
+            self.check.check(self.app, "abcdefg", config)
         with pytest.raises(CheckFailed):
             self.check.check(self.app, "abcdefg", config)
 
@@ -137,6 +139,8 @@ def test_no_contexts_with_none_defined(self):
 
         config = {"repo": "getsentry/freight"}
 
+        with pytest.raises(CheckPending):
+            self.check.check(self.app, "abcdefg", config)
         with pytest.raises(CheckFailed):
             self.check.check(self.app, "abcdefg", config)