Apply linting, add missing type hints

- add missing type hints
- remove mypy: checks weren't running
- configure ruff checks
- remove black; use ruff instead
- add 'format' tox command
- optimise Dockerfile to avoid rebuilding the venv on any source change

.github/workflows/lint.yml

@@ -11,5 +11,5 @@ jobs:
           python-version: 3.11
       - name: Install tox
         run: pip install tox
-      - name: Lint with ruff, black and mypy
+      - name: Lint
         run: tox -e lint

.gitignore

@@ -1,6 +1,6 @@
-clones
-.tox
 **/__pycache__/
+/.coverage
+/.tox
+/clones
 /config.toml
-.coverage
-tests_output
+/tests_output

Dockerfile

@@ -1,27 +1,32 @@
 FROM python:3.12-slim
 
-WORKDIR /app
-
 RUN groupadd --gid 10001 app \
-  && useradd -m -g app --uid 10001 -s /usr/sbin/nologin app
+  && useradd -m -g app --uid 10001 -d /app -s /usr/sbin/nologin app
 
 RUN apt-get update && \
     apt-get install --yes git mercurial curl vim && \
     apt-get -q --yes autoremove && \
     apt-get clean && \
     rm -rf /root/.cache
 
+WORKDIR /app
+
 # git-cinnabar
 COPY install_git-cinnabar.sh .
 RUN ./install_git-cinnabar.sh
 RUN mv git-cinnabar git-remote-hg /usr/bin/
 
 # install test dependencies
-RUN pip install -U pip pytest pytest-mock pytest-cov
+RUN pip install -U pip pytest pytest-mock pytest-cov pip-tools
 
-# Copy local code to the container image.
-COPY . /app
-RUN chown -R app: /app
+# setup just the venv so changes to the source won't require a full venv
+# rebuild
+COPY --chown=app:app README.md .
+COPY --chown=app:app pyproject.toml .
+RUN pip-compile --verbose pyproject.toml
+RUN pip install -r requirements.txt
 
+# copy app and install
+COPY --chown=app:app . /app
+RUN pip install /app
 USER app
-RUN pip install -e .

README.md

@@ -20,11 +20,17 @@ process the next message in the queue.
 
 ## build and test
 
+Format and test/lint code:
+
+```console
+$ tox -e format,lint
+```
+
+Run tests:
+
 ```console
-$ mkdir -p tests_output
-$ chmod a+w tests_output
-$ docker-compose build
-$ docker-compose run --rm sync
+$ docker compose run --build sync
+$ docker compose down
 ```
 
 ## Known limitations

git_hg_sync/__main__.py

@@ -1,6 +1,5 @@
 import argparse
 import sys
-import logging
 from pathlib import Path
 
 import sentry_sdk
@@ -13,7 +12,7 @@
 from git_hg_sync.repo_synchronizer import RepoSynchronizer
 
 
-def get_parser():
+def get_parser() -> argparse.ArgumentParser:
     parser = argparse.ArgumentParser()
     parser.add_argument(
         "-c",
@@ -25,7 +24,7 @@ def get_parser():
     return parser
 
 
-def get_connection(config: PulseConfig):
+def get_connection(config: PulseConfig) -> Connection:
     return Connection(
         hostname=config.host,
         port=config.port,
@@ -36,7 +35,7 @@ def get_connection(config: PulseConfig):
     )
 
 
-def get_queue(config):
+def get_queue(config: Config | PulseConfig) -> Queue:
     exchange = Exchange(config.exchange, type="topic")
     return Queue(
         name=config.queue,
@@ -47,7 +46,7 @@ def get_queue(config):
 
 
 def start_app(
-    config: Config, logger: logging.Logger, *, one_shot: bool = False
+    config: Config, logger: commandline.StructuredLogger, *, one_shot: bool = False
 ) -> None:
     pulse_config = config.pulse
     connection = get_connection(pulse_config)

git_hg_sync/application.py

@@ -1,7 +1,7 @@
 import signal
 import sys
+from collections.abc import Sequence
 from types import FrameType
-from typing import Optional, Sequence
 
 from mozlog import get_proxy_logger
 
@@ -14,20 +14,19 @@
 
 
 class Application:
-
     def __init__(
         self,
         worker: PulseWorker,
         repo_synchronizers: dict[str, RepoSynchronizer],
         mappings: Sequence[Mapping],
-    ):
+    ) -> None:
         self._worker = worker
         self._worker.event_handler = self._handle_event
         self._repo_synchronizers = repo_synchronizers
         self._mappings = mappings
 
     def run(self) -> None:
-        def signal_handler(sig: int, frame: Optional[FrameType]) -> None:
+        def signal_handler(_sig: int, _frame: FrameType | None) -> None:
             if self._worker.should_stop:
                 logger.info("Process killed by user")
                 sys.exit(1)

git_hg_sync/config.py

@@ -1,8 +1,8 @@
 import pathlib
-import tomllib
 from typing import Self
 
 import pydantic
+import tomllib
 
 from git_hg_sync.mapping import BranchMapping, TagMapping
 
@@ -43,7 +43,6 @@ class Config(pydantic.BaseModel):
     def verify_all_mappings_reference_tracked_repositories(
         self,
     ) -> Self:
-
         tracked_urls = [tracked_repo.url for tracked_repo in self.tracked_repositories]
         for mapping in self.branch_mappings:
             if mapping.source_url not in tracked_urls:
@@ -55,6 +54,6 @@ def verify_all_mappings_reference_tracked_repositories(
     @staticmethod
     def from_file(file_path: pathlib.Path) -> "Config":
         assert file_path.exists(), f"config file {file_path} doesn't exists"
-        with open(file_path, "rb") as config_file:
+        with file_path.open("rb") as config_file:
             config = tomllib.load(config_file)
         return Config(**config)

git_hg_sync/mapping.py

@@ -1,7 +1,8 @@
 import re
+from collections.abc import Sequence
 from dataclasses import dataclass
 from functools import cached_property
-from typing import Sequence, TypeAlias
+from typing import TypeAlias
 
 import pydantic
 

git_hg_sync/pulse_worker.py

@@ -1,15 +1,16 @@
-from typing import Protocol
+from typing import Any, Protocol
 
+import kombu
 from kombu.mixins import ConsumerMixin
 from mozlog import get_proxy_logger
 
 from git_hg_sync.events import Push, Tag
 
-logger = get_proxy_logger("pluse_consumer")
+logger = get_proxy_logger("pulse_consumer")
 
 
 class EventHandler(Protocol):
-    def __call__(self, event: Push | Tag):
+    def __call__(self, event: Push | Tag) -> None:
         pass
 
 
@@ -21,13 +22,19 @@ class PulseWorker(ConsumerMixin):
     event_handler: EventHandler | None
     """Function that will be called whenever an event is received"""
 
-    def __init__(self, connection, queue, *, one_shot=False):
+    def __init__(
+        self,
+        connection: kombu.Connection,
+        queue: kombu.Queue,
+        *,
+        one_shot: bool = False,
+    ) -> None:
         self.connection = connection
         self.task_queue = queue
         self.one_shot = one_shot
 
     @staticmethod
-    def parse_entity(raw_entity):
+    def parse_entity(raw_entity: Any) -> Push | Tag:
         logger.debug(f"parse_entity: {raw_entity}")
         message_type = raw_entity.pop("type")
         match message_type:
@@ -38,13 +45,17 @@ def parse_entity(raw_entity):
             case _:
                 raise EntityTypeError(f"unsupported type {message_type}")
 
-    def get_consumers(self, Consumer, channel):
-        consumer = Consumer(
+    def get_consumers(
+        self,
+        consumer_class: type[kombu.Consumer],
+        _channel: Any,
+    ) -> list[kombu.Consumer]:
+        consumer = consumer_class(
             self.task_queue, auto_declare=False, callbacks=[self.on_task]
         )
         return [consumer]
 
-    def on_task(self, body, message):
+    def on_task(self, body: Any, message: kombu.Message) -> None:
         logger.info(f"Received message: {body}")
         raw_entity = body["payload"]
         event = PulseWorker.parse_entity(raw_entity)

git_hg_sync/repo_synchronizer.py

@@ -1,10 +1,10 @@
 from pathlib import Path
 
-
 from git import Repo, exc
-from git_hg_sync.mapping import SyncOperation, SyncBranchOperation, SyncTagOperation
 from mozlog import get_proxy_logger
 
+from git_hg_sync.mapping import SyncBranchOperation, SyncOperation, SyncTagOperation
+
 logger = get_proxy_logger("sync_repo")
 
 
@@ -17,12 +17,11 @@ class MercurialMetadataNotFoundError(RepoSyncError):
 
 
 class RepoSynchronizer:
-
     def __init__(
         self,
         clone_directory: Path,
         url: str,
-    ):
+    ) -> None:
         self._clone_directory = clone_directory
         self._src_remote = url
 
@@ -42,7 +41,7 @@ def _get_clone_repo(self) -> Repo:
 
     def _commit_has_mercurial_metadata(self, repo: Repo, git_commit: str) -> bool:
         stdout = repo.git.cinnabar(["git2hg", git_commit])
-        return not all([char == "0" for char in stdout.strip()])
+        return not all(char == "0" for char in stdout.strip())
 
     def _fetch_all_from_remote(self, repo: Repo, remote: str) -> None:
         try:
@@ -89,7 +88,7 @@ def sync(self, destination_url: str, operations: list[SyncOperation]) -> None:
         ]
 
         # Create tag branches locally
-        tag_branches = set([op.tags_destination_branch for op in tag_ops])
+        tag_branches = {op.tags_destination_branch for op in tag_ops}
         for tag_branch in tag_branches:
             repo.git.fetch(
                 [

pyproject.toml

@@ -7,15 +7,61 @@ name = "git-hg-sync"
 readme = "README.md"
 requires-python = ">=3.10"
 version = "0.1"
-dependencies = ['kombu', 'mozillapulse', 'GitPython', 'mozlog', "pydantic", "sentry_sdk"]
+dependencies = [
+    "GitPython",
+    "kombu",
+    "mozillapulse",
+    "mozlog",
+    "pydantic",
+    "pytest>=8.3.4",
+    "sentry_sdk",
+]
 
 [tool.ruff]
-line-length = 100
-
-[[tool.mypy.overrides]]
-module = [
-  'kombu.*',
-  'mozlog'
+lint.select = [
+    "A", # flake8-builtins
+    "ANN", # flake8-annotations
+    "ARG", # flake8-unused-arguments
+    "B", # flake8-bugbear
+    "BLE", # flake8-blind-except
+    "C4", # flake8-comprehensions
+    "E", # pycodestyle-error
+    "ERA", # eradicate
+    "EXE", # flake8-executable
+    "F", # Pyflakes
+    "FIX", # flake8-fixme
+    "G", # flake8-logging-format
+    "I", # isort
+    "ISC", # flake8-implicit-str-concat
+    "N", # pep8-naming
+    "PERF", # perflint
+    "PIE", # flake8-pie
+    "PL", # Pylint
+    "PTH", # flake8-use-pathlib
+    "RET", # flake8-return
+    "RUF", # Ruff-specific rules
+    "TCH", # flake8-type-checking
+    "SIM", # flake8-simplify
+    "SLF", # flake8-self
+    "UP", # pyupgrade
+    "W", # pycodestyle-warning
+    "YTT", # flake8-2020
+]
+lint.ignore = [
+    "A003", # builtin-attribute-shadowing
+    "ANN401", # any-type
+    "B009", # get-attr-with-constant
+    "B904", # raise-without-from-inside-except
+    "E501", # line-too-long
+    "ERA001", # commented-out-code
+    "G004", # logging-f-string
+    "ISC001", # single-line-implicit-string-concatenation (formatter will fix)
+    "PERF203", # try-except-in-loop
+    "PERF401", # manual-list-comprehension
+    "PLR", # pylint-refactor
+    "PLW2901", # redefined-loop-name
+    "RUF005", # collection-literal-concatenation
+    "RUF012", # mutable-class-default
+    "SIM105", # use-contextlib-suppress
+    "W191", # tab-indentation (formatter will fix)
 ]
-
-ignore_missing_imports = true