Run git update in the background

Instead of doing so while handling the webhook, as that causes GitHub to
timeout on the big matplotlib.github.com repo waiting for us.

Author: QuLogic

test_webhook.py

@@ -35,14 +35,6 @@ async def test_signature(monkeypatch):
         'unused')
 
 
-async def test_update_repo_error():
-    """Test that updating a repository fails as expected."""
-    with pytest.raises(web.HTTPServerError,
-                       match='non-existent does not exist'):
-        await update_repo(Path('non-existent'), 'unused',
-                          'matplotlib/non-existent')
-
-
 async def test_update_repo_empty(tmp_path_factory):
     """Test that updating an empty repository works as expected."""
     repo1 = tmp_path_factory.mktemp('repo1')
@@ -137,15 +129,25 @@ async def test_github_webhook_errors(aiohttp_client, monkeypatch):
     assert resp.status == 400
     assert 'incorrect repository' in await resp.text()
 
+    # Problem on our side.
+    resp = await client.post(
+        '/gh/non-existent',
+        headers={**valid_headers, 'X-GitHub-Event': 'push'},
+        data='{"sender": {"login": "QuLogic"}, "ref": "refs/heads/gh-pages", '
+             '"repository": {"name": "non-existent", '
+             '"owner": {"login": "matplotlib"}}}')
+    assert resp.status == 500
+    assert 'non-existent does not exist' in await resp.text()
+
 
-async def test_github_webhook_valid(aiohttp_client, monkeypatch):
+async def test_github_webhook_valid(aiohttp_client, monkeypatch, tmp_path):
     """Test valid input to webhook."""
     client = await aiohttp_client(create_app())
 
     # Do no actual work, since that's tested above.
     monkeypatch.setattr(webhook, 'verify_signature',
                         mock.Mock(verify_signature, return_value=True))
-    monkeypatch.setenv('SITE_DIR', 'non-existent-site-dir')
+    monkeypatch.setenv('SITE_DIR', str(tmp_path))
     ur_mock = mock.Mock(update_repo, return_value=None)
     monkeypatch.setattr(webhook, 'update_repo', ur_mock)
 
@@ -177,6 +179,8 @@ async def test_github_webhook_valid(aiohttp_client, monkeypatch):
     ur_mock.assert_not_called()
 
     # Push event to gh-pages branch should run an update.
+    tmp_repo = tmp_path / 'non-existent-repo'
+    (tmp_repo / '.git').mkdir(parents=True, exist_ok=True)
     resp = await client.post(
         '/gh/non-existent-repo',
         headers={**valid_headers, 'X-GitHub-Event': 'push'},
@@ -186,5 +190,4 @@ async def test_github_webhook_valid(aiohttp_client, monkeypatch):
              ' "owner": {"login": "matplotlib"}}}')
     assert resp.status == 200
     ur_mock.assert_called_once_with(
-        Path('non-existent-site-dir/non-existent-repo'), 'foo',
-        'matplotlib/non-existent-repo')
+        tmp_repo, 'foo', 'matplotlib/non-existent-repo')

webhook.py

@@ -31,18 +31,22 @@
 
 async def update_repo(repo: Path, delivery: str, name: str):
     """Update a git repo at the given path."""
-    if not (repo / '.git').is_dir():
-        raise web.HTTPServerError(
-            reason=f'{delivery}: Checkout for {name} does not exist')
-
     log.info('%s: Running git pull for %s at %s', delivery, name, repo)
     proc = await asyncio.create_subprocess_exec('git', 'pull', cwd=repo)
+    await proc.wait()
+    if proc.returncode != 0:
+        log.error('%s: Running git pull for %s at %s failed: %d',
+                  delivery, name, repo, proc.returncode)
+
+
+def handle_update_repo_result(task: asyncio.Task):
+    """Clean up the git repo update tasks, logging an error if necessary."""
     try:
-        await asyncio.wait_for(proc.wait(), timeout=60)
-    except asyncio.TimeoutError:
-        proc.kill()
-        raise web.HTTPServerError(
-            reason=f'{delivery}: Timed out updating git checkout of {name}')
+        task.result()
+    except asyncio.CancelledError:
+        pass
+    except Exception:
+        log.exception('Exception raised by task %r', task)
 
 
 async def verify_signature(data: bytes, signature: bytes, repo: str,
@@ -137,7 +141,14 @@ async def github_webhook(request: web.Request):
         return web.Response(status=200)
 
     checkout = Path(os.environ.get('SITE_DIR', 'sites'), repository)
-    await update_repo(checkout, delivery, f'{organization}/{repository}')
+    if not (checkout / '.git').is_dir():
+        raise web.HTTPInternalServerError(
+            reason=(f'{delivery}: Checkout for {organization}/{repository} '
+                    'does not exist'))
+    task = asyncio.create_task(
+        update_repo(checkout, delivery, f'{organization}/{repository}'),
+        name=f'update_repo {repository}')
+    task.add_done_callback(handle_update_repo_result)
 
     return web.Response(status=200)
 
@@ -166,4 +177,11 @@ def create_app():
         host = 'localhost'
         port = 8080
     app = create_app()
-    web.run_app(app, host=host, port=port)
+    try:
+        web.run_app(app, host=host, port=port)
+    except KeyboardInterrupt:
+        # Finish up git update tasks.
+        git_update_tasks = {task for task in asyncio.all_tasks()
+                            if task.get_name().startswith('update_repo')}
+        asyncio.run_until_complete(
+            asyncio.gather(*git_update_tasks, return_exceptions=True))