diff --git a/src/ahriman/application/handlers/web.py b/src/ahriman/application/handlers/web.py index 6e62de43..df5ca37a 100644 --- a/src/ahriman/application/handlers/web.py +++ b/src/ahriman/application/handlers/web.py @@ -55,3 +55,7 @@ class Web(Handler): application = setup_service(architecture, configuration, spawner) run_server(application) + + # terminate spawn process at the last + spawner.stop() + spawner.join() diff --git a/src/ahriman/core/spawn.py b/src/ahriman/core/spawn.py index 1413309a..bccf45d2 100644 --- a/src/ahriman/core/spawn.py +++ b/src/ahriman/core/spawn.py @@ -60,7 +60,7 @@ class Spawn(Thread, LazyLogging): self.lock = Lock() self.active: Dict[str, Process] = {} # stupid pylint does not know that it is possible - self.queue: Queue[Tuple[str, bool]] = Queue() # pylint: disable=unsubscriptable-object + self.queue: Queue[Tuple[str, bool] | None] = Queue() # pylint: disable=unsubscriptable-object @staticmethod def process(callback: Callable[[argparse.Namespace, str], bool], args: argparse.Namespace, architecture: str, @@ -78,55 +78,7 @@ class Spawn(Thread, LazyLogging): result = callback(args, architecture) queue.put((process_id, result)) - def key_import(self, key: str, server: Optional[str]) -> None: - """ - import key to service cache - - Args: - key(str): key to import - server(str): PGP key server - """ - kwargs = {} if server is None else {"key-server": server} - self.spawn_process("service-key-import", key, **kwargs) - - def packages_add(self, packages: Iterable[str], *, now: bool) -> None: - """ - add packages - - Args: - packages(Iterable[str]): packages list to add - now(bool): build packages now - """ - kwargs = {"source": PackageSource.AUR.value} # avoid abusing by building non-aur packages - if now: - kwargs["now"] = "" - self.spawn_process("package-add", *packages, **kwargs) - - def packages_rebuild(self, depends_on: str) -> None: - """ - rebuild packages which depend on the specified package - - Args: - depends_on(str): packages dependency - """ - self.spawn_process("repo-rebuild", **{"depends-on": depends_on}) - - def packages_remove(self, packages: Iterable[str]) -> None: - """ - remove packages - - Args: - packages(Iterable[str]): packages list to remove - """ - self.spawn_process("package-remove", *packages) - - def packages_update(self, ) -> None: - """ - run full repository update - """ - self.spawn_process("repo-update") - - def spawn_process(self, command: str, *args: str, **kwargs: str) -> None: + def _spawn_process(self, command: str, *args: str, **kwargs: str) -> None: """ spawn external ahriman process with supplied arguments @@ -161,6 +113,54 @@ class Spawn(Thread, LazyLogging): with self.lock: self.active[process_id] = process + def key_import(self, key: str, server: Optional[str]) -> None: + """ + import key to service cache + + Args: + key(str): key to import + server(str): PGP key server + """ + kwargs = {} if server is None else {"key-server": server} + self._spawn_process("service-key-import", key, **kwargs) + + def packages_add(self, packages: Iterable[str], *, now: bool) -> None: + """ + add packages + + Args: + packages(Iterable[str]): packages list to add + now(bool): build packages now + """ + kwargs = {"source": PackageSource.AUR.value} # avoid abusing by building non-aur packages + if now: + kwargs["now"] = "" + self._spawn_process("package-add", *packages, **kwargs) + + def packages_rebuild(self, depends_on: str) -> None: + """ + rebuild packages which depend on the specified package + + Args: + depends_on(str): packages dependency + """ + self._spawn_process("repo-rebuild", **{"depends-on": depends_on}) + + def packages_remove(self, packages: Iterable[str]) -> None: + """ + remove packages + + Args: + packages(Iterable[str]): packages list to remove + """ + self._spawn_process("package-remove", *packages) + + def packages_update(self, ) -> None: + """ + run full repository update + """ + self._spawn_process("repo-update") + def run(self) -> None: """ thread run method @@ -174,3 +174,9 @@ class Spawn(Thread, LazyLogging): if process is not None: process.terminate() # make sure lol process.join() + + def stop(self) -> None: + """ + gracefully terminate thread + """ + self.queue.put(None) diff --git a/src/ahriman/web/web.py b/src/ahriman/web/web.py index ce9e7961..826d10b8 100644 --- a/src/ahriman/web/web.py +++ b/src/ahriman/web/web.py @@ -115,7 +115,7 @@ def run_server(application: web.Application) -> None: port = configuration.getint("web", "port") unix_socket = create_socket(configuration, application) - web.run_app(application, host=host, port=port, sock=unix_socket, handle_signals=False, + web.run_app(application, host=host, port=port, sock=unix_socket, handle_signals=True, access_log=logging.getLogger("http"), access_log_class=FilteredAccessLogger) diff --git a/tests/ahriman/application/handlers/test_handler_web.py b/tests/ahriman/application/handlers/test_handler_web.py index 91d4698a..01aee4e4 100644 --- a/tests/ahriman/application/handlers/test_handler_web.py +++ b/tests/ahriman/application/handlers/test_handler_web.py @@ -28,14 +28,19 @@ def test_run(args: argparse.Namespace, configuration: Configuration, repository: must run command """ args = _default_args(args) - mocker.patch("ahriman.core.spawn.Spawn.start") mocker.patch("ahriman.core.repository.Repository.load", return_value=repository) setup_mock = mocker.patch("ahriman.web.web.setup_service") run_mock = mocker.patch("ahriman.web.web.run_server") + start_mock = mocker.patch("ahriman.core.spawn.Spawn.start") + stop_mock = mocker.patch("ahriman.core.spawn.Spawn.stop") + join_mock = mocker.patch("ahriman.core.spawn.Spawn.join") Web.run(args, "x86_64", configuration, report=False, unsafe=False) setup_mock.assert_called_once_with("x86_64", configuration, pytest.helpers.anyvar(int)) run_mock.assert_called_once_with(pytest.helpers.anyvar(int)) + start_mock.assert_called_once_with() + stop_mock.assert_called_once_with() + join_mock.assert_called_once_with() def test_disallow_auto_architecture_run() -> None: diff --git a/tests/ahriman/core/test_spawn.py b/tests/ahriman/core/test_spawn.py index cdccac83..98f2cabc 100644 --- a/tests/ahriman/core/test_spawn.py +++ b/tests/ahriman/core/test_spawn.py @@ -36,11 +36,25 @@ def test_process_error(spawner: Spawn) -> None: assert spawner.queue.empty() +def test_spawn_process(spawner: Spawn, mocker: MockerFixture) -> None: + """ + must correctly spawn child process + """ + start_mock = mocker.patch("multiprocessing.Process.start") + + 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", "?" + ]) + + def test_key_import(spawner: Spawn, mocker: MockerFixture) -> None: """ must call key import """ - spawn_mock = mocker.patch("ahriman.core.spawn.Spawn.spawn_process") + spawn_mock = mocker.patch("ahriman.core.spawn.Spawn._spawn_process") spawner.key_import("0xdeadbeaf", None) spawn_mock.assert_called_once_with("service-key-import", "0xdeadbeaf") @@ -49,7 +63,7 @@ def test_key_import_with_server(spawner: Spawn, mocker: MockerFixture) -> None: """ must call key import with server specified """ - spawn_mock = mocker.patch("ahriman.core.spawn.Spawn.spawn_process") + spawn_mock = mocker.patch("ahriman.core.spawn.Spawn._spawn_process") spawner.key_import("0xdeadbeaf", "keyserver.ubuntu.com") spawn_mock.assert_called_once_with("service-key-import", "0xdeadbeaf", **{"key-server": "keyserver.ubuntu.com"}) @@ -58,7 +72,7 @@ def test_packages_add(spawner: Spawn, mocker: MockerFixture) -> None: """ must call package addition """ - spawn_mock = mocker.patch("ahriman.core.spawn.Spawn.spawn_process") + spawn_mock = mocker.patch("ahriman.core.spawn.Spawn._spawn_process") spawner.packages_add(["ahriman", "linux"], now=False) spawn_mock.assert_called_once_with("package-add", "ahriman", "linux", source="aur") @@ -67,7 +81,7 @@ def test_packages_add_with_build(spawner: Spawn, mocker: MockerFixture) -> None: """ must call package addition with update """ - spawn_mock = mocker.patch("ahriman.core.spawn.Spawn.spawn_process") + spawn_mock = mocker.patch("ahriman.core.spawn.Spawn._spawn_process") spawner.packages_add(["ahriman", "linux"], now=True) spawn_mock.assert_called_once_with("package-add", "ahriman", "linux", source="aur", now="") @@ -76,7 +90,7 @@ def test_packages_rebuild(spawner: Spawn, mocker: MockerFixture) -> None: """ must call package rebuild """ - spawn_mock = mocker.patch("ahriman.core.spawn.Spawn.spawn_process") + spawn_mock = mocker.patch("ahriman.core.spawn.Spawn._spawn_process") spawner.packages_rebuild("python") spawn_mock.assert_called_once_with("repo-rebuild", **{"depends-on": "python"}) @@ -85,7 +99,7 @@ def test_packages_remove(spawner: Spawn, mocker: MockerFixture) -> None: """ must call package removal """ - spawn_mock = mocker.patch("ahriman.core.spawn.Spawn.spawn_process") + spawn_mock = mocker.patch("ahriman.core.spawn.Spawn._spawn_process") spawner.packages_remove(["ahriman", "linux"]) spawn_mock.assert_called_once_with("package-remove", "ahriman", "linux") @@ -94,25 +108,11 @@ def test_packages_update(spawner: Spawn, mocker: MockerFixture) -> None: """ must call repo update """ - spawn_mock = mocker.patch("ahriman.core.spawn.Spawn.spawn_process") + spawn_mock = mocker.patch("ahriman.core.spawn.Spawn._spawn_process") spawner.packages_update() spawn_mock.assert_called_once_with("repo-update") -def test_spawn_process(spawner: Spawn, mocker: MockerFixture) -> None: - """ - must correctly spawn child process - """ - start_mock = mocker.patch("multiprocessing.Process.start") - - 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", "?" - ]) - - def test_run(spawner: Spawn, mocker: MockerFixture) -> None: """ must implement run method @@ -145,3 +145,14 @@ def test_run_pop(spawner: Spawn) -> None: second.terminate.assert_called_once_with() second.join.assert_called_once_with() assert not spawner.active + + +def test_stop(spawner: Spawn) -> None: + """ + must gracefully terminate thread + """ + spawner.start() + spawner.stop() + spawner.join() + + assert not spawner.is_alive() diff --git a/tests/ahriman/web/test_web.py b/tests/ahriman/web/test_web.py index 5a0e80dd..49cede47 100644 --- a/tests/ahriman/web/test_web.py +++ b/tests/ahriman/web/test_web.py @@ -100,7 +100,7 @@ def test_run(application: web.Application, mocker: MockerFixture) -> None: run_server(application) run_application_mock.assert_called_once_with( - application, host="127.0.0.1", port=port, sock=None, handle_signals=False, + application, host="127.0.0.1", port=port, sock=None, handle_signals=True, access_log=pytest.helpers.anyvar(int), access_log_class=FilteredAccessLogger ) @@ -115,7 +115,7 @@ def test_run_with_auth(application_with_auth: web.Application, mocker: MockerFix run_server(application_with_auth) run_application_mock.assert_called_once_with( - application_with_auth, host="127.0.0.1", port=port, sock=None, handle_signals=False, + application_with_auth, host="127.0.0.1", port=port, sock=None, handle_signals=True, access_log=pytest.helpers.anyvar(int), access_log_class=FilteredAccessLogger ) @@ -130,7 +130,7 @@ def test_run_with_debug(application_with_debug: web.Application, mocker: MockerF run_server(application_with_debug) run_application_mock.assert_called_once_with( - application_with_debug, host="127.0.0.1", port=port, sock=None, handle_signals=False, + application_with_debug, host="127.0.0.1", port=port, sock=None, handle_signals=True, access_log=pytest.helpers.anyvar(int), access_log_class=FilteredAccessLogger ) @@ -147,6 +147,6 @@ def test_run_with_socket(application: web.Application, mocker: MockerFixture) -> run_server(application) socket_mock.assert_called_once_with(application["configuration"], application) run_application_mock.assert_called_once_with( - application, host="127.0.0.1", port=port, sock=42, handle_signals=False, + application, host="127.0.0.1", port=port, sock=42, handle_signals=True, access_log=pytest.helpers.anyvar(int), access_log_class=FilteredAccessLogger )