gracefully terminate web server

In previous revisions server was terminated by itself, thus no lock or
socket was removed. In new version, graceful termination of the queue
has been added as well as server now handles singals

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()

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:
+    def _spawn_process(self, command: str, *args: str, **kwargs: 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:
         """
         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)

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)
 
 

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:

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()

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
     )