Address first review comments

Author: Steffen Ohrendorf

.github/workflows/ci.yml

@@ -12,6 +12,33 @@ jobs:
     steps:
       - uses: actions/checkout@v1
 
+      - name: Set up Python
+        uses: actions/setup-python@v1
+        with:
+          python-version: 3.8
+
+      - name: Install Poetry
+        run: pip install poetry==$POETRY_VERSION
+        env:
+          POETRY_VERSION: 1.0.2
+
+      - name: Cache Poetry virtualenv
+        uses: actions/cache@v1
+        id: poetry-cache
+        with:
+          path: ~/.cache/pypoetry/virtualenvs
+          key: poetry-${{ hashFiles('**/poetry.lock') }}
+          restore-keys: |
+            poetry-${{ hashFiles('**/poetry.lock') }}
+      - name: Install Dependencies
+        if: steps.poetry-cache.outputs.cache-hit != 'true'
+        run: |
+          cd docker && poetry install
+
+      - name: Lint
+        run: |
+          cd docker && poetry run flake8
+
       - name: Build and publish image
         run: |
           # For branch names other than master (e.g. feature/test), append last branch name component (test) to the tag

docker/ghd.py

@@ -1,12 +1,12 @@
 #!/usr/bin/env python3
 import asyncio
-import click
 import os
-import colorama
-
 from functools import wraps
-from github import DeploymentState, GitHub, read_github_event_data, get_current_deployment_id, get_current_environment
-from util import get_repo_or_fallback
+
+import click
+import colorama
+from github import DeploymentState, GitHub, get_current_deployment_id, get_current_environment, read_github_event_data
+from util import get_repo_fallback
 
 
 def coroutine(f):
@@ -17,69 +17,120 @@ def wrapper(*args, **kwargs):
     return wrapper
 
 
+def click_repo_option():
+    repo_path = get_repo_fallback(read_github_event_data())
+    return click.option("-r", "--repo",
+                        required=repo_path is None,
+                        default=repo_path,
+                        help="Repository to use, e.g. moneymeets/ghd")
+
+
+def click_deployment_id_option():
+    deployment_id = get_current_deployment_id()
+    return click.option("-d", "--deployment-id",
+                        type=int,
+                        required=deployment_id is None,
+                        default=deployment_id,
+                        help="Deployment ID")
+
+
 @click.group()
 def main_group():
     pass
 
 
 @main_group.command(name="list", short_help="List deployments")
-@click.option("-r", "--repo", required=False, help="Repository to use, e.g. moneymeets/ghd")
-@click.option("-v", "--verbose", required=False, is_flag=True, flag_value=True, default=False,
+@click_repo_option()
+@click.option("-v", "--verbose",
+              required=False,
+              is_flag=True,
+              flag_value=True,
+              default=False,
               help="Print deployment states (slow)")
-@click.option("-l", "--limit", required=False, default=10, help="How many deployments to list")
+@click.option("-l", "--limit",
+              required=False,
+              default=10,
+              help="How many deployments to list")
 @coroutine
 async def cmd_list(repo: str, verbose: bool, limit: int):
-    github_event_data = read_github_event_data()
-    async with GitHub(repo_path=get_repo_or_fallback(repo, github_event_data)) as gh:
+    async with GitHub(repo_path=repo) as gh:
         await gh.list(limit=limit, verbose=verbose)
 
 
 @main_group.command(name="deploy", short_help="Create new deployment")
-@click.option("-r", "--repo", required=False, help="Repository to use, e.g. moneymeets/ghd")
-@click.option("-R", "--ref", required=True, help="Reference to create the deployment from")
-@click.option("-e", "--environment", required=True, prompt=True, help="Environment name")
-@click.option("-T", "--task", default="deploy", help="Deployment task")
-@click.option("-t", "--transient", required=False, is_flag=True, flag_value=True, prompt=True, default=False,
+@click_repo_option()
+@click.option("-R", "--ref",
+              required=True,
+              help="Reference to create the deployment from")
+@click.option("-e", "--environment",
+              required=True,
+              prompt=True,
+              help="Environment name")
+@click.option("-T", "--task",
+              default="deploy",
+              help="Deployment task")
+@click.option("-t", "--transient",
+              required=False,
+              is_flag=True,
+              flag_value=True,
+              prompt=True,
+              default=False,
               help="Mark as transient environment")
-@click.option("-p", "--production", required=False, is_flag=True, flag_value=True, prompt=True, default=False,
+@click.option("-p", "--production",
+              required=False,
+              is_flag=True,
+              flag_value=True,
+              prompt=True,
+              default=False,
               help="Mark as production environment")
-@click.option("-d", "--description", default="Deployed via GHD", prompt=True, help="Deployment description")
+@click.option("-d", "--description",
+              default="Deployed via GHD",
+              prompt=True,
+              help="Deployment description")
 @coroutine
 async def cmd_deploy(repo: str, ref: str, environment: str, task: str, transient: bool, production: bool,
                      description: str):
-    github_event_data = read_github_event_data()
-    async with GitHub(repo_path=get_repo_or_fallback(repo, github_event_data)) as gh:
-        await gh.deploy(environment=environment, ref=ref or os.environ.get("GITHUB_SHA"),
+    async with GitHub(repo_path=repo) as gh:
+        await gh.deploy(environment=environment,
+                        ref=ref or os.environ.get("GITHUB_SHA"),
                         transient=transient,
                         production=production,
                         task=task,
                         description=description)
 
 
 @main_group.command(name="set-state", short_help="Set deployment state")
-@click.option("-r", "--repo", required=False, help="Repository to use, e.g. moneymeets/ghd")
-@click.option("-e", "--environment", required=True, default=get_current_environment(), help="Environment name")
-@click.option("-d", "--deployment-id", type=int, required=True, default=get_current_deployment_id(),
-              help="Deployment ID")
+@click_repo_option()
+@click.option("-e", "--environment",
+              required=True,
+              default=get_current_environment(),
+              help="Environment name")
+@click_deployment_id_option()
 @click.option("-s", "--state",
               type=click.Choice(choices=DeploymentState.__members__.keys()),
-              required=True, help="State")
-@click.option("-D", "--description", default="Deployed via GHD", help="Deployment description")
+              required=True,
+              help="State")
+@click.option("-D", "--description",
+              default="Deployed via GHD",
+              help="Deployment description")
 @coroutine
 async def cmd_set_state(repo: str, environment: str, deployment_id: int, state: str, description: str):
-    async with GitHub(repo_path=get_repo_or_fallback(repo, read_github_event_data())) as gh:
+    async with GitHub(repo_path=repo) as gh:
         await gh.create_deployment_status(deployment_id=deployment_id,
                                           state=DeploymentState[state],
                                           environment=environment,
                                           description=description)
 
 
 @main_group.command(name="inspect", short_help="Inspect deployment state history")
-@click.option("-r", "--repo", required=False, help="Repository to use, e.g. moneymeets/ghd")
-@click.argument("deployment-id", type=int, required=True, nargs=1)
+@click_repo_option()
+@click.argument("deployment-id",
+                type=int,
+                required=True,
+                nargs=1)
 @coroutine
 async def cmd_inspect(repo: str, deployment_id: int):
-    async with GitHub(repo_path=get_repo_or_fallback(repo, read_github_event_data())) as gh:
+    async with GitHub(repo_path=repo) as gh:
         await gh.inspect(deployment_id=deployment_id)
 
 
@@ -88,5 +139,5 @@ def run_main():
     main_group()
 
 
-if __name__ == '__main__':
+if __name__ == "__main__":
     run_main()

docker/github.py

@@ -1,15 +1,14 @@
+import enum
 import json
 import os
 import sys
-import enum
 
 import aiohttp
 import colorama
 import progressbar
 import tabulate
-
-from util import short_sha, bool_to_str, deep_dict_get
-from output import print_success, color_unknown, print_info, color_str
+from output import color_str, color_unknown, print_info, print_success
+from util import bool_to_str, deep_dict_get, short_sha
 
 
 class DeploymentState(enum.Enum):
@@ -22,6 +21,10 @@ class DeploymentState(enum.Enum):
 
 
 class GitHub:
+    @staticmethod
+    def _api_url(path: str) -> str:
+        return f"https://api.github.com{path}"
+
     def __init__(self, repo_path: str):
         assert repo_path
 
@@ -61,7 +64,7 @@ def __exit__(self,
         # __exit__ should exist in pair with __enter__ but never executed
         pass  # pragma: no cover
 
-    async def __aenter__(self) -> 'GitHub':
+    async def __aenter__(self) -> "GitHub":
         return self
 
     async def __aexit__(self,
@@ -72,47 +75,39 @@ async def __aexit__(self,
         await self.session_ant_man.close()
 
     async def get(self, path: str):
-        # print(f"GET {path}")
-        async with self.session_ant_man.get(f"https://api.github.com{path}") as response:
+        async with self.session_ant_man.get(self._api_url(path)) as response:
             result = await response.json()
-            # print(result)
             return result
 
     async def get_flash(self, path: str):
-        # print(f"GET {path}")
-        async with self.session_flash.get(f"https://api.github.com{path}") as response:
+        async with self.session_flash.get(self._api_url(path)) as response:
             result = await response.json()
-            # print(result)
             return result
 
     async def post(self, path: str, json):
-        # print(f"POST {path}")
-        async with self.session_ant_man.post(f"https://api.github.com{path}", json=json) as response:
+        async with self.session_ant_man.post(self._api_url(path), json=json) as response:
             result = await response.json()
-            # print(result)
             return result
 
     async def post_flash(self, path: str, json):
-        # print(f"POST {path}")
-        async with self.session_flash.post(f"https://api.github.com{path}", json=json) as response:
+        async with self.session_flash.post(self._api_url(path), json=json) as response:
             result = await response.json()
-            # print(result)
             return result
 
     async def get_deployments(self) -> list:
         try:
-            return sorted(await self.get(f'/repos/{self.repo_path}/deployments'), key=lambda e: e["id"], reverse=True)
+            return sorted(await self.get(f"/repos/{self.repo_path}/deployments"), key=lambda e: e["id"], reverse=True)
         except TypeError:
             return []
 
     async def get_deployment_statuses(self, deployment_id: int) -> list:
-        return sorted(await self.get(f'/repos/{self.repo_path}/deployments/{deployment_id}/statuses'),
+        return sorted(await self.get(f"/repos/{self.repo_path}/deployments/{deployment_id}/statuses"),
                       key=lambda e: e["id"],
                       reverse=True)
 
     async def create_deployment(self, ref: str, environment: str, transient: bool, production: bool, task: str,
                                 description: str):
-        return await self.post(f'/repos/{self.repo_path}/deployments', {
+        return await self.post(f"/repos/{self.repo_path}/deployments", {
             "ref": ref,
             "auto_merge": False,
             "environment": environment,
@@ -125,7 +120,7 @@ async def create_deployment(self, ref: str, environment: str, transient: bool, p
 
     async def create_deployment_status(self, deployment_id: int, state: DeploymentState, environment: str,
                                        description: str):
-        return await self.post_flash(f'/repos/{self.repo_path}/deployments/{deployment_id}/statuses', {
+        return await self.post_flash(f"/repos/{self.repo_path}/deployments/{deployment_id}/statuses", {
             "state": state.name,
             "description": description,
             "environment": environment,
@@ -204,18 +199,18 @@ async def inspect(self, deployment_id: int):
 
     async def deploy(self, environment: str, ref: str, transient: bool, production: bool, task: str, description: str):
         print_info("Creating deployment")
-        tmp = await self.create_deployment(ref=ref,
-                                           environment=environment,
-                                           transient=transient,
-                                           production=production,
-                                           task=task,
-                                           description=description)
-        if "id" not in tmp:
-            print(tmp)
+        deployment_creation_result = await self.create_deployment(ref=ref,
+                                                                  environment=environment,
+                                                                  transient=transient,
+                                                                  production=production,
+                                                                  task=task,
+                                                                  description=description)
+        if "id" not in deployment_creation_result:
+            print(deployment_creation_result)
             raise RuntimeError()
 
-        print(f"::set-output name=deployment_id::{tmp['id']}")
-        print_success(f"Deployment {tmp['id']} created")
+        print(f"::set-output name=deployment_id::{deployment_creation_result['id']}")
+        print_success(f"Deployment {deployment_creation_result['id']} created")
 
 
 _github_event_data = None
@@ -228,7 +223,8 @@ def read_github_event_data():
         return _github_event_data
 
     _github_event_data = dict()
-    if (github_event_path := os.environ.get("GITHUB_EVENT_PATH")) and os.path.exists(github_event_path):
+    github_event_path = os.environ.get("GITHUB_EVENT_PATH")  # TODO: Use walrus operator when flake8 supports it
+    if github_event_path and os.path.exists(github_event_path):
         print_info("Found GitHub Event Path")
         with open(github_event_path, "r") as f:
             _github_event_data = json.load(f)