diff --git a/.flake8 b/.flake8 index 6deafc261..1fd489337 100644 --- a/.flake8 +++ b/.flake8 @@ -1,2 +1,2 @@ [flake8] -max-line-length = 120 +max-line-length = 160 diff --git a/moler/cmd/unix/ssh.py b/moler/cmd/unix/ssh.py index 788ee5a9a..eaa54670c 100644 --- a/moler/cmd/unix/ssh.py +++ b/moler/cmd/unix/ssh.py @@ -116,7 +116,8 @@ def __init__(self, connection, login=None, password=None, host="0", prompt=None, self._was_overridden_key_pass_keyboard = False self._permission_denied_key_pass_keyboard_cmd = None if permission_denied_key_pass_keyboard is not None: - self._permission_denied_key_pass_keyboard_cmd = permission_denied_key_pass_keyboard.format(host=host) # pylint-disable-line: consider-using-f-string + self._permission_denied_key_pass_keyboard_cmd = permission_denied_key_pass_keyboard.format( + host=host) # pylint-disable-line: consider-using-f-string def build_command_string(self) -> str: """ diff --git a/moler/config/loggers.py b/moler/config/loggers.py index 4a18b2a00..520162af3 100644 --- a/moler/config/loggers.py +++ b/moler/config/loggers.py @@ -294,19 +294,43 @@ def _is_foreign_file_handler(handler): """ Tell whether a FileHandler was not created by Moler and must not be reopened. - Other libraries (e.g. pytest, since its 9.0 logging change) attach their own - FileHandlers to Moler's non-propagating loggers. pytest's default handler points - to os.devnull ('/dev/null'); reopening it with a Moler suffix/path would yield an - unwritable path like '/dev/null.suffix' and raise PermissionError. Such handlers - are owned by the other library, so Moler should leave them untouched. + Other libraries (e.g. pytest, since its 9.0 logging change) may attach their own + FileHandlers to Moler's non-propagating loggers. Reopening such handlers under + Moler path/suffix rules may break external logging and can fail for paths that + should stay unchanged (for example os.devnull). + + Moler marks its own handlers via ``handler._moler_owned``. For backward + compatibility with already-created unmarked handlers we treat handlers under + current Moler logging path as Moler-owned as well. :param handler: logging.FileHandler to inspect. - :return: True if the handler points to os.devnull and should be skipped. + :return: True when handler is foreign and should be skipped. """ try: - return os.path.abspath(handler.baseFilename) == os.path.abspath(os.devnull) + handler_path = os.path.abspath(handler.baseFilename) + + # Never rewrite os.devnull handlers (for example pytest internal file handler). + if handler_path == os.path.abspath(os.devnull): + return True + + # Primary ownership signal for current Moler-created handlers. + if hasattr(handler, "_moler_owned"): + return not bool(handler._moler_owned) + + # Backward compatibility for unmarked handlers created in older code paths. + # Moler always writes its log files directly inside the logging path using + # flat filenames (for example "moler.log"), so an unmarked handler is treated + # as Moler-owned only when its file resides directly in that directory. A file + # located elsewhere (for example a nested pytest tmp dir) stays foreign even + # when the logging path happens to be one of its ancestors. + moler_log_path = os.path.abspath(_logging_path) + return os.path.dirname(handler_path) != moler_log_path except (AttributeError, TypeError): return False + except ValueError: + # Different path roots on some platforms (for example different drives on Windows) + # mean the handler path is outside Moler logging path. + return True def _reopen_all_logfiles_with_new_suffix(logger_suffixes, new_suffix, logger_name): @@ -467,6 +491,7 @@ def setup_new_file_handler( ) cfh.setLevel(log_level) cfh.setFormatter(formatter) + cfh._moler_owned = True if log_filter: cfh.addFilter(log_filter) logger.addHandler(cfh) @@ -862,6 +887,7 @@ class RawFileHandler(logging.FileHandler): def __init__(self, *args, **kwargs): """RawFileHandler must use RawDataFormatter and level == RAW_DATA only""" super(RawFileHandler, self).__init__(*args, **kwargs) + self._moler_owned = True raw_formatter = RawDataFormatter() self.setFormatter(raw_formatter) self.setLevel(RAW_DATA) diff --git a/test/test_loggers.py b/test/test_loggers.py index 8e7536f40..a28498dee 100644 --- a/test/test_loggers.py +++ b/test/test_loggers.py @@ -330,3 +330,72 @@ def test_loggers_suffix(): assert all(value is None for value in moler.config.loggers._logging_suffixes.values()) change_logging_suffix(None) assert all(value is None for value in moler.config.loggers._logging_suffixes.values()) + + +def test_setup_new_file_handler_marks_handler_as_moler_owned(tmp_path): + import moler.config.loggers as m_logger + + logger_name = "marker_logger" + logger = logging.getLogger(logger_name) + logger.handlers = [] + log_path = tmp_path / "owned.log" + + handler = m_logger.setup_new_file_handler( + logger_name=logger_name, + log_level=logging.INFO, + log_filename=str(log_path), + formatter=logging.Formatter("%(message)s"), + ) + + try: + assert getattr(handler, "_moler_owned", False) is True + finally: + handler.close() + logger.handlers = [] + + +def test_reopen_with_new_suffix_skips_foreign_file_handler(tmp_path, monkeypatch): + import moler.config.loggers as m_logger + + logger_name = "foreign_suffix_logger" + logger = logging.getLogger(logger_name) + logger.handlers = [] + + foreign_path = tmp_path / "foreign.log" + handler = logging.FileHandler(filename=str(foreign_path)) + logger.addHandler(handler) + monkeypatch.setattr(m_logger, "active_loggers", {logger_name}) + + try: + m_logger._reopen_all_logfiles_with_new_suffix( # pylint: disable=protected-access + logger_suffixes={logger_name: None}, + new_suffix=".suffix", + logger_name=logger_name, + ) + assert handler.baseFilename == str(foreign_path) + finally: + handler.close() + logger.handlers = [] + + +def test_reconfigure_logging_path_skips_foreign_file_handler(tmp_path, monkeypatch): + import moler.config.loggers as m_logger + + logger_name = "foreign_path_logger" + logger = logging.getLogger(logger_name) + logger.handlers = [] + + foreign_path = tmp_path / "foreign.log" + handler = logging.FileHandler(filename=str(foreign_path)) + logger.addHandler(handler) + monkeypatch.setattr(m_logger, "active_loggers", {logger_name}) + + try: + m_logger._reopen_all_logfiles_in_new_path( # pylint: disable=protected-access + old_logging_path="/old/moler/path", + new_logging_path="/new/moler/path", + ) + assert handler.baseFilename == str(foreign_path) + finally: + handler.close() + logger.handlers = []