From a0189f3b27e80339f906907303a8c60b395ed930 Mon Sep 17 00:00:00 2001 From: Mayyhem Date: Thu, 21 May 2026 09:29:54 -0400 Subject: [PATCH] Close second Windows file handle blocking openhound.log rename after rollover --- src/openhound/core/logging.py | 21 ++++++++++++------- tests/test_log_handlers.py | 38 +++++++++++++++++++++++------------ 2 files changed, 39 insertions(+), 20 deletions(-) diff --git a/src/openhound/core/logging.py b/src/openhound/core/logging.py index a11e253..896702d 100644 --- a/src/openhound/core/logging.py +++ b/src/openhound/core/logging.py @@ -229,6 +229,13 @@ def _is_valid_path(base_path: Path, spec_path: Path) -> Path: ) return resolved_spec_path + @staticmethod + def _close_handlers(logger: logging.Logger) -> None: + """Remove and close all handlers attached directly to a logger.""" + for handler in logger.handlers[:]: + logger.removeHandler(handler) + handler.close() + @staticmethod def default_platform_path() -> Path: """Get the default log path based on platform @@ -260,7 +267,7 @@ def set_handler(self, name: str) -> None: log_file_path = self.base_path / f"ext_{name}.log" valid_path = self._is_valid_path(self.base_path, log_file_path) self.dlt_logger.setLevel(self.root_logger.level) - self.dlt_logger.handlers.clear() + self._close_handlers(self.dlt_logger) self.handlers[self.runtime_mode](self.dlt_logger, valid_path) self.dlt_logger.propagate = False @@ -293,14 +300,14 @@ def setup(self) -> None: self.backup_count = dlt_backup_count if dlt_backup_count else self.backup_count self.interval = dlt_interval if dlt_interval else self.interval - # Clear the default DLT log handlers and set our own handlers based on the runtime mode - self.dlt_logger.handlers.clear() - self.handlers[self.runtime_mode](self.dlt_logger, self.log_file_path) - self.dlt_logger.propagate = False + # The root logger owns the default OpenHound file handler. DLT logs propagate + # there until set_handler routes them to an extension-specific log file. + self._close_handlers(self.dlt_logger) + self.dlt_logger.setLevel(getattr(logging, self.level)) + self.dlt_logger.propagate = True - # Set the root logger level and handlers based on the runtime mode self.root_logger.setLevel(getattr(logging, self.level)) - self.root_logger.handlers.clear() + self._close_handlers(self.root_logger) self.handlers[self.runtime_mode](self.root_logger, self.log_file_path) def container_handlers(self, logger: logging.Logger, file_path: Path) -> None: diff --git a/tests/test_log_handlers.py b/tests/test_log_handlers.py index 01f59a8..737e212 100644 --- a/tests/test_log_handlers.py +++ b/tests/test_log_handlers.py @@ -4,6 +4,14 @@ from openhound.core.logging import RotatingFileHandler, logger_override +def _rotating_handlers(logger: logging.Logger) -> list[RotatingFileHandler]: + return [ + handler + for handler in logger.handlers + if isinstance(handler, RotatingFileHandler) + ] + + def test_root_handler_setup(): """Test that the root logger has a handler configured and that it is a RotatingFileHandler""" root_logger = logging.getLogger() @@ -11,31 +19,35 @@ def test_root_handler_setup(): "The root logger should have at least one handler configured" ) - base_handler = root_logger.handlers[0] - assert isinstance(base_handler, RotatingFileHandler), ( + rotating_handlers = _rotating_handlers(root_logger) + assert len(rotating_handlers) == 1, ( "The root logger should use RotatingFileHandler" ) + base_handler = rotating_handlers[0] assert base_handler.baseFilename.endswith("openhound.log"), ( "The root logger should log to 'openhound.log'" ) -def test_dlt_handler_setup(): - """Test that the default DLT handler is overwritten by our RotatingFileHandler""" +def test_dlt_handler_setup(tmp_path): + """Test that default DLT logs propagate to the root OpenHound handler.""" + logger_override.base_path = tmp_path + logger_override.setup() + dlt_logger = logging.getLogger("dlt") - assert len(dlt_logger.handlers) > 0, ( - "The default DLT logger should have at least one handler configured" + assert len(dlt_logger.handlers) == 0, ( + "The default DLT logger should not own a separate file handler" ) - assert dlt_logger.propagate is False, ( - "The DLT logger should have propagation disabled to prevent duplicate logs in the root logger" + assert dlt_logger.propagate is True, ( + "The DLT logger should propagate to the root OpenHound handler" ) - base_handler = dlt_logger.handlers[0] - assert isinstance(base_handler, RotatingFileHandler), ( - "The default DLT logger should use RotatingFileHandler" + root_handlers = _rotating_handlers(logging.getLogger()) + assert len(root_handlers) == 1, ( + "Only the root logger should own the default OpenHound file handler" ) - assert base_handler.baseFilename.endswith("openhound.log"), ( - "The default DLT logger should log to 'openhound.log'" + assert root_handlers[0].baseFilename.endswith("openhound.log"), ( + "Default DLT logs should flow to 'openhound.log'" )