From dff5b775a9274ca7c11af9b7c21b2c47a7a9b588 Mon Sep 17 00:00:00 2001 From: Evgenii Alekseev Date: Tue, 15 Jul 2025 01:59:03 +0300 Subject: [PATCH] refactor: move logs rotation to separated trigger which is enabled by default Previous solution, well, worked kinda fine-ish, though we have much better mechanisms to do so --- docs/ahriman.core.housekeeping.rst | 21 +++++ docs/ahriman.core.rst | 1 + docs/architecture.rst | 1 + docs/configuration.rst | 10 ++- package/archlinux/PKGBUILD | 2 + package/share/ahriman/settings/ahriman.ini | 4 +- .../ahriman.ini.d/00-housekeeping.ini | 3 + src/ahriman/core/configuration/schema.py | 5 -- src/ahriman/core/housekeeping/__init__.py | 20 +++++ .../housekeeping/logs_rotation_trigger.py | 87 +++++++++++++++++++ src/ahriman/core/log/http_log_handler.py | 10 --- tests/ahriman/core/housekeeping/conftest.py | 19 ++++ .../test_logs_rotation_trigger.py | 26 ++++++ .../ahriman/core/log/test_http_log_handler.py | 15 ---- tests/testresources/core/ahriman.ini | 3 + 15 files changed, 193 insertions(+), 34 deletions(-) create mode 100644 docs/ahriman.core.housekeeping.rst create mode 100644 package/share/ahriman/settings/ahriman.ini.d/00-housekeeping.ini create mode 100644 src/ahriman/core/housekeeping/__init__.py create mode 100644 src/ahriman/core/housekeeping/logs_rotation_trigger.py create mode 100644 tests/ahriman/core/housekeeping/conftest.py create mode 100644 tests/ahriman/core/housekeeping/test_logs_rotation_trigger.py diff --git a/docs/ahriman.core.housekeeping.rst b/docs/ahriman.core.housekeeping.rst new file mode 100644 index 00000000..7a80d7c6 --- /dev/null +++ b/docs/ahriman.core.housekeeping.rst @@ -0,0 +1,21 @@ +ahriman.core.housekeeping package +================================= + +Submodules +---------- + +ahriman.core.housekeeping.logs\_rotation\_trigger module +-------------------------------------------------------- + +.. automodule:: ahriman.core.housekeeping.logs_rotation_trigger + :members: + :no-undoc-members: + :show-inheritance: + +Module contents +--------------- + +.. automodule:: ahriman.core.housekeeping + :members: + :no-undoc-members: + :show-inheritance: diff --git a/docs/ahriman.core.rst b/docs/ahriman.core.rst index b69f9d62..e302d8b1 100644 --- a/docs/ahriman.core.rst +++ b/docs/ahriman.core.rst @@ -15,6 +15,7 @@ Subpackages ahriman.core.distributed ahriman.core.formatters ahriman.core.gitremote + ahriman.core.housekeeping ahriman.core.http ahriman.core.log ahriman.core.report diff --git a/docs/architecture.rst b/docs/architecture.rst index c7b51faa..51dca71c 100644 --- a/docs/architecture.rst +++ b/docs/architecture.rst @@ -40,6 +40,7 @@ This package contains everything required for the most of application actions an * ``ahriman.core.distributed`` package with triggers and helpers for distributed build system. * ``ahriman.core.formatters`` package provides ``Printer`` sub-classes for printing data (e.g. package properties) to stdout which are used by some handlers. * ``ahriman.core.gitremote`` is a package with remote PKGBUILD triggers. Should not be called directly. +* ``ahriman.core.housekeeping`` package provides few triggers for removing old data. * ``ahriman.core.http`` package provides HTTP clients which can be used later by other classes. * ``ahriman.core.log`` is a log utils package. It includes logger loader class, custom HTTP based logger and some wrappers. * ``ahriman.core.report`` is a package with reporting triggers. Should not be called directly. diff --git a/docs/configuration.rst b/docs/configuration.rst index f779365c..87d74714 100644 --- a/docs/configuration.rst +++ b/docs/configuration.rst @@ -81,7 +81,6 @@ Base configuration settings. * ``apply_migrations`` - perform database migrations on the application start, boolean, optional, default ``yes``. Useful if you are using git version. Note, however, that this option must be changed only if you know what to do and going to handle migrations manually. * ``database`` - path to the application SQLite database, string, required. * ``include`` - path to directory with configuration files overrides, string, optional. Files will be read in alphabetical order. -* ``keep_last_logs`` - amount of build logs to be kept for each package, integer, optional ,default ``0``. Logs will be cleared at the end of each process. * ``logging`` - path to logging configuration, string, required. Check ``logging.ini`` for reference. ``alpm:*`` groups @@ -180,7 +179,7 @@ Web server settings. This feature requires ``aiohttp`` libraries to be installed * ``wait_timeout`` - wait timeout in seconds, maximum amount of time to be waited before lock will be free, integer, optional. ``keyring`` group --------------------- +----------------- Keyring package generator plugin. @@ -198,6 +197,13 @@ Keyring generator plugin * ``revoked`` - list of revoked packagers keys, space separated list of strings, optional. * ``trusted`` - list of master keys, space separated list of strings, optional, if not set, the ``key`` option from ``sign`` group will be used. +``housekeeping`` group +---------------------- + +This section describes settings for the ``ahriman.core.housekeeping.LogsRotationTrigger`` plugin. + +* ``keep_last_logs`` - amount of build logs to be kept for each package, integer, optional ,default ``0``. Logs will be cleared at the end of each process. + ``mirrorlist`` group -------------------- diff --git a/package/archlinux/PKGBUILD b/package/archlinux/PKGBUILD index a2a9fdf3..722f5c16 100644 --- a/package/archlinux/PKGBUILD +++ b/package/archlinux/PKGBUILD @@ -40,6 +40,7 @@ package_ahriman-core() { 'rsync: sync by using rsync') install="$pkgbase.install" backup=('etc/ahriman.ini' + 'etc/ahriman.ini.d/00-housekeeping.ini' 'etc/ahriman.ini.d/logging.ini') cd "$pkgbase-$pkgver" @@ -49,6 +50,7 @@ package_ahriman-core() { # keep usr/share configs as reference and copy them to /etc install -Dm644 "$pkgdir/usr/share/$pkgbase/settings/ahriman.ini" "$pkgdir/etc/ahriman.ini" + install -Dm644 "$pkgdir/usr/share/$pkgbase/settings/ahriman.ini.d/00-housekeeping.ini" "$pkgdir/etc/ahriman.ini.d/00-housekeeping.ini" install -Dm644 "$pkgdir/usr/share/$pkgbase/settings/ahriman.ini.d/logging.ini" "$pkgdir/etc/ahriman.ini.d/logging.ini" install -Dm644 "$srcdir/$pkgbase.sysusers" "$pkgdir/usr/lib/sysusers.d/$pkgbase.conf" diff --git a/package/share/ahriman/settings/ahriman.ini b/package/share/ahriman/settings/ahriman.ini index e53a1fa2..40b8acb0 100644 --- a/package/share/ahriman/settings/ahriman.ini +++ b/package/share/ahriman/settings/ahriman.ini @@ -7,8 +7,6 @@ logging = ahriman.ini.d/logging.ini ;apply_migrations = yes ; Path to the application SQLite database. database = ${repository:root}/ahriman.db -; Keep last build logs for each package -keep_last_logs = 5 [alpm] ; Path to pacman system database cache. @@ -45,9 +43,11 @@ triggers[] = ahriman.core.gitremote.RemotePullTrigger triggers[] = ahriman.core.report.ReportTrigger triggers[] = ahriman.core.upload.UploadTrigger triggers[] = ahriman.core.gitremote.RemotePushTrigger +triggers[] = ahriman.core.housekeeping.LogsRotationTrigger ; List of well-known triggers. Used only for configuration purposes. triggers_known[] = ahriman.core.gitremote.RemotePullTrigger triggers_known[] = ahriman.core.gitremote.RemotePushTrigger +triggers_known[] = ahriman.core.housekeeping.LogsRotationTrigger triggers_known[] = ahriman.core.report.ReportTrigger triggers_known[] = ahriman.core.upload.UploadTrigger ; Maximal age in seconds of the VCS packages before their version will be updated with its remote source. diff --git a/package/share/ahriman/settings/ahriman.ini.d/00-housekeeping.ini b/package/share/ahriman/settings/ahriman.ini.d/00-housekeeping.ini new file mode 100644 index 00000000..2fdc2990 --- /dev/null +++ b/package/share/ahriman/settings/ahriman.ini.d/00-housekeeping.ini @@ -0,0 +1,3 @@ +[logs-rotation] +; Keep last build logs for each package +keep_last_logs = 5 diff --git a/src/ahriman/core/configuration/schema.py b/src/ahriman/core/configuration/schema.py index e8999fbb..57eafc23 100644 --- a/src/ahriman/core/configuration/schema.py +++ b/src/ahriman/core/configuration/schema.py @@ -45,11 +45,6 @@ CONFIGURATION_SCHEMA: ConfigurationSchema = { "path_exists": True, "path_type": "dir", }, - "keep_last_logs": { - "type": "integer", - "coerce": "integer", - "min": 0, - }, "logging": { "type": "path", "coerce": "absolute_path", diff --git a/src/ahriman/core/housekeeping/__init__.py b/src/ahriman/core/housekeeping/__init__.py new file mode 100644 index 00000000..23ffa301 --- /dev/null +++ b/src/ahriman/core/housekeeping/__init__.py @@ -0,0 +1,20 @@ +# +# Copyright (c) 2021-2025 ahriman team. +# +# This file is part of ahriman +# (see https://github.com/arcan1s/ahriman). +# +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . +# +from ahriman.core.housekeeping.logs_rotation_trigger import LogsRotationTrigger diff --git a/src/ahriman/core/housekeeping/logs_rotation_trigger.py b/src/ahriman/core/housekeeping/logs_rotation_trigger.py new file mode 100644 index 00000000..62502621 --- /dev/null +++ b/src/ahriman/core/housekeeping/logs_rotation_trigger.py @@ -0,0 +1,87 @@ +# +# Copyright (c) 2021-2025 ahriman team. +# +# This file is part of ahriman +# (see https://github.com/arcan1s/ahriman). +# +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . +# +from ahriman.core import context +from ahriman.core.configuration import Configuration +from ahriman.core.status import Client +from ahriman.core.triggers import Trigger +from ahriman.models.package import Package +from ahriman.models.repository_id import RepositoryId +from ahriman.models.result import Result + + +class LogsRotationTrigger(Trigger): + """ + rotate logs after build processes + + Attributes: + keep_last_records(int): number of last records to keep + """ + + CONFIGURATION_SCHEMA = { + "logs-rotation": { + "type": "dict", + "schema": { + "keep_last_logs": { + "type": "integer", + "required": True, + "coerce": "integer", + "min": 0, + }, + }, + }, + } + + def __init__(self, repository_id: RepositoryId, configuration: Configuration) -> None: + """ + Args: + repository_id(RepositoryId): repository unique identifier + configuration(Configuration): configuration instance + """ + Trigger.__init__(self, repository_id, configuration) + + section = next(iter(self.configuration_sections(configuration))) + self.keep_last_records = configuration.getint( # read old-style first and then fallback to new style + "settings", "keep_last_logs", + fallback=configuration.getint(section, "keep_last_logs")) + + @classmethod + def configuration_sections(cls, configuration: Configuration) -> list[str]: + """ + extract configuration sections from configuration + + Args: + configuration(Configuration): configuration instance + + Returns: + list[str]: read configuration sections belong to this trigger + """ + return list(cls.CONFIGURATION_SCHEMA.keys()) + + def on_result(self, result: Result, packages: list[Package]) -> None: + """ + run trigger + + Args: + result(Result): build result + packages(list[Package]): list of all available packages + """ + ctx = context.get() + reporter = ctx.get(Client) + reporter.logs_rotate(self.keep_last_records) diff --git a/src/ahriman/core/log/http_log_handler.py b/src/ahriman/core/log/http_log_handler.py index bb90f1a8..f7c8ede3 100644 --- a/src/ahriman/core/log/http_log_handler.py +++ b/src/ahriman/core/log/http_log_handler.py @@ -17,7 +17,6 @@ # You should have received a copy of the GNU General Public License # along with this program. If not, see . # -import atexit import logging import uuid @@ -37,7 +36,6 @@ class HttpLogHandler(logging.Handler): method Attributes: - keep_last_records(int): number of last records to keep reporter(Client): build status reporter instance suppress_errors(bool): suppress logging errors (e.g. if no web server available) """ @@ -56,7 +54,6 @@ class HttpLogHandler(logging.Handler): self.reporter = Client.load(repository_id, configuration, report=report) self.suppress_errors = suppress_errors - self.keep_last_records = configuration.getint("settings", "keep_last_logs", fallback=0) @classmethod def load(cls, repository_id: RepositoryId, configuration: Configuration, *, report: bool) -> Self: @@ -83,7 +80,6 @@ class HttpLogHandler(logging.Handler): root.addHandler(handler) LogRecordId.DEFAULT_PROCESS_ID = str(uuid.uuid4()) # assign default process identifier for log records - atexit.register(handler.rotate) return handler @@ -104,9 +100,3 @@ class HttpLogHandler(logging.Handler): if self.suppress_errors: return self.handleError(record) - - def rotate(self) -> None: - """ - rotate log records, removing older ones - """ - self.reporter.logs_rotate(self.keep_last_records) diff --git a/tests/ahriman/core/housekeeping/conftest.py b/tests/ahriman/core/housekeeping/conftest.py new file mode 100644 index 00000000..f1f3a0cb --- /dev/null +++ b/tests/ahriman/core/housekeeping/conftest.py @@ -0,0 +1,19 @@ +import pytest + +from ahriman.core.configuration import Configuration +from ahriman.core.housekeeping import LogsRotationTrigger + + +@pytest.fixture +def logs_rotation_trigger(configuration: Configuration) -> LogsRotationTrigger: + """ + logs roration trigger fixture + + Args: + configuration(Configuration): configuration fixture + + Returns: + LogsRotationTrigger: logs rotation trigger test instance + """ + _, repository_id = configuration.check_loaded() + return LogsRotationTrigger(repository_id, configuration) diff --git a/tests/ahriman/core/housekeeping/test_logs_rotation_trigger.py b/tests/ahriman/core/housekeeping/test_logs_rotation_trigger.py new file mode 100644 index 00000000..d32c04ae --- /dev/null +++ b/tests/ahriman/core/housekeeping/test_logs_rotation_trigger.py @@ -0,0 +1,26 @@ +from pytest_mock import MockerFixture +from unittest.mock import MagicMock + +from ahriman.core.configuration import Configuration +from ahriman.core.housekeeping import LogsRotationTrigger +from ahriman.core.status import Client +from ahriman.models.result import Result + + +def test_configuration_sections(configuration: Configuration) -> None: + """ + must correctly parse target list + """ + assert LogsRotationTrigger.configuration_sections(configuration) == ["logs-rotation"] + + +def test_rotate(logs_rotation_trigger: LogsRotationTrigger, mocker: MockerFixture) -> None: + """ + must rotate logs + """ + client_mock = MagicMock() + context_mock = mocker.patch("ahriman.core._Context.get", return_value=client_mock) + + logs_rotation_trigger.on_result(Result(), []) + context_mock.assert_called_once_with(Client) + client_mock.logs_rotate.assert_called_once_with(logs_rotation_trigger.keep_last_records) diff --git a/tests/ahriman/core/log/test_http_log_handler.py b/tests/ahriman/core/log/test_http_log_handler.py index 56a9ca46..715011c3 100644 --- a/tests/ahriman/core/log/test_http_log_handler.py +++ b/tests/ahriman/core/log/test_http_log_handler.py @@ -20,14 +20,12 @@ def test_load(configuration: Configuration, mocker: MockerFixture) -> None: add_mock = mocker.patch("logging.Logger.addHandler") load_mock = mocker.patch("ahriman.core.status.Client.load") - atexit_mock = mocker.patch("atexit.register") _, repository_id = configuration.check_loaded() handler = HttpLogHandler.load(repository_id, configuration, report=False) assert handler add_mock.assert_called_once_with(handler) load_mock.assert_called_once_with(repository_id, configuration, report=False) - atexit_mock.assert_called_once_with(handler.rotate) def test_load_exist(configuration: Configuration) -> None: @@ -96,16 +94,3 @@ def test_emit_skip(configuration: Configuration, log_record: logging.LogRecord, handler.emit(log_record) log_mock.assert_not_called() - - -def test_rotate(configuration: Configuration, mocker: MockerFixture) -> None: - """ - must rotate logs - """ - rotate_mock = mocker.patch("ahriman.core.status.Client.logs_rotate") - - _, repository_id = configuration.check_loaded() - handler = HttpLogHandler(repository_id, configuration, report=False, suppress_errors=False) - - handler.rotate() - rotate_mock.assert_called_once_with(handler.keep_last_records) diff --git a/tests/testresources/core/ahriman.ini b/tests/testresources/core/ahriman.ini index be6edda3..522e85dc 100644 --- a/tests/testresources/core/ahriman.ini +++ b/tests/testresources/core/ahriman.ini @@ -37,6 +37,9 @@ target = [keyring] target = keyring +[logs-rotation] +keep_last_logs = 5 + [mirrorlist] target = mirrorlist servers = http://localhost