Type hints and checking in CI (#173)

* Type hints and checking in CI

* Only install test deps when required

* Mark newly frozen mods as frozen

* Enable mypy linter in VS Code

Co-authored-by: Leon Wright <[email protected]>

.gitignore

@@ -4,3 +4,4 @@
 *.egg-info
 /pr_tester
 __pycache__
+.mypy_cache/

.vscode/settings.json

@@ -8,6 +8,7 @@
     "--rcfile",
     "/home/netkan/workspace/.pylintrc"
   ],
+  "python.linting.mypyEnabled": true,
   "python.linting.pylintUseMinimalCheckers": false,
   "files.exclude": {
     "**/__pycache__": true

netkan/Dockerfile

@@ -2,9 +2,11 @@ FROM python:3.7 as base
 RUN useradd -ms /bin/bash netkan
 ADD . /netkan
 WORKDIR /netkan
+RUN pip install mypy
 RUN chown -R netkan:netkan /netkan
 USER netkan
 RUN pip install --user . --no-warn-script-location
+RUN mypy .
 RUN python -m unittest -v
 RUN /home/netkan/.local/bin/netkan --help
 
@@ -22,9 +24,11 @@ CMD ["--help"]
 FROM production as dev
 USER root
 ADD . /netkan
+RUN chown -R netkan:netkan /netkan
 ADD run_dev.sh /usr/local/bin/
 USER netkan
 RUN pip install --user /netkan/.[development]
+RUN pip install --user /netkan/.[test]
 ENTRYPOINT ["/usr/local/bin/run_dev.sh"]
 
 FROM production

netkan/mypy.ini

@@ -0,0 +1,16 @@
+[mypy]
+ignore_missing_imports = true
+warn_redundant_casts = true
+show_error_context = true
+show_column_numbers = true
+show_error_codes = true
+pretty = true
+
+# Don't increase strictness for tests
+[mypy-netkan.*]
+check_untyped_defs = true
+disallow_incomplete_defs = true
+disallow_untyped_defs = true
+disallow_any_generics = true
+warn_unreachable = true
+strict_equality = true

netkan/netkan/auto_freezer.py

@@ -2,20 +2,22 @@
 from datetime import datetime, timezone, timedelta
 from pathlib import Path
 import git
+from typing import Iterable
 
 from .status import ModStatus
 from .repos import NetkanRepo
+from .github_pr import GitHubPR
 
 
 class AutoFreezer:
 
     BRANCH_NAME = 'freeze/auto'
 
-    def __init__(self, nk_repo, github_pr):
+    def __init__(self, nk_repo: NetkanRepo, github_pr: GitHubPR = None) -> None:
         self.nk_repo = nk_repo
         self.github_pr = github_pr
 
-    def freeze_idle_mods(self, days_limit):
+    def freeze_idle_mods(self, days_limit: int) -> None:
         update_cutoff = datetime.now(timezone.utc) - timedelta(days=days_limit)
         self.nk_repo.git_repo.remotes.origin.pull('master', strategy_option='ours')
         ids_to_freeze = [ident for ident in self._ids() if self._too_old(ident, update_cutoff)]
@@ -30,7 +32,7 @@ def freeze_idle_mods(self, days_limit):
             self._submit_pr(self.BRANCH_NAME, days_limit)
             self.nk_repo.git_repo.heads.master.checkout()
 
-    def mark_frozen_mods(self):
+    def mark_frozen_mods(self) -> None:
         with ModStatus.batch_write() as batch:
             logging.info('Marking frozen mods...')
             for mod in ModStatus.scan(rate_limit=5):
@@ -40,10 +42,10 @@ def mark_frozen_mods(self):
                     batch.save(mod)
             logging.info('Done!')
 
-    def _is_frozen(self, ident):
+    def _is_frozen(self, ident: str) -> bool:
         return not self.nk_repo.nk_path(ident).exists()
 
-    def _checkout_branch(self, name):
+    def _checkout_branch(self, name: str) -> None:
         try:
             self.nk_repo.git_repo.remotes.origin.fetch(name)
         except git.GitCommandError:
@@ -52,10 +54,10 @@ def _checkout_branch(self, name):
         (getattr(self.nk_repo.git_repo.heads, name, None)
          or self.nk_repo.git_repo.create_head(name)).checkout()
 
-    def _ids(self):
+    def _ids(self) -> Iterable[str]:
         return (nk.identifier for nk in self.nk_repo.netkans())
 
-    def _too_old(self, ident, update_cutoff):
+    def _too_old(self, ident: str, update_cutoff: datetime) -> bool:
         status = ModStatus.get(ident)
         last_indexed = getattr(status, 'last_indexed', None)
         if not last_indexed:
@@ -65,19 +67,20 @@ def _too_old(self, ident, update_cutoff):
         else:
             return last_indexed < update_cutoff
 
-    def _add_freezee(self, ident):
+    def _add_freezee(self, ident: str) -> None:
         self.nk_repo.git_repo.index.move([
             self.nk_repo.nk_path(ident).as_posix(),
             self.nk_repo.frozen_path(ident).as_posix()
         ])
         self.nk_repo.git_repo.index.commit(f'Freeze {ident}')
 
-    def _submit_pr(self, branch_name, days):
-        logging.info('Submitting pull request for %s', branch_name)
-        self.nk_repo.git_repo.remotes.origin.push(f'{branch_name}:{branch_name}')
-        self.github_pr.create_pull_request(
-            branch=branch_name,
-            title='Freeze idle mods',
-            body=(f'The attached mods have not updated in {days} or more days.'
-                  ' Freeze them to save the bot some CPU cycles.'),
-        )
+    def _submit_pr(self, branch_name: str, days: int) -> None:
+        if self.github_pr:
+            logging.info('Submitting pull request for %s', branch_name)
+            self.nk_repo.git_repo.remotes.origin.push(f'{branch_name}:{branch_name}')
+            self.github_pr.create_pull_request(
+                branch=branch_name,
+                title='Freeze idle mods',
+                body=(f'The attached mods have not updated in {days} or more days.'
+                      ' Freeze them to save the bot some CPU cycles.'),
+            )

netkan/netkan/cli/__init__.py

@@ -21,7 +21,7 @@
 
 
 @click.group()
-def netkan():
+def netkan() -> None:
     pass
 
 

netkan/netkan/cli/common.py

@@ -1,6 +1,9 @@
 import sys
 import logging
 from pathlib import Path
+from git import Repo
+from typing import Union, Callable, Any, Optional, Dict
+
 import click
 
 from ..repos import NetkanRepo, CkanMetaRepo
@@ -9,7 +12,7 @@
 from ..github_pr import GitHubPR
 
 
-def ctx_callback(ctx, param, value):
+def ctx_callback(ctx: click.Context, param: click.Parameter, value: Union[str, int]) -> Union[str, int]:
     shared = ctx.ensure_object(SharedArgs)
     setattr(shared, param.name, value)
     return value
@@ -52,27 +55,27 @@ def ctx_callback(ctx, param, value):
 
 class SharedArgs(object):
 
-    def __init__(self):
+    def __init__(self) -> None:
         self._environment_data = None
-        self._debug = None
-        self._ssh_key = None
-        self._ckanmeta_repo = None
-        self._netkan_repo = None
-        self._github_pr = None
+        self._debug: Optional[bool] = None
+        self._ssh_key: Optional[str] = None
+        self._ckanmeta_repo: Optional[CkanMetaRepo] = None
+        self._netkan_repo: Optional[NetkanRepo] = None
+        self._github_pr: Optional[GitHubPR] = None
 
-    def __getattribute__(self, name):
+    def __getattribute__(self, name: str) -> Any:
         attr = super().__getattribute__(name)
         if not name.startswith('_') and attr is None:
             logging.fatal("Expecting attribute '%s' to be set; exiting disgracefully!", name)
             sys.exit(1)
         return attr
 
     @property
-    def debug(self):
+    def debug(self) -> Optional[bool]:
         return self._debug
 
     @debug.setter
-    def debug(self, value):
+    def debug(self, value: bool) -> None:
         # When there isn't a flag passed we get a None instead, setting
         # it as a 'False' for consistency.
         self._debug = value or False
@@ -82,46 +85,46 @@ def debug(self, value):
             sys.excepthook = catch_all
 
     @property
-    def ssh_key(self):
+    def ssh_key(self) -> Optional[str]:
         return self._ssh_key
 
     @ssh_key.setter
-    def ssh_key(self, value):
+    def ssh_key(self, value: str) -> None:
         init_ssh(value, Path(Path.home(), '.ssh'))
         self._ssh_key = value
 
     @property
-    def ckanmeta_repo(self):
+    def ckanmeta_repo(self) -> CkanMetaRepo:
         if not self._ckanmeta_repo:
             self._ckanmeta_repo = CkanMetaRepo(
                 init_repo(self._ckanmeta_remote, '/tmp/CKAN-meta', self.deep_clone))
         return self._ckanmeta_repo
 
     @property
-    def ckanmeta_remote(self):
+    def ckanmeta_remote(self) -> str:
         return self._ckanmeta_remote
 
     @ckanmeta_remote.setter
-    def ckanmeta_remote(self, value):
+    def ckanmeta_remote(self, value: str) -> None:
         self._ckanmeta_remote = value
 
     @property
-    def netkan_repo(self):
+    def netkan_repo(self) -> NetkanRepo:
         if not self._netkan_repo:
             self._netkan_repo = NetkanRepo(
                 init_repo(self._netkan_remote, '/tmp/NetKAN', self.deep_clone))
         return self._netkan_repo
 
     @property
-    def netkan_remote(self):
+    def netkan_remote(self) -> str:
         return self._netkan_remote
 
     @netkan_remote.setter
-    def netkan_remote(self, value):
+    def netkan_remote(self, value: str) -> None:
         self._netkan_remote = value
 
     @property
-    def github_pr(self):
+    def github_pr(self) -> GitHubPR:
         if not self._github_pr:
             self._github_pr = GitHubPR(self.token, self.repo, self.user)
         return self._github_pr
@@ -130,7 +133,7 @@ def github_pr(self):
 pass_state = click.make_pass_decorator(SharedArgs, ensure=True)  # pylint: disable=invalid-name
 
 
-def common_options(func):
+def common_options(func: Callable[..., Any]) -> Callable[..., Any]:
     for option in reversed(_COMMON_OPTIONS):
         func = option(func)
     return func

netkan/netkan/cli/services.py

@@ -3,7 +3,7 @@
 import boto3
 import click
 
-from .common import common_options, pass_state
+from .common import common_options, pass_state, SharedArgs
 
 from ..indexer import MessageHandler
 from ..scheduler import NetkanScheduler
@@ -14,7 +14,7 @@
 @click.command()
 @common_options
 @pass_state
-def indexer(common):
+def indexer(common: SharedArgs) -> None:
     sqs = boto3.resource('sqs')
     queue = sqs.get_queue_by_name(QueueName=common.queue)
 
@@ -55,7 +55,7 @@ def indexer(common):
 )
 @common_options
 @pass_state
-def scheduler(common, max_queued, group, min_cpu, min_io):
+def scheduler(common: SharedArgs, max_queued: int, group: str, min_cpu: int, min_io: int) -> None:
     sched = NetkanScheduler(
         common.netkan_repo, common.ckanmeta_repo, common.queue,
         nonhooks_group=(group == 'all' or group == 'nonhooks'),
@@ -69,17 +69,17 @@ def scheduler(common, max_queued, group, min_cpu, min_io):
 @click.command()
 @common_options
 @pass_state
-def mirrorer(common):
+def mirrorer(common: SharedArgs) -> None:
     Mirrorer(
         common.ckanmeta_repo, common.ia_access, common.ia_secret,
-        common.ia_collection
+        common.ia_collection, common.token
     ).process_queue(common.queue, common.timeout)
 
 
 @click.command()
 @common_options
 @pass_state
-def spacedock_adder(common):
+def spacedock_adder(common: SharedArgs) -> None:
     sd_adder = SpaceDockAdder(
         common.queue,
         common.timeout,