Apply linting, add missing type hints

.github/workflows/docker-compose.yml → .github/workflows/docker_compose.yml

@@ -21,13 +21,4 @@ jobs:
 
       - name: Execute tests in the running services
         run: |
-           docker compose run sync
-
-      - name: Pytest coverage comment
-        uses: MishaKav/pytest-coverage-comment@main
-        with:
-          pytest-xml-coverage-path: tests_output/coverage.xml
-          title: Integration tests Coverage
-          badge-title: Integration tests Coverage
-          junitxml-path: tests_output/pytest.xml
-          junitxml-title: JUnit Xml Summary
+           docker compose run sync

.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

.github/workflows/test.yml

@@ -12,18 +12,10 @@ jobs:
           cache: 'pip'
       - name: Install dependencies
         run: |
-          python -m pip install --upgrade pip pytest pytest-mock pytest-cov
+          python -m pip install --upgrade pip pytest pytest-mock
           pip install -e .
           ./install_git-cinnabar.sh
           echo "${{ github.workspace }}" >> $GITHUB_PATH
       - name: Test with pytest
         run: |
-          pytest --junitxml=pytest.xml --cov-report "xml:coverage.xml" --cov=git_hg_sync tests/
-      - name: Pytest coverage comment
-        uses: MishaKav/pytest-coverage-comment@main
-        with:
-          pytest-xml-coverage-path: coverage.xml
-          title: Unit tests Coverage
-          badge-title: Unit tests Coverage
-          junitxml-path: pytest.xml
-          junitxml-title: JUnit Xml Summary
+          pytest --junitxml=pytest.xml 

.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 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

docker-compose.yaml

@@ -1,7 +1,7 @@
 services:
   sync:
     build: .
-    command: ['pytest', '--junitxml=./tests_output/pytest.xml', '--cov-report', 'term', '--cov-report', 'xml:./tests_output/coverage.xml', '--cov=git_hg_sync', 'tests/']
+    command: ['pytest', '--junitxml=./tests_output/pytest.xml', 'tests/']
     volumes:
       - ./tests_output:/app/tests_output
     environment:

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(
                 [