po_mode: Implement suggestions from review

checkmk_weblate_syncer/config.py

@@ -1,3 +1,4 @@
+from collections.abc import Sequence
 from pathlib import Path
 from typing import Annotated
 
@@ -31,10 +32,10 @@ class PotModeConfig(BaseConfig, frozen=True):
     locale_pot_path: Annotated[Path, AfterValidator(_validate_path_is_relative)]
 
 
+class PoFilePair(BaseModel, frozen=True):
+    checkmk: Annotated[Path, AfterValidator(_validate_path_is_relative)]
+    locale: Annotated[Path, AfterValidator(_validate_path_is_relative)]
+
+
 class PoModeConfig(BaseConfig, frozen=True):
-    checkmk_locale_po_file_path_pairs: list[
-        tuple[
-            Annotated[Path, AfterValidator(_validate_path_is_relative)],
-            Annotated[Path, AfterValidator(_validate_path_is_relative)],
-        ]
-    ]
+    po_file_pairs: Sequence[PoFilePair]

checkmk_weblate_syncer/git.py

@@ -1,12 +1,48 @@
+from collections.abc import Sequence
 from pathlib import Path
+from subprocess import CalledProcessError
 
 from git import Repo
 
+from .config import RepositoryConfig
+from .logging import LOGGER
 
-def repository_in_clean_state(path: Path, branch: str) -> Repo:
+
+def repository_in_clean_state(
+    repo_config: RepositoryConfig,
+    repo_name: str,
+) -> Repo:
+    LOGGER.info("Cleaning up and updating %s repository", repo_name)
+    try:
+        return _repository_in_clean_state(
+            repo_config.path,
+            repo_config.branch,
+        )
+    except Exception as e:
+        LOGGER.error("Error while cleaning up and updating %s repository", repo_name)
+        LOGGER.exception(e)
+        raise e
+
+
+def _repository_in_clean_state(path: Path, branch: str) -> Repo:
     repo = Repo(path)
     repo.git.reset("--hard")
     repo.remotes.origin.fetch()
     repo.git.checkout(branch)
     repo.git.reset("--hard", f"origin/{branch}")
     return repo
+
+
+def commit_and_push_files(
+    repo: Repo,
+    files_to_push_to_repo: Sequence[Path],
+) -> None:
+    try:
+        repo.index.add(files_to_push_to_repo)
+        repo.index.commit("Updateing files")
+        repo.remotes.origin.push()
+    except CalledProcessError as e:
+        LOGGER.error("Committing and pushing files failed")
+        LOGGER.exception(e)
+        raise e
+    LOGGER.info("Committing and pushing files succeeded")

checkmk_weblate_syncer/po.py

@@ -7,9 +7,19 @@
 
 from git import Repo
 
-from .config import PoModeConfig
+from .config import PoFilePair, PoModeConfig, RepositoryConfig
+from .git import commit_and_push_files, repository_in_clean_state
 from .logging import LOGGER
-from .utils import get_repository_in_clean_state_with_logging
+
+
+@dataclass(frozen=True)
+class _Success:
+    path: Path
+
+
+@dataclass(frozen=True)
+class _Failure:
+    error_message: str
 
 
 @dataclass
@@ -19,69 +29,40 @@ class FileWithFormatErrors:
 
 
 def run(config: PoModeConfig) -> None:
-    checkmk_repo = get_repository_in_clean_state_with_logging(
+    checkmk_repo = repository_in_clean_state(
         config.checkmk_repository,
         "checkmk",
     )
-    get_repository_in_clean_state_with_logging(
+    repository_in_clean_state(
         config.locale_repository,
         "locale",
     )
 
     files_with_format_errors: list[FileWithFormatErrors] = []
     files_to_push_to_checkmk: list[Path] = []
 
-    for (
-        checkmk_po_file_path,
-        locale_po_file_path,
-    ) in config.checkmk_locale_po_file_path_pairs:
-        LOGGER.info("Checking formatting errors in %s", locale_po_file_path)
-        try:
-            run_subprocess(
-                ["msgfmt", "--check-format", locale_po_file_path],
-                check=True,
-                capture_output=True,
-                encoding="UTF-8",
-            )
-        except CalledProcessError as e:
+    for file_pair in config.po_file_pairs:
+        result = _process_po_file_pair(
+            file_pair=file_pair,
+            checkmk_repo=config.checkmk_repository,
+            locale_repo=config.locale_repository,
+        )
+        if isinstance(result, _Failure):
             files_with_format_errors.append(
-                FileWithFormatErrors(locale_po_file_path, e.stderr)
+                FileWithFormatErrors(
+                    file_path=file_pair.locale,
+                    error_message=result.error_message,
+                )
             )
-            LOGGER.error(
-                "Formatting errors found in %s.\n\nStderr:\n%s",
-                locale_po_file_path,
-                e.stderr,
-            )
-            LOGGER.exception(e)
             continue
-        except Exception as e:
-            LOGGER.error("Checking formatting errors failed")
-            LOGGER.exception(e)
-            raise e
-
-        locale_po_file = config.locale_repository.path / locale_po_file_path
-        checkmk_po_file = config.checkmk_repository.path / checkmk_po_file_path
-
-        LOGGER.info("Removing unwanted lines from %s", locale_po_file_path)
-        po_file_content = _remove_unwanted_lines(locale_po_file)
-
-        LOGGER.info(
-            "Writing stripped .po file to checkmk repository: %s", checkmk_po_file_path
-        )
-        try:
-            checkmk_po_file.write_text(po_file_content)
-            files_to_push_to_checkmk.append(checkmk_po_file_path)
-        except Exception as e:
-            LOGGER.error("Writing stripped .po file failed: %s", checkmk_po_file_path)
-            LOGGER.exception(e)
-            raise e
+        files_to_push_to_checkmk.append(result.path)
 
     LOGGER.info("Checking if any .po files changed in the checkmk repository")
     if not _check_if_repo_is_dirty(checkmk_repo):
         return
 
     LOGGER.info("Committing and pushing .po file to checkmk repository")
-    _commit_and_push_po_files(
+    commit_and_push_files(
         repo=checkmk_repo,
         files_to_push_to_repo=files_to_push_to_checkmk,
     )
@@ -96,19 +77,50 @@ def run(config: PoModeConfig) -> None:
     sys.exit(0)
 
 
-def _commit_and_push_po_files(
-    repo: Repo,
-    files_to_push_to_repo: list[Path],
-) -> None:
+def _process_po_file_pair(
+    file_pair: PoFilePair,
+    checkmk_repo: RepositoryConfig,
+    locale_repo: RepositoryConfig,
+) -> _Success | _Failure:
+    checkmk_po_file_path = file_pair.checkmk
+    locale_po_file_path = file_pair.locale
+    LOGGER.info("Checking formatting errors in %s", locale_po_file_path)
     try:
-        repo.index.add(files_to_push_to_repo)
-        repo.index.commit("Updateing .po files")
-        repo.remotes.origin.push()
+        run_subprocess(
+            ["msgfmt", "--check-format", locale_po_file_path],
+            check=True,
+            capture_output=True,
+            encoding="UTF-8",
+        )
     except CalledProcessError as e:
-        LOGGER.error("Committing and pushing .po files failed")
+        LOGGER.error(
+            "Formatting errors found in %s.\n\nStderr:\n%s",
+            locale_po_file_path,
+            e.stderr,
+        )
         LOGGER.exception(e)
-        raise e
-    LOGGER.info("Committing and pushing .po files succeeded")
+        return _Failure(e.stderr)
+    except Exception as e:
+        LOGGER.error("Checking formatting errors failed")
+        LOGGER.exception(e)
+        return _Failure(str(e))
+
+    locale_po_file = locale_repo.path / locale_po_file_path
+    checkmk_po_file = checkmk_repo.path / checkmk_po_file_path
+
+    LOGGER.info("Removing unwanted lines from %s", locale_po_file_path)
+    po_file_content = _remove_unwanted_lines(locale_po_file)
+
+    LOGGER.info(
+        "Writing stripped .po file to checkmk repository: %s", checkmk_po_file_path
+    )
+    try:
+        checkmk_po_file.write_text(po_file_content)
+    except Exception as e:
+        LOGGER.error("Writing stripped .po file failed: %s", checkmk_po_file_path)
+        LOGGER.exception(e)
+        return _Failure(str(e))
+    return _Success(checkmk_po_file_path)
 
 
 def _check_if_repo_is_dirty(repo: Repo) -> bool:

checkmk_weblate_syncer/pot.py

@@ -2,16 +2,16 @@
 from subprocess import run as run_subprocess
 
 from .config import PotModeConfig
+from .git import commit_and_push_files, repository_in_clean_state
 from .logging import LOGGER
-from .utils import get_repository_in_clean_state_with_logging
 
 
 def run(config: PotModeConfig) -> None:
-    get_repository_in_clean_state_with_logging(
+    repository_in_clean_state(
         config.checkmk_repository,
         "checkmk",
     )
-    locale_repo = get_repository_in_clean_state_with_logging(
+    locale_repo = repository_in_clean_state(
         config.locale_repository,
         "locale",
     )
@@ -57,11 +57,7 @@ def run(config: PotModeConfig) -> None:
         raise e
 
     LOGGER.info("Committing and pushing pot file to locale repository")
-    try:
-        locale_repo.index.add([path_pot_file])
-        locale_repo.index.commit("Update pot file")
-        locale_repo.remotes.origin.push()
-    except CalledProcessError as e:
-        LOGGER.error("Committing and pushing pot file failed")
-        LOGGER.exception(e)
-        raise e
+    commit_and_push_files(
+        repo=locale_repo,
+        files_to_push_to_repo=[path_pot_file],
+    )