From b561bcc25d22da20b678316a9f123f1d11bb34ff Mon Sep 17 00:00:00 2001 From: Evgeniy Alekseev Date: Tue, 14 Sep 2021 03:57:20 +0300 Subject: [PATCH] remove own implementations of getlist and getpath method in order to use converters feature --- docs/configuration.md | 4 +- src/ahriman/application/handlers/handler.py | 3 +- src/ahriman/core/auth/auth.py | 4 +- src/ahriman/core/build_tools/task.py | 6 +-- src/ahriman/core/configuration.py | 50 +++++++------------ src/ahriman/core/repository/properties.py | 2 +- .../application/handlers/test_handler.py | 11 ++-- .../ahriman/core/repository/test_executor.py | 15 +++--- tests/ahriman/core/test_configuration.py | 2 +- 9 files changed, 42 insertions(+), 55 deletions(-) diff --git a/docs/configuration.md b/docs/configuration.md index 24e33c4d..19f155b1 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -71,7 +71,7 @@ Settings for signing packages or repository. Group name must refer to architectu Report generation settings. -* `target` - list of reports to be generated, space separated list of strings, optional. Allowed values are `html`, `email`. +* `target` - list of reports to be generated, space separated list of strings, required. Allowed values are `html`, `email`. ### `email:*` groups @@ -103,7 +103,7 @@ Group name must refer to architecture, e.g. it should be `html:x86_64` for x86_6 Remote synchronization settings. -* `target` - list of synchronizations to be used, space separated list of strings, optional. Allowed values are `rsync`, `s3`. +* `target` - list of synchronizations to be used, space separated list of strings, required. Allowed values are `rsync`, `s3`. ### `rsync:*` groups diff --git a/src/ahriman/application/handlers/handler.py b/src/ahriman/application/handlers/handler.py index 85bd88ad..21c928df 100644 --- a/src/ahriman/application/handlers/handler.py +++ b/src/ahriman/application/handlers/handler.py @@ -92,7 +92,8 @@ class Handler: config = Configuration() config.load(args.configuration) - root = config.getpath("repository", "root") + # wtf??? + root = config.getpath("repository", "root") # pylint: disable=assignment-from-no-return architectures = RepositoryPaths.known_architectures(root) if not architectures: diff --git a/src/ahriman/core/auth/auth.py b/src/ahriman/core/auth/auth.py index a3be9ef4..f1b94d7f 100644 --- a/src/ahriman/core/auth/auth.py +++ b/src/ahriman/core/auth/auth.py @@ -52,9 +52,9 @@ class Auth: self.logger = logging.getLogger("http") self.allow_read_only = configuration.getboolean("auth", "allow_read_only") - self.allowed_paths = set(configuration.getlist("auth", "allowed_paths")) + self.allowed_paths = set(configuration.getlist("auth", "allowed_paths", fallback=[])) self.allowed_paths.update(self.ALLOWED_PATHS) - self.allowed_paths_groups = set(configuration.getlist("auth", "allowed_paths_groups")) + self.allowed_paths_groups = set(configuration.getlist("auth", "allowed_paths_groups", fallback=[])) self.allowed_paths_groups.update(self.ALLOWED_PATHS_GROUPS) self.enabled = provider.is_enabled self.max_age = configuration.getint("auth", "max_age", fallback=7 * 24 * 3600) diff --git a/src/ahriman/core/build_tools/task.py b/src/ahriman/core/build_tools/task.py index 32045f37..0b0ecf0f 100644 --- a/src/ahriman/core/build_tools/task.py +++ b/src/ahriman/core/build_tools/task.py @@ -53,10 +53,10 @@ class Task: self.package = package self.paths = paths - self.archbuild_flags = configuration.getlist("build", "archbuild_flags") + self.archbuild_flags = configuration.getlist("build", "archbuild_flags", fallback=[]) self.build_command = configuration.get("build", "build_command") - self.makepkg_flags = configuration.getlist("build", "makepkg_flags") - self.makechrootpkg_flags = configuration.getlist("build", "makechrootpkg_flags") + self.makepkg_flags = configuration.getlist("build", "makepkg_flags", fallback=[]) + self.makechrootpkg_flags = configuration.getlist("build", "makechrootpkg_flags", fallback=[]) @property def cache_path(self) -> Path: diff --git a/src/ahriman/core/configuration.py b/src/ahriman/core/configuration.py index e7c2a1ea..a7195b3c 100644 --- a/src/ahriman/core/configuration.py +++ b/src/ahriman/core/configuration.py @@ -41,13 +41,14 @@ class Configuration(configparser.RawConfigParser): ARCHITECTURE_SPECIFIC_SECTIONS = ["build", "html", "rsync", "s3", "sign", "web"] - _UNSET = object() - def __init__(self) -> None: """ default constructor. In the most cases must not be called directly """ - configparser.RawConfigParser.__init__(self, allow_no_value=True) + configparser.RawConfigParser.__init__(self, allow_no_value=True, converters={ + "list": lambda value: value.split(), + "path": self.__convert_path, + }) self.path: Optional[Path] = None @property @@ -89,6 +90,17 @@ class Configuration(configparser.RawConfigParser): """ return f"{section}:{suffix}" + def __convert_path(self, parsed: str) -> Path: + """ + convert string value to path object + :param parsed: string configuration value + :return: path object which represents the configuration value + """ + value = Path(parsed) + if self.path is None or value.is_absolute(): + return value + return self.path.parent / value + def dump(self) -> Dict[str, Dict[str, str]]: """ dump configuration to dictionary @@ -99,35 +111,11 @@ class Configuration(configparser.RawConfigParser): for section in self.sections() } - def getlist(self, section: str, key: str) -> List[str]: - """ - get space separated string list option - :param section: section name - :param key: key name - :return: list of string if option is set, empty list otherwise - """ - raw = self.get(section, key, fallback=None) - if not raw: # empty string or none - return [] - return raw.split() + # pylint and mypy are too stupid to find these methods + # pylint: disable=missing-function-docstring,multiple-statements,unused-argument,no-self-use + def getlist(self, *args: Any, **kwargs: Any) -> List[str]: ... - def getpath(self, section: str, key: str, fallback: Any = _UNSET) -> Path: - """ - helper to generate absolute configuration path for relative settings value - :param section: section name - :param key: key name - :param fallback: optional fallback value - :return: absolute path according to current path configuration - """ - try: - value = Path(self.get(section, key)) - except (configparser.NoOptionError, configparser.NoSectionError): - if fallback is self._UNSET: - raise - value = fallback - if self.path is None or not isinstance(value, Path) or value.is_absolute(): - return value - return self.path.parent / value + def getpath(self, *args: Any, **kwargs: Any) -> Path: ... def load(self, path: Path) -> None: """ diff --git a/src/ahriman/core/repository/properties.py b/src/ahriman/core/repository/properties.py index 54e9ebc2..88d860e4 100644 --- a/src/ahriman/core/repository/properties.py +++ b/src/ahriman/core/repository/properties.py @@ -60,7 +60,7 @@ class Properties: self.paths = RepositoryPaths(configuration.getpath("repository", "root"), architecture) self.paths.create_tree() - self.ignore_list = configuration.getlist("build", "ignore_packages") + self.ignore_list = configuration.getlist("build", "ignore_packages", fallback=[]) self.pacman = Pacman(configuration) self.sign = GPG(architecture, configuration) self.repo = Repo(self.name, self.paths, self.sign.repository_sign_args) diff --git a/tests/ahriman/application/handlers/test_handler.py b/tests/ahriman/application/handlers/test_handler.py index a8a770b0..d46b05b5 100644 --- a/tests/ahriman/application/handlers/test_handler.py +++ b/tests/ahriman/application/handlers/test_handler.py @@ -67,27 +67,26 @@ def test_execute_single(args: argparse.Namespace, mocker: MockerFixture) -> None starmap_mock.assert_not_called() -def test_extract_architectures(args: argparse.Namespace, mocker: MockerFixture) -> None: +def test_extract_architectures(args: argparse.Namespace, configuration: Configuration, mocker: MockerFixture) -> None: """ must generate list of available architectures """ args.architecture = [] - args.configuration = Path("") - mocker.patch("ahriman.core.configuration.Configuration.getpath") + args.configuration = configuration.path known_architectures_mock = mocker.patch("ahriman.models.repository_paths.RepositoryPaths.known_architectures") Handler.extract_architectures(args) known_architectures_mock.assert_called_once() -def test_extract_architectures_empty(args: argparse.Namespace, mocker: MockerFixture) -> None: +def test_extract_architectures_empty(args: argparse.Namespace, configuration: Configuration, + mocker: MockerFixture) -> None: """ must raise exception if no available architectures found """ args.architecture = [] args.command = "config" - args.configuration = Path("") - mocker.patch("ahriman.core.configuration.Configuration.getpath") + args.configuration = configuration.path mocker.patch("ahriman.models.repository_paths.RepositoryPaths.known_architectures", return_value=set()) with pytest.raises(MissingArchitecture): diff --git a/tests/ahriman/core/repository/test_executor.py b/tests/ahriman/core/repository/test_executor.py index 8bbd6ba4..2062e37a 100644 --- a/tests/ahriman/core/repository/test_executor.py +++ b/tests/ahriman/core/repository/test_executor.py @@ -3,6 +3,7 @@ import pytest from pathlib import Path from pytest_mock import MockerFixture from unittest import mock +from unittest.mock import MagicMock from ahriman.core.report.report import Report from ahriman.core.repository.executor import Executor @@ -137,14 +138,13 @@ def test_process_report(executor: Executor, package_ahriman: Package, mocker: Mo report_mock.assert_called_once() -def test_process_report_auto(executor: Executor, mocker: MockerFixture) -> None: +def test_process_report_auto(executor: Executor) -> None: """ must process report in auto mode if no targets supplied """ - configuration_getlist_mock = mocker.patch("ahriman.core.configuration.Configuration.getlist") - + configuration_mock = executor.configuration = MagicMock() executor.process_report(None, []) - configuration_getlist_mock.assert_called_once() + configuration_mock.getlist.assert_called_once() def test_process_upload(executor: Executor, mocker: MockerFixture) -> None: @@ -158,14 +158,13 @@ def test_process_upload(executor: Executor, mocker: MockerFixture) -> None: upload_mock.assert_called_once() -def test_process_upload_auto(executor: Executor, mocker: MockerFixture) -> None: +def test_process_upload_auto(executor: Executor) -> None: """ must process sync in auto mode if no targets supplied """ - configuration_getlist_mock = mocker.patch("ahriman.core.configuration.Configuration.getlist") - + configuration_mock = executor.configuration = MagicMock() executor.process_sync(None, []) - configuration_getlist_mock.assert_called_once() + configuration_mock.getlist.assert_called_once() def test_process_update(executor: Executor, package_ahriman: Package, mocker: MockerFixture) -> None: diff --git a/tests/ahriman/core/test_configuration.py b/tests/ahriman/core/test_configuration.py index e61a3572..5392301d 100644 --- a/tests/ahriman/core/test_configuration.py +++ b/tests/ahriman/core/test_configuration.py @@ -104,7 +104,7 @@ def test_getlist_empty(configuration: Configuration) -> None: """ must return list of string correctly for non-existing option """ - assert configuration.getlist("build", "test_list") == [] + assert configuration.getlist("build", "test_list", fallback=[]) == [] configuration.set_option("build", "test_list", "") assert configuration.getlist("build", "test_list") == []