diff --git a/src/kleinanzeigen_bot/__init__.py b/src/kleinanzeigen_bot/__init__.py index 6ae7511..e303a3e 100644 --- a/src/kleinanzeigen_bot/__init__.py +++ b/src/kleinanzeigen_bot/__init__.py @@ -3,14 +3,13 @@ SPDX-FileCopyrightText: © Sebastian Thomschke and contributors SPDX-License-Identifier: AGPL-3.0-or-later SPDX-ArtifactOfProjectHomePage: https://github.com/Second-Hand-Friends/kleinanzeigen-bot/ """ -import asyncio, atexit, copy, importlib.metadata, json, logging, os, re, signal, shutil, sys, textwrap, time +import asyncio, atexit, copy, importlib.metadata, json, os, re, signal, shutil, sys, textwrap, time import getopt # pylint: disable=deprecated-module import urllib.parse as urllib_parse import urllib.request as urllib_request from collections.abc import Iterable from datetime import datetime from gettext import gettext as _ -from logging.handlers import RotatingFileHandler from typing import Any, Final import certifi, colorama, nodriver @@ -28,8 +27,8 @@ from ._version import __version__ # W0406: possibly a bug, see https://github.com/PyCQA/pylint/issues/3933 -LOG:Final[logging.Logger] = loggers.get_logger(__name__) -LOG.setLevel(logging.INFO) +LOG:Final[loggers.Logger] = loggers.get_logger(__name__) +LOG.setLevel(loggers.INFO) colorama.just_fix_windows_console() @@ -51,7 +50,7 @@ class KleinanzeigenBot(WebScrapingMixin): self.categories:dict[str, str] = {} - self.file_log:logging.FileHandler | None = None + self.file_log:loggers.LogFileHandle | None = None log_file_basename = is_frozen() and os.path.splitext(os.path.basename(sys.executable))[0] or self.__module__ self.log_file_path:str | None = abspath(f"{log_file_basename}.log") @@ -61,9 +60,8 @@ class KleinanzeigenBot(WebScrapingMixin): def __del__(self) -> None: if self.file_log: - self.file_log.flush() - loggers.LOG_ROOT.removeHandler(self.file_log) self.file_log.close() + self.file_log = None self.close_browser_session() def get_version(self) -> str: @@ -239,8 +237,8 @@ class KleinanzeigenBot(WebScrapingMixin): case "--lang": set_current_locale(Locale.of(value)) case "-v" | "--verbose": - LOG.setLevel(logging.DEBUG) - logging.getLogger("nodriver").setLevel(logging.INFO) + LOG.setLevel(loggers.DEBUG) + loggers.get_logger("nodriver").setLevel(loggers.INFO) match len(arguments): case 0: @@ -258,10 +256,7 @@ class KleinanzeigenBot(WebScrapingMixin): return LOG.info("Logging to [%s]...", self.log_file_path) - self.file_log = RotatingFileHandler(filename = self.log_file_path, maxBytes = 10 * 1024 * 1024, backupCount = 10, encoding = "utf-8") - self.file_log.setLevel(logging.DEBUG) - self.file_log.setFormatter(logging.Formatter("%(asctime)s [%(levelname)s] %(message)s")) - loggers.LOG_ROOT.addHandler(self.file_log) + self.file_log = loggers.configure_file_logging(self.log_file_path) LOG.info("App version: %s", self.get_version()) LOG.info("Python version: %s", sys.version) @@ -611,7 +606,7 @@ class KleinanzeigenBot(WebScrapingMixin): LOG.info("Publishing ad '%s'...", ad_cfg["title"]) - if LOG.isEnabledFor(logging.DEBUG): + if loggers.is_debug(LOG): LOG.debug(" -> effective ad meta:") YAML().dump(ad_cfg, sys.stdout) diff --git a/src/kleinanzeigen_bot/utils/loggers.py b/src/kleinanzeigen_bot/utils/loggers.py index 9171e32..43c5fc5 100644 --- a/src/kleinanzeigen_bot/utils/loggers.py +++ b/src/kleinanzeigen_bot/utils/loggers.py @@ -5,6 +5,7 @@ SPDX-ArtifactOfProjectHomePage: https://github.com/Second-Hand-Friends/kleinanze """ import copy, logging, re, sys from gettext import gettext as _ +from logging.handlers import RotatingFileHandler from typing import Any, Final # @UnusedImport import colorama @@ -12,12 +13,14 @@ from . import i18n, reflect __all__ = [ "Logger", - "LOG_ROOT", + "LogFileHandle", "DEBUG", "INFO", "configure_console_logging", + "configure_file_logging", "flush_all_handlers", - "get_logger" + "get_logger", + "is_debug" ] Logger = logging.Logger @@ -91,6 +94,46 @@ def configure_console_logging() -> None: LOG_ROOT.addHandler(stderr_log) +class LogFileHandle: + """Encapsulates a log file handler with close and status methods.""" + + def __init__(self, file_path: str, handler: RotatingFileHandler, logger: logging.Logger): + self.file_path = file_path + self._handler:RotatingFileHandler | None = handler + self._logger = logger + + def close(self) -> None: + """Flushes, removes, and closes the log handler.""" + if self._handler: + self._handler.flush() + self._logger.removeHandler(self._handler) + self._handler.close() + self._handler = None + + def is_closed(self) -> bool: + """Returns whether the log handler has been closed.""" + return not self._handler + + +def configure_file_logging(log_file_path:str) -> LogFileHandle: + """ + Sets up a file logger and returns a callable to flush, remove, and close it. + + @param log_file_path: Path to the log file. + @return: Callable[[], None]: A function that cleans up the log handler. + """ + fh = RotatingFileHandler( + filename = log_file_path, + maxBytes = 10 * 1024 * 1024, # 10 MB + backupCount = 10, + encoding = "utf-8" + ) + fh.setLevel(logging.DEBUG) + fh.setFormatter(logging.Formatter("%(asctime)s [%(levelname)s] %(message)s")) + LOG_ROOT.addHandler(fh) + return LogFileHandle(log_file_path, fh, LOG_ROOT) + + def flush_all_handlers() -> None: for handler in LOG_ROOT.handlers: handler.flush() diff --git a/tests/unit/test_bot.py b/tests/unit/test_bot.py index 79f1809..cee09fc 100644 --- a/tests/unit/test_bot.py +++ b/tests/unit/test_bot.py @@ -41,10 +41,11 @@ class TestKleinanzeigenBot: bot.configure_file_logging() file_log = bot.file_log - assert not file_log.stream.closed # type: ignore[union-attr] + assert file_log is not None + assert not file_log.is_closed() # Delete and garbage collect the bot instance to ensure the destructor (__del__) is called del bot gc.collect() - assert file_log.stream is None # type: ignore[union-attr] + assert file_log.is_closed() diff --git a/tests/unit/test_init.py b/tests/unit/test_init.py index 93e4896..2a5e04d 100644 --- a/tests/unit/test_init.py +++ b/tests/unit/test_init.py @@ -3,7 +3,7 @@ SPDX-FileCopyrightText: © Jens Bergmann and contributors SPDX-License-Identifier: AGPL-3.0-or-later SPDX-ArtifactOfProjectHomePage: https://github.com/Second-Hand-Friends/kleinanzeigen-bot/ """ -import copy, logging, os, tempfile +import copy, os, tempfile from collections.abc import Generator from datetime import datetime, timedelta from pathlib import Path @@ -16,6 +16,7 @@ from ruamel.yaml import YAML from kleinanzeigen_bot import LOG, KleinanzeigenBot from kleinanzeigen_bot._version import __version__ from kleinanzeigen_bot.ads import calculate_content_hash +from kleinanzeigen_bot.utils import loggers @pytest.fixture @@ -224,7 +225,7 @@ class TestKleinanzeigenBotCommandLine: def test_parse_args_handles_verbose_flag(self, test_bot: KleinanzeigenBot) -> None: """Verify that verbose flag sets correct log level.""" test_bot.parse_args(["dummy", "--verbose"]) - assert LOG.level == logging.DEBUG + assert loggers.is_debug(LOG) def test_parse_args_handles_config_path(self, test_bot: KleinanzeigenBot, test_data_dir: str) -> None: """Verify that config path is set correctly.""" @@ -432,10 +433,10 @@ class TestKleinanzeigenBotBasics: def test_get_log_level(self, test_bot: KleinanzeigenBot) -> None: """Test log level configuration.""" # Reset log level to default - LOG.setLevel(logging.INFO) - assert LOG.level == logging.INFO + LOG.setLevel(loggers.INFO) + assert not loggers.is_debug(LOG) test_bot.parse_args(['script.py', '-v']) - assert LOG.level == logging.DEBUG + assert loggers.is_debug(LOG) def test_get_config_file_path(self, test_bot: KleinanzeigenBot) -> None: """Test config file path handling.""" @@ -476,7 +477,7 @@ class TestKleinanzeigenBotArgParsing: def test_parse_args_verbose(self, test_bot: KleinanzeigenBot) -> None: """Test parsing verbose flag.""" test_bot.parse_args(['script.py', '-v', 'help']) - assert logging.getLogger('kleinanzeigen_bot').level == logging.DEBUG + assert loggers.is_debug(loggers.get_logger('kleinanzeigen_bot')) def test_parse_args_config_path(self, test_bot: KleinanzeigenBot) -> None: """Test parsing config path."""