From cafa76f680ae95f9ba52641f35e4f365394cb6bd Mon Sep 17 00:00:00 2001 From: Evgeniy Alekseev Date: Thu, 9 Sep 2021 00:35:07 +0300 Subject: [PATCH] make _call method of handlers public and also simplify process spawn --- src/ahriman/application/handlers/handler.py | 6 ++-- src/ahriman/core/spawn.py | 33 ++++++++----------- .../application/handlers/test_handler.py | 4 +-- tests/ahriman/core/test_spawn.py | 32 +++++++++--------- 4 files changed, 33 insertions(+), 42 deletions(-) diff --git a/src/ahriman/application/handlers/handler.py b/src/ahriman/application/handlers/handler.py index 850b6c0d..85bd88ad 100644 --- a/src/ahriman/application/handlers/handler.py +++ b/src/ahriman/application/handlers/handler.py @@ -40,7 +40,7 @@ class Handler: ALLOW_MULTI_ARCHITECTURE_RUN = True @classmethod - def _call(cls: Type[Handler], args: argparse.Namespace, architecture: str) -> bool: + def call(cls: Type[Handler], args: argparse.Namespace, architecture: str) -> bool: """ additional function to wrap all calls for multiprocessing library :param args: command line args @@ -72,9 +72,9 @@ class Handler: with Pool(len(architectures)) as pool: result = pool.starmap( - cls._call, [(args, architecture) for architecture in architectures]) + cls.call, [(args, architecture) for architecture in architectures]) else: - result = [cls._call(args, architectures.pop())] + result = [cls.call(args, architectures.pop())] return 0 if all(result) else 1 diff --git a/src/ahriman/core/spawn.py b/src/ahriman/core/spawn.py index 5da31bff..2664f74f 100644 --- a/src/ahriman/core/spawn.py +++ b/src/ahriman/core/spawn.py @@ -25,7 +25,7 @@ import uuid from multiprocessing import Process, Queue from threading import Lock, Thread -from typing import Callable, Dict, Iterable, Optional, Tuple +from typing import Callable, Dict, Iterable, Tuple from ahriman.core.configuration import Configuration @@ -57,27 +57,21 @@ class Spawn(Thread): self.lock = Lock() self.active: Dict[str, Process] = {} # stupid pylint does not know that it is possible - self.queue: Queue[Tuple[str, Optional[Exception]]] = Queue() # pylint: disable=unsubscriptable-object + self.queue: Queue[Tuple[str, bool]] = Queue() # pylint: disable=unsubscriptable-object @staticmethod - def process(callback: Callable[[argparse.Namespace, str, Configuration, bool], None], - args: argparse.Namespace, architecture: str, configuration: Configuration, - process_id: str, queue: Queue[Tuple[str, Optional[Exception]]]) -> None: # pylint: disable=unsubscriptable-object + def process(callback: Callable[[argparse.Namespace, str], bool], args: argparse.Namespace, architecture: str, + process_id: str, queue: Queue[Tuple[str, bool]]) -> None: # pylint: disable=unsubscriptable-object """ helper to run external process :param callback: application run function (i.e. Handler.run method) :param args: command line arguments :param architecture: repository architecture - :param configuration: configuration instance :param process_id: process unique identifier :param queue: output queue """ - try: - callback(args, architecture, configuration, args.no_report) - error = None - except Exception as e: - error = e - queue.put((process_id, error)) + result = callback(args, architecture) + queue.put((process_id, result)) def packages_add(self, packages: Iterable[str], now: bool) -> None: """ @@ -121,12 +115,14 @@ class Spawn(Thread): arguments.append(f"--{argument}") if value: arguments.append(value) + + process_id = str(uuid.uuid4()) + self.logger.info("full command line arguments of %s are %s", process_id, arguments) parsed = self.args_parser.parse_args(arguments) - callback = parsed.handler.run - process_id = str(uuid.uuid4()) + callback = parsed.handler.call process = Process(target=self.process, - args=(callback, parsed, self.architecture, self.configuration, process_id, self.queue), + args=(callback, parsed, self.architecture, process_id, self.queue), daemon=True) process.start() @@ -137,11 +133,8 @@ class Spawn(Thread): """ thread run method """ - for process_id, error in iter(self.queue.get, None): - if error is None: - self.logger.info("process %s has been terminated successfully", process_id) - else: - self.logger.exception("process %s has been terminated with exception %s", process_id, error) + for process_id, status in iter(self.queue.get, None): + self.logger.info("process %s has been terminated with status %s", process_id, status) with self.lock: process = self.active.pop(process_id, None) diff --git a/tests/ahriman/application/handlers/test_handler.py b/tests/ahriman/application/handlers/test_handler.py index ce7ac526..a8a770b0 100644 --- a/tests/ahriman/application/handlers/test_handler.py +++ b/tests/ahriman/application/handlers/test_handler.py @@ -20,7 +20,7 @@ def test_call(args: argparse.Namespace, mocker: MockerFixture) -> None: enter_mock = mocker.patch("ahriman.application.lock.Lock.__enter__") exit_mock = mocker.patch("ahriman.application.lock.Lock.__exit__") - assert Handler._call(args, "x86_64") + assert Handler.call(args, "x86_64") enter_mock.assert_called_once() exit_mock.assert_called_once() @@ -30,7 +30,7 @@ def test_call_exception(args: argparse.Namespace, mocker: MockerFixture) -> None must process exception """ mocker.patch("ahriman.application.lock.Lock.__enter__", side_effect=Exception()) - assert not Handler._call(args, "x86_64") + assert not Handler.call(args, "x86_64") def test_execute(args: argparse.Namespace, mocker: MockerFixture) -> None: diff --git a/tests/ahriman/core/test_spawn.py b/tests/ahriman/core/test_spawn.py index 9c33f3d1..2cce5b89 100644 --- a/tests/ahriman/core/test_spawn.py +++ b/tests/ahriman/core/test_spawn.py @@ -9,15 +9,15 @@ def test_process(spawner: Spawn) -> None: must process external process run correctly """ args = MagicMock() - args.no_report = False callback = MagicMock() + callback.return_value = True - spawner.process(callback, args, spawner.architecture, spawner.configuration, "id", spawner.queue) + spawner.process(callback, args, spawner.architecture, "id", spawner.queue) - callback.assert_called_with(args, spawner.architecture, spawner.configuration, False) - (uuid, error) = spawner.queue.get() + callback.assert_called_with(args, spawner.architecture) + (uuid, status) = spawner.queue.get() assert uuid == "id" - assert error is None + assert status assert spawner.queue.empty() @@ -26,13 +26,13 @@ def test_process_error(spawner: Spawn) -> None: must process external run with error correctly """ callback = MagicMock() - callback.side_effect = Exception() + callback.return_value = False - spawner.process(callback, MagicMock(), spawner.architecture, spawner.configuration, "id", spawner.queue) + spawner.process(callback, MagicMock(), spawner.architecture, "id", spawner.queue) - (uuid, error) = spawner.queue.get() + (uuid, status) = spawner.queue.get() assert uuid == "id" - assert isinstance(error, Exception) + assert not status assert spawner.queue.empty() @@ -90,16 +90,14 @@ def test_run(spawner: Spawn, mocker: MockerFixture) -> None: """ must implement run method """ - logging_exception_mock = mocker.patch("logging.Logger.exception") - logging_info_mock = mocker.patch("logging.Logger.info") + logging_mock = mocker.patch("logging.Logger.info") - spawner.queue.put(("1", None)) - spawner.queue.put(("2", Exception())) + spawner.queue.put(("1", False)) + spawner.queue.put(("2", True)) spawner.queue.put(None) # terminate spawner.run() - logging_exception_mock.assert_called_once() - logging_info_mock.assert_called_once() + logging_mock.assert_called() def test_run_pop(spawner: Spawn) -> None: @@ -109,8 +107,8 @@ def test_run_pop(spawner: Spawn) -> None: first = spawner.active["1"] = MagicMock() second = spawner.active["2"] = MagicMock() - spawner.queue.put(("1", None)) - spawner.queue.put(("2", Exception())) + spawner.queue.put(("1", False)) + spawner.queue.put(("2", True)) spawner.queue.put(None) # terminate spawner.run()