From f87ca3f6f4a44acf4b4bef62d4c4ce8acbbae7d0 Mon Sep 17 00:00:00 2001 From: Luka Racic Date: Wed, 8 May 2024 00:21:26 +0200 Subject: [PATCH] po_mode: Implement suggestions from review --- checkmk_weblate_syncer/config.py | 13 ++-- checkmk_weblate_syncer/git.py | 38 +++++++++- checkmk_weblate_syncer/po.py | 124 +++++++++++++++++-------------- checkmk_weblate_syncer/pot.py | 18 ++--- checkmk_weblate_syncer/utils.py | 20 ----- 5 files changed, 119 insertions(+), 94 deletions(-) delete mode 100644 checkmk_weblate_syncer/utils.py diff --git a/checkmk_weblate_syncer/config.py b/checkmk_weblate_syncer/config.py index 375f229..3a39877 100644 --- a/checkmk_weblate_syncer/config.py +++ b/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] diff --git a/checkmk_weblate_syncer/git.py b/checkmk_weblate_syncer/git.py index 2421217..807a60c 100644 --- a/checkmk_weblate_syncer/git.py +++ b/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") diff --git a/checkmk_weblate_syncer/po.py b/checkmk_weblate_syncer/po.py index 4216961..0d040ba 100644 --- a/checkmk_weblate_syncer/po.py +++ b/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,11 +29,11 @@ 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", ) @@ -31,57 +41,28 @@ def run(config: PoModeConfig) -> None: 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: diff --git a/checkmk_weblate_syncer/pot.py b/checkmk_weblate_syncer/pot.py index 9a743f7..2ba4900 100644 --- a/checkmk_weblate_syncer/pot.py +++ b/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], + ) diff --git a/checkmk_weblate_syncer/utils.py b/checkmk_weblate_syncer/utils.py deleted file mode 100644 index 963abc2..0000000 --- a/checkmk_weblate_syncer/utils.py +++ /dev/null @@ -1,20 +0,0 @@ -from git import Repo - -from .config import RepositoryConfig -from .git import repository_in_clean_state -from .logging import LOGGER - - -def get_repository_in_clean_state_with_logging( - 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