Make Awaiting Workflow Run A User Responsibility

Moves the responsibility of re-requesting a workflow run from the API to
the user. This makes the requests much shorter-lived, and allows the
client to control how they want to handle sleeping and retrying.

This also has the benefit of removing the only real piece of async code,
so now the view is completely sync once again.

Signed-off-by: Hassan Abouelela <[email protected]>

3 files changed, 131 insertions, 131 deletions

diff --git a/pydis_site/apps/api/github_utils.py b/pydis_site/apps/api/github_utils.py
index 70dccdff..707b36e5 100644
--- a/pydis_site/apps/api/github_utils.py
+++ b/pydis_site/apps/api/github_utils.py
@@ -1,17 +1,17 @@
 """Utilities for working with the GitHub API."""
 
-import asyncio
 import datetime
 import math
 
 import httpx
 import jwt
-from asgiref.sync import async_to_sync
 
 from pydis_site import settings
 
-MAX_POLLS = 20
-"""The maximum number of attempts at fetching a workflow run."""
+MAX_RUN_TIME = datetime.timedelta(minutes=3)
+"""The maximum time allowed before an action is declared timed out."""
+ISO_FORMAT_STRING = "%Y-%m-%dT%H:%M:%SZ"
+"""The datetime string format GitHub uses."""
 
 
 class ArtifactProcessingError(Exception):
@@ -44,6 +44,12 @@ class RunTimeoutError(ArtifactProcessingError):
     status = 408
 
 
+class RunPendingError(ArtifactProcessingError):
+    """The requested workflow run is still pending, try again later."""
+
+    status = 202
+
+
 def generate_token() -> str:
     """
     Generate a JWT token to access the GitHub API.
@@ -66,7 +72,7 @@ def generate_token() -> str:
     )
 
 
-async def authorize(owner: str, repo: str) -> httpx.AsyncClient:
+def authorize(owner: str, repo: str) -> httpx.Client:
     """
     Get an access token for the requested repository.
 
@@ -75,7 +81,7 @@ async def authorize(owner: str, repo: str) -> httpx.AsyncClient:
         - POST <app_access_token> to get a token to access the given app
         - GET installation/repositories and check if the requested one is part of those
     """
-    client = httpx.AsyncClient(
+    client = httpx.Client(
         base_url=settings.GITHUB_API,
         headers={"Authorization": f"bearer {generate_token()}"},
         timeout=settings.TIMEOUT_PERIOD,
@@ -83,7 +89,7 @@ async def authorize(owner: str, repo: str) -> httpx.AsyncClient:
 
     try:
         # Get a list of app installations we have access to
-        apps = await client.get("app/installations")
+        apps = client.get("app/installations")
         apps.raise_for_status()
 
         for app in apps.json():
@@ -92,11 +98,11 @@ async def authorize(owner: str, repo: str) -> httpx.AsyncClient:
                 continue
 
             # Get the repositories of the specified owner
-            app_token = await client.post(app["access_tokens_url"])
+            app_token = client.post(app["access_tokens_url"])
             app_token.raise_for_status()
             client.headers["Authorization"] = f"bearer {app_token.json()['token']}"
 
-            repos = await client.get("installation/repositories")
+            repos = client.get("installation/repositories")
             repos.raise_for_status()
 
             # Search for the request repository
@@ -111,44 +117,39 @@ async def authorize(owner: str, repo: str) -> httpx.AsyncClient:
 
     except BaseException as e:
         # Close the client if we encountered an unexpected exception
-        await client.aclose()
+        client.close()
         raise e
 
 
-async def wait_for_run(client: httpx.AsyncClient, run: dict) -> str:
-    """Wait for the provided `run` to finish, and return the URL to its artifacts."""
-    polls = 0
-    while polls <= MAX_POLLS:
-        if run["status"] != "completed":
-            # The action is still processing, wait a bit longer
-            polls += 1
-            await asyncio.sleep(10)
-
-        elif run["conclusion"] != "success":
-            # The action failed, or did not run
-            raise ActionFailedError(f"The requested workflow ended with: {run['conclusion']}")
+def check_run_status(run: dict) -> str:
+    """Check if the provided run has been completed, otherwise raise an exception."""
+    created_at = datetime.datetime.strptime(run["created_at"], ISO_FORMAT_STRING)
+    run_time = datetime.datetime.now() - created_at
 
+    if run["status"] != "completed":
+        if run_time <= MAX_RUN_TIME:
+            raise RunPendingError(
+                f"The requested run is still pending. It was created "
+                f"{run_time.seconds // 60}:{run_time.seconds % 60 :>02} minutes ago."
+            )
         else:
-            # The desired action was found, and it ended successfully
-            return run["artifacts_url"]
+            raise RunTimeoutError("The requested workflow was not ready in time.")
 
-        run = await client.get(run["url"])
-        run.raise_for_status()
-        run = run.json()
+    if run["conclusion"] != "success":
+        # The action failed, or did not run
+        raise ActionFailedError(f"The requested workflow ended with: {run['conclusion']}")
 
-    raise RunTimeoutError("The requested workflow was not ready in time.")
+    # The requested action is ready
+    return run["artifacts_url"]
 
 
-@async_to_sync
-async def get_artifact(
-    owner: str, repo: str, sha: str, action_name: str, artifact_name: str
-) -> str:
+def get_artifact(owner: str, repo: str, sha: str, action_name: str, artifact_name: str) -> str:
     """Get a download URL for a build artifact."""
-    client = await authorize(owner, repo)
+    client = authorize(owner, repo)
 
     try:
         # Get the workflow runs for this repository
-        runs = await client.get(f"/repos/{owner}/{repo}/actions/runs", params={"per_page": 100})
+        runs = client.get(f"/repos/{owner}/{repo}/actions/runs", params={"per_page": 100})
         runs.raise_for_status()
         runs = runs.json()
 
@@ -161,16 +162,16 @@ async def get_artifact(
                 "Could not find a run matching the provided settings in the previous hundred runs."
             )
 
-        # Wait for the workflow to finish
-        url = await wait_for_run(client, run)
+        # Check the workflow status
+        url = check_run_status(run)
 
         # Filter the artifacts, and return the download URL
-        artifacts = await client.get(url)
+        artifacts = client.get(url)
         artifacts.raise_for_status()
 
         for artifact in artifacts.json()["artifacts"]:
             if artifact["name"] == artifact_name:
-                data = await client.get(artifact["archive_download_url"])
+                data = client.get(artifact["archive_download_url"])
                 if data.status_code == 302:
                     return str(data.next_request.url)
 
@@ -180,4 +181,4 @@ async def get_artifact(
         raise NotFoundError("Could not find an artifact matching the provided name.")
 
     finally:
-        await client.aclose()
+        client.close()
diff --git a/pydis_site/apps/api/tests/test_github_utils.py b/pydis_site/apps/api/tests/test_github_utils.py
index dc17d609..78f2979d 100644
--- a/pydis_site/apps/api/tests/test_github_utils.py
+++ b/pydis_site/apps/api/tests/test_github_utils.py
@@ -1,6 +1,4 @@
-import asyncio
 import datetime
-import random
 import unittest
 from unittest import mock
 
@@ -14,16 +12,6 @@ from django.urls import reverse
 from .. import github_utils
 
 
-def patched_raise_for_status(response: httpx.Response):
-    """Fake implementation of raise_for_status which does not need a request to be set."""
-    if response.status_code // 100 != 2:  # pragma: no cover
-        raise httpx.HTTPStatusError(
-            f"Non 2xx response code: {response.status_code}",
-            request=getattr(response, "_request", httpx.Request("GET", "")),
-            response=response
-        )
-
-
 class GeneralUtilityTests(unittest.TestCase):
     """Test the utility methods which do not fit in another class."""
 
@@ -51,53 +39,50 @@ class GeneralUtilityTests(unittest.TestCase):
         self.assertLess(decoded["exp"], (datetime.datetime.now() + delta).timestamp())
 
 
[email protected]("httpx.AsyncClient", autospec=True)
[email protected]("asyncio.sleep", new=mock.AsyncMock(return_value=asyncio.Future))
[email protected]("httpx.Response.raise_for_status", new=patched_raise_for_status)
-class WaitForTests(unittest.IsolatedAsyncioTestCase):
-    """Tests the wait_for utility."""
-
-    async def test_wait_for_successful_run(self, client_mock: mock.Mock):
-        """Test that the wait_for method handles successfully runs."""
-        final_url = "some_url" + str(random.randint(0, 10))
-
-        client_mock.get.side_effect = responses = [
-            httpx.Response(200, json={"status": "queued", "url": ""}),
-            httpx.Response(200, json={"status": "pending", "url": ""}),
-            httpx.Response(200, json={
-                "status": "completed",
-                "conclusion": "success",
-                "url": "",
-                "artifacts_url": final_url
-            })
-        ]
+class WaitForTests(unittest.TestCase):
+    """Tests the check_run_status utility."""
 
-        result = await github_utils.wait_for_run(client_mock, responses[0].json())
-        self.assertEqual(final_url, result)
+    def test_completed_run(self):
+        final_url = "some_url_string_1234"
 
-    async def test_wait_for_failed_run(self, client_mock: mock.Mock):
-        """Test that the wait_for method handles failed runs."""
-        client_mock.get.return_value = httpx.Response(200, json={
+        result = github_utils.check_run_status({
             "status": "completed",
-            "conclusion": "failed",
+            "conclusion": "success",
+            "created_at": datetime.datetime.now().strftime(github_utils.ISO_FORMAT_STRING),
+            "artifacts_url": final_url,
         })
+        self.assertEqual(final_url, result)
 
-        with self.assertRaises(github_utils.ActionFailedError):
-            await github_utils.wait_for_run(client_mock, {"status": "pending", "url": ""})
+    def test_pending_run(self):
+        """Test that a pending run raises the proper exception."""
+        with self.assertRaises(github_utils.RunPendingError):
+            github_utils.check_run_status({
+                "status": "pending",
+                "created_at": datetime.datetime.now().strftime(github_utils.ISO_FORMAT_STRING),
+            })
 
-    async def test_wait_for_timeout(self, client_mock: mock.Mock):
-        """Test that the wait_for method quits after a few attempts."""
-        client_mock.get.side_effect = responses = [
-            httpx.Response(200, json={"status": "pending", "url": ""})
-        ] * (github_utils.MAX_POLLS + 5)
+    def test_timeout_error(self):
+        """Test that a timeout is declared after a certain duration."""
+        # Set the creation time to well before the MAX_RUN_TIME
+        # to guarantee the right conclusion
+        created = (
+            datetime.datetime.now() - github_utils.MAX_RUN_TIME - datetime.timedelta(minutes=10)
+        ).strftime(github_utils.ISO_FORMAT_STRING)
 
         with self.assertRaises(github_utils.RunTimeoutError):
-            await github_utils.wait_for_run(client_mock, responses[0].json())
+            github_utils.check_run_status({"status": "pending", "created_at": created})
+
+    def test_failed_run(self):
+        """Test that a failed run raises the proper exception."""
+        with self.assertRaises(github_utils.ActionFailedError):
+            github_utils.check_run_status({
+                "status": "completed",
+                "conclusion": "failed",
+                "created_at": datetime.datetime.now().strftime(github_utils.ISO_FORMAT_STRING),
+            })
 
 
-async def get_response_authorize(
-    _: httpx.AsyncClient, request: httpx.Request, **__
-) -> httpx.Response:
+def get_response_authorize(_: httpx.Client, request: httpx.Request, **__) -> httpx.Response:
     """
     Helper method for the authorize tests.
 
@@ -141,76 +126,83 @@ async def get_response_authorize(
     return httpx.Response(500, request=request)  # pragma: no cover
 
 
[email protected]("httpx.AsyncClient.send", new=get_response_authorize)
[email protected]("httpx.Client.send", new=get_response_authorize)
 @mock.patch.object(github_utils, "generate_token", new=mock.Mock(return_value="JWT initial token"))
-class AuthorizeTests(unittest.IsolatedAsyncioTestCase):
+class AuthorizeTests(unittest.TestCase):
     """Test the authorize utility."""
 
-    async def test_invalid_apps_auth(self):
+    def test_invalid_apps_auth(self):
         """Test that an exception is raised if authorization was attempted with an invalid token."""
         with mock.patch.object(github_utils, "generate_token", return_value="Invalid token"):
             with self.assertRaises(httpx.HTTPStatusError) as error:
-                await github_utils.authorize("VALID_OWNER", "VALID_REPO")
+                github_utils.authorize("VALID_OWNER", "VALID_REPO")
 
         exception: httpx.HTTPStatusError = error.exception
         self.assertEqual(401, exception.response.status_code)
         self.assertEqual("auth app/installations", exception.response.json()["error"])
 
-    async def test_missing_repo(self):
+    def test_missing_repo(self):
         """Test that an exception is raised when the selected owner or repo are not available."""
         with self.assertRaises(github_utils.NotFoundError):
-            await github_utils.authorize("INVALID_OWNER", "VALID_REPO")
+            github_utils.authorize("INVALID_OWNER", "VALID_REPO")
         with self.assertRaises(github_utils.NotFoundError):
-            await github_utils.authorize("VALID_OWNER", "INVALID_REPO")
+            github_utils.authorize("VALID_OWNER", "INVALID_REPO")
 
-    async def test_valid_authorization(self):
+    def test_valid_authorization(self):
         """Test that an accessible repository can be accessed."""
-        client = await github_utils.authorize("VALID_OWNER", "VALID_REPO")
+        client = github_utils.authorize("VALID_OWNER", "VALID_REPO")
         self.assertEqual("bearer app access token", client.headers.get("Authorization"))
 
 
-async def get_response_get_artifact(request: httpx.Request, **_) -> httpx.Response:
-    """
-    Helper method for the get_artifact tests.
+class ArtifactFetcherTests(unittest.TestCase):
+    """Test the get_artifact utility."""
 
-    Requests are intercepted before being sent out, and the appropriate responses are returned.
-    """
-    path = request.url.path
+    @staticmethod
+    def get_response_get_artifact(request: httpx.Request, **_) -> httpx.Response:
+        """
+        Helper method for the get_artifact tests.
 
-    if "force_error" in path:
-        return httpx.Response(404, request=request)
+        Requests are intercepted before being sent out, and the appropriate responses are returned.
+        """
+        path = request.url.path
 
-    if request.method == "GET":
-        if path == "/repos/owner/repo/actions/runs":
-            return httpx.Response(200, request=request, json={"workflow_runs": [{
-                "name": "action_name",
-                "head_sha": "action_sha"
-            }]})
-        elif path == "/artifact_url":
-            return httpx.Response(200, request=request, json={"artifacts": [{
-                "name": "artifact_name",
-                "archive_download_url": "artifact_download_url"
-            }]})
-        elif path == "/artifact_download_url":
-            response = httpx.Response(302, request=request)
-            response.next_request = httpx.Request("GET", httpx.URL("https://final_download.url"))
-            return response
-
-    # Reaching this point means something has gone wrong
-    return httpx.Response(500, request=request)  # pragma: no cover
+        if "force_error" in path:
+            return httpx.Response(404, request=request)
 
+        if request.method == "GET":
+            if path == "/repos/owner/repo/actions/runs":
+                return httpx.Response(
+                    200, request=request, json={"workflow_runs": [{
+                        "name": "action_name",
+                        "head_sha": "action_sha"
+                    }]}
+                )
+            elif path == "/artifact_url":
+                return httpx.Response(
+                    200, request=request, json={"artifacts": [{
+                        "name": "artifact_name",
+                        "archive_download_url": "artifact_download_url"
+                    }]}
+                )
+            elif path == "/artifact_download_url":
+                response = httpx.Response(302, request=request)
+                response.next_request = httpx.Request(
+                    "GET",
+                    httpx.URL("https://final_download.url")
+                )
+                return response
 
-class ArtifactFetcherTests(unittest.IsolatedAsyncioTestCase):
-    """Test the get_artifact utility."""
+        # Reaching this point means something has gone wrong
+        return httpx.Response(500, request=request)  # pragma: no cover
 
     def setUp(self) -> None:
         self.call_args = ["owner", "repo", "action_sha", "action_name", "artifact_name"]
-        self.client = httpx.AsyncClient(base_url="https://example.com")
+        self.client = httpx.Client(base_url="https://example.com")
 
         self.patchers = [
-            mock.patch.object(self.client, "send", new=get_response_get_artifact),
+            mock.patch.object(self.client, "send", new=self.get_response_get_artifact),
             mock.patch.object(github_utils, "authorize", return_value=self.client),
-            mock.patch.object(github_utils, "wait_for_run", return_value="artifact_url"),
+            mock.patch.object(github_utils, "check_run_status", return_value="artifact_url"),
         ]
 
         for patcher in self.patchers:
@@ -266,7 +258,7 @@ class GitHubArtifactViewTests(django.test.TestCase):
         }
         cls.url = reverse("api:github-artifacts", kwargs=cls.kwargs)
 
-    async def test_successful(self, artifact_mock: mock.Mock):
+    def test_successful(self, artifact_mock: mock.Mock):
         """Test a proper response is returned with proper input."""
         artifact_mock.return_value = "final download url"
         result = self.client.get(self.url)
@@ -274,7 +266,7 @@ class GitHubArtifactViewTests(django.test.TestCase):
         self.assertIsInstance(result, rest_framework.response.Response)
         self.assertEqual({"url": artifact_mock.return_value}, result.data)
 
-    async def test_failed_fetch(self, artifact_mock: mock.Mock):
+    def test_failed_fetch(self, artifact_mock: mock.Mock):
         """Test that a proper error is returned when the request fails."""
         artifact_mock.side_effect = github_utils.NotFoundError("Test error message")
         result = self.client.get(self.url)
diff --git a/static-builds/netlify_build.py b/static-builds/netlify_build.py
index 13cd0279..a473bd91 100644
--- a/static-builds/netlify_build.py
+++ b/static-builds/netlify_build.py
@@ -8,6 +8,7 @@
 
 import json
 import os
+import time
 import zipfile
 from pathlib import Path
 from urllib import parse
@@ -29,7 +30,7 @@ if __name__ == "__main__":
     print(f"Fetching download URL from {download_url}")
     response = httpx.get(download_url, follow_redirects=True)
 
-    if response.status_code != 200:
+    if response.status_code // 100 != 2:
         try:
             print(response.json())
         except json.JSONDecodeError:
@@ -37,6 +38,12 @@ if __name__ == "__main__":
 
         response.raise_for_status()
 
+    # The workflow is still pending, retry in a bit
+    while response.status_code == 202:
+        print(f"{response.json()['error']}. Retrying in 10 seconds.")
+        time.sleep(10)
+        response = httpx.get(download_url, follow_redirects=True)
+
     url = response.json()["url"]
     print(f"Downloading build from {url}")
     zipped_content = httpx.get(url, follow_redirects=True)