From 8f2c33fe5d064171d9ac04ee6ce05ecc20dbdf3a Mon Sep 17 00:00:00 2001 From: Evgenii Alekseev Date: Tue, 30 May 2023 18:09:35 +0300 Subject: [PATCH] mimic parent arguments during spawn process (#99) --- .../application/handlers/unsafe_commands.py | 5 +- src/ahriman/application/handlers/web.py | 36 ++++++++++- src/ahriman/core/spawn.py | 14 ++--- .../application/handlers/test_handler_web.py | 63 +++++++++++++++++++ tests/ahriman/conftest.py | 2 +- tests/ahriman/core/test_spawn.py | 9 +-- 6 files changed, 113 insertions(+), 16 deletions(-) diff --git a/src/ahriman/application/handlers/unsafe_commands.py b/src/ahriman/application/handlers/unsafe_commands.py index ed0002af..dde37f59 100644 --- a/src/ahriman/application/handlers/unsafe_commands.py +++ b/src/ahriman/application/handlers/unsafe_commands.py @@ -79,5 +79,6 @@ class UnsafeCommands(Handler): """ # should never fail # pylint: disable=protected-access - subparser = next(action for action in parser._actions if isinstance(action, argparse._SubParsersAction)) - return sorted(action_name for action_name, action in subparser.choices.items() if action.get_default("unsafe")) + subparser = next((action for action in parser._actions if isinstance(action, argparse._SubParsersAction)), None) + actions = subparser.choices if subparser is not None else {} + return sorted(action_name for action_name, action in actions.items() if action.get_default("unsafe")) diff --git a/src/ahriman/application/handlers/web.py b/src/ahriman/application/handlers/web.py index b6ae9793..07963e47 100644 --- a/src/ahriman/application/handlers/web.py +++ b/src/ahriman/application/handlers/web.py @@ -19,6 +19,8 @@ # import argparse +from collections.abc import Generator + from ahriman.application.handlers import Handler from ahriman.core.configuration import Configuration from ahriman.core.spawn import Spawn @@ -31,6 +33,7 @@ class Web(Handler): ALLOW_AUTO_ARCHITECTURE_RUN = False ALLOW_MULTI_ARCHITECTURE_RUN = False # required to be able to spawn external processes + COMMAND_ARGS_WHITELIST = ["force", "log_handler", ""] @classmethod def run(cls, args: argparse.Namespace, architecture: str, configuration: Configuration, *, @@ -48,7 +51,8 @@ class Web(Handler): # we are using local import for optional dependencies from ahriman.web.web import run_server, setup_service - spawner = Spawn(args.parser(), architecture, configuration) + spawner_args = Web.extract_arguments(args, architecture, configuration) + spawner = Spawn(args.parser(), architecture, list(spawner_args)) spawner.start() application = setup_service(architecture, configuration, spawner) @@ -57,3 +61,33 @@ class Web(Handler): # terminate spawn process at the last spawner.stop() spawner.join() + + @staticmethod + def extract_arguments(args: argparse.Namespace, architecture: str, + configuration: Configuration) -> Generator[str, None, None]: + """ + extract list of arguments used for current command, except for command specific ones + + Args: + args(argparse.Namespace): command line args + architecture(str): repository architecture + configuration(Configuration): configuration instance + + Returns: + Generator[str, None, None]: command line arguments which were used for this specific command + """ + # read architecture from the same argument list + yield from ["--architecture", architecture] + # read configuration path from current settings + if (configuration_path := configuration.path) is not None: + yield from ["--configuration", str(configuration_path)] + + # arguments from command line + if args.force: + yield "--force" + if args.log_handler is not None: + yield from ["--log-handler", args.log_handler.value] + if args.quiet: + yield "--quiet" + if args.unsafe: + yield "--unsafe" diff --git a/src/ahriman/core/spawn.py b/src/ahriman/core/spawn.py index e0e41ccb..fc8c55a7 100644 --- a/src/ahriman/core/spawn.py +++ b/src/ahriman/core/spawn.py @@ -26,7 +26,6 @@ from collections.abc import Callable, Iterable from multiprocessing import Process, Queue from threading import Lock, Thread -from ahriman.core.configuration import Configuration from ahriman.core.log import LazyLogging from ahriman.models.package_source import PackageSource @@ -39,23 +38,24 @@ class Spawn(Thread, LazyLogging): Attributes: active(dict[str, Process]): map of active child processes required to avoid zombies architecture(str): repository architecture - configuration(Configuration): configuration instance + command_arguments(list[str]): base command line arguments queue(Queue[tuple[str, bool]]): multiprocessing queue to read updates from processes """ - def __init__(self, args_parser: argparse.ArgumentParser, architecture: str, configuration: Configuration) -> None: + def __init__(self, args_parser: argparse.ArgumentParser, architecture: str, command_arguments: list[str]) -> None: """ default constructor Args: args_parser(argparse.ArgumentParser): command line parser for the application architecture(str): repository architecture - configuration(Configuration): configuration instance + command_arguments(list[str]): base command line arguments """ Thread.__init__(self, name="spawn") self.architecture = architecture + self.args_parser = args_parser - self.configuration = configuration + self.command_arguments = command_arguments self.lock = Lock() self.active: dict[str, Process] = {} @@ -88,9 +88,7 @@ class Spawn(Thread, LazyLogging): **kwargs(str): named command arguments """ # default arguments - arguments = ["--architecture", self.architecture] - if self.configuration.path is not None: - arguments.extend(["--configuration", str(self.configuration.path)]) + arguments = self.command_arguments[:] # positional command arguments arguments.append(command) arguments.extend(args) diff --git a/tests/ahriman/application/handlers/test_handler_web.py b/tests/ahriman/application/handlers/test_handler_web.py index 01aee4e4..e668b1bc 100644 --- a/tests/ahriman/application/handlers/test_handler_web.py +++ b/tests/ahriman/application/handlers/test_handler_web.py @@ -6,6 +6,7 @@ from pytest_mock import MockerFixture from ahriman.application.handlers import Web from ahriman.core.configuration import Configuration from ahriman.core.repository import Repository +from ahriman.models.log_handler import LogHandler def _default_args(args: argparse.Namespace) -> argparse.Namespace: @@ -19,6 +20,11 @@ def _default_args(args: argparse.Namespace) -> argparse.Namespace: argparse.Namespace: generated arguments for these test cases """ args.parser = lambda: True + args.force = False + args.log_handler = None + args.report = True + args.quiet = False + args.unsafe = False return args @@ -43,6 +49,63 @@ def test_run(args: argparse.Namespace, configuration: Configuration, repository: join_mock.assert_called_once_with() +def test_extract_arguments(args: argparse.Namespace, configuration: Configuration): + """ + must extract correct args + """ + expected = [ + "--architecture", "x86_64", + "--configuration", str(configuration.path), + ] + + probe = _default_args(args) + assert list(Web.extract_arguments(probe, "x86_64", configuration)) == expected + + probe.force = True + expected.extend(["--force"]) + assert list(Web.extract_arguments(probe, "x86_64", configuration)) == expected + + probe.log_handler = LogHandler.Console + expected.extend(["--log-handler", probe.log_handler.value]) + assert list(Web.extract_arguments(probe, "x86_64", configuration)) == expected + + probe.quiet = True + expected.extend(["--quiet"]) + assert list(Web.extract_arguments(probe, "x86_64", configuration)) == expected + + probe.unsafe = True + expected.extend(["--unsafe"]) + assert list(Web.extract_arguments(probe, "x86_64", configuration)) == expected + + +def test_extract_arguments_full(parser: argparse.ArgumentParser, configuration: Configuration): + """ + must extract all available args except for blacklisted + """ + # append all options from parser + args = argparse.Namespace() + for action in parser._actions: + if action.default == argparse.SUPPRESS: + continue + # extract option from the following list + value = action.const or \ + next(iter(action.choices or []), None) or \ + (not action.default if isinstance(action.default, bool) else None) or \ + "random string" + if action.type is not None: + value = action.type(value) + setattr(args, action.dest, value) + + assert list(Web.extract_arguments(args, "x86_64", configuration)) == [ + "--architecture", "x86_64", + "--configuration", str(configuration.path), + "--force", + "--log-handler", "console", + "--quiet", + "--unsafe", + ] + + def test_disallow_auto_architecture_run() -> None: """ must not allow auto architecture run diff --git a/tests/ahriman/conftest.py b/tests/ahriman/conftest.py index a7a7a138..f4467442 100644 --- a/tests/ahriman/conftest.py +++ b/tests/ahriman/conftest.py @@ -482,7 +482,7 @@ def spawner(configuration: Configuration) -> Spawn: Returns: Spawn: spawner fixture """ - return Spawn(MagicMock(), "x86_64", configuration) + return Spawn(MagicMock(), "x86_64", ["--architecture", "x86_64", "--configuration", str(configuration.path)]) @pytest.fixture diff --git a/tests/ahriman/core/test_spawn.py b/tests/ahriman/core/test_spawn.py index 1b2366e9..e4dcee04 100644 --- a/tests/ahriman/core/test_spawn.py +++ b/tests/ahriman/core/test_spawn.py @@ -44,10 +44,11 @@ def test_spawn_process(spawner: Spawn, mocker: MockerFixture) -> None: spawner._spawn_process("add", "ahriman", now="", maybe="?") start_mock.assert_called_once_with() - spawner.args_parser.parse_args.assert_called_once_with([ - "--architecture", spawner.architecture, "--configuration", str(spawner.configuration.path), - "add", "ahriman", "--now", "--maybe", "?" - ]) + spawner.args_parser.parse_args.assert_called_once_with( + spawner.command_arguments + [ + "add", "ahriman", "--now", "--maybe", "?" + ] + ) def test_key_import(spawner: Spawn, mocker: MockerFixture) -> None: