From d1ba43c287ace39f32c36177a60fd0f8b6d51146 Mon Sep 17 00:00:00 2001 From: Vishal Doshi Date: Tue, 13 Jan 2026 12:41:03 -0500 Subject: [PATCH 1/8] Add typing to log_config module. --- modellogger/log_config.py | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/modellogger/log_config.py b/modellogger/log_config.py index ef733e6..e42fb79 100644 --- a/modellogger/log_config.py +++ b/modellogger/log_config.py @@ -1,7 +1,7 @@ import logging import sys import time -from logging import INFO +from typing import Any, Dict, Optional class DefaultFormatter(logging.Formatter): @@ -15,7 +15,7 @@ class DefaultFormatter(logging.Formatter): "RESET": "\x1b[0m", } - def __init__(self, app_name=".", include_colors=True): + def __init__(self, app_name: str = ".", include_colors: bool = True): super().__init__() self.app_name = app_name self.include_colors = include_colors @@ -23,7 +23,7 @@ def __init__(self, app_name=".", include_colors=True): "%(asctime)s - {app_name} - %(name)s - %(levelname)s - %(message)s" ) - def format(self, record): + def format(self, record: logging.LogRecord) -> str: date_format = "%Y-%m-%dT%H:%M:%SZ" log_fmt = self.base_format.format(app_name=self.app_name) @@ -36,7 +36,9 @@ def format(self, record): return formatter.format(record) -def configure_logging(app_name=".", level=INFO, log_file=None): +def configure_logging( + app_name: str = ".", level: int = logging.INFO, log_file: Optional[str] = None +) -> None: logger = logging.getLogger() logger.setLevel(level) @@ -52,13 +54,15 @@ def configure_logging(app_name=".", level=INFO, log_file=None): logger.addHandler(console_handler) -def get_logger(name): +def get_logger(name: str) -> logging.Logger: logger = logging.getLogger(name) return logger -def get_config_dict(app_name=".", log_file=None): - config = { +def get_config_dict( + app_name: str = ".", log_file: Optional[str] = None +) -> Dict[str, Any]: + config: Dict[str, Any] = { "version": 1, "disable_existing_loggers": False, "formatters": { From 797885eb47d4d76898ecc7b237f7cb3cebc6df9c Mon Sep 17 00:00:00 2001 From: Vishal Doshi Date: Tue, 13 Jan 2026 12:45:07 -0500 Subject: [PATCH 2/8] Add py.typed file. --- modellogger/py.typed | 0 1 file changed, 0 insertions(+), 0 deletions(-) create mode 100644 modellogger/py.typed diff --git a/modellogger/py.typed b/modellogger/py.typed new file mode 100644 index 0000000..e69de29 From ce43a18845b84c1f70ddc9b7501053f1dd363546 Mon Sep 17 00:00:00 2001 From: Vishal Doshi Date: Tue, 13 Jan 2026 13:02:48 -0500 Subject: [PATCH 3/8] Preserve existing handlers. --- modellogger/log_config.py | 45 +++++++++++++++++++++++++++++++++------ 1 file changed, 38 insertions(+), 7 deletions(-) diff --git a/modellogger/log_config.py b/modellogger/log_config.py index e42fb79..c0a8f07 100644 --- a/modellogger/log_config.py +++ b/modellogger/log_config.py @@ -1,4 +1,5 @@ import logging +import os import sys import time from typing import Any, Dict, Optional @@ -37,22 +38,52 @@ def format(self, record: logging.LogRecord) -> str: def configure_logging( - app_name: str = ".", level: int = logging.INFO, log_file: Optional[str] = None + app_name: str = ".", + level: int = logging.INFO, + log_file: Optional[str] = None, ) -> None: + """Configures the root logger. Preserves existing handlers (if any), but + sets their formatters to use the DefaultFormatter. + + Args: + app_name: Name of the application to include in log messages. + level: Logging level to set. + log_file: If provided, log to this file instead of stderr. + """ logger = logging.getLogger() logger.setLevel(level) - logger.handlers.clear() - - if log_file: + # check for existing matching handlers + existing_file_handler = False + existing_stream_handler = False + for handler in logger.handlers: + if ( + isinstance(handler, logging.FileHandler) + and log_file is not None + and handler.baseFilename == os.path.abspath(os.fspath(log_file)) + ): + existing_file_handler = True + elif ( + isinstance(handler, logging.StreamHandler) and handler.stream == sys.stderr + ): + existing_stream_handler = True + + if log_file and not existing_file_handler: file_handler = logging.FileHandler(log_file) - file_handler.setFormatter(DefaultFormatter(app_name, include_colors=False)) logger.addHandler(file_handler) - else: + elif not existing_stream_handler: console_handler = logging.StreamHandler(stream=sys.stderr) - console_handler.setFormatter(DefaultFormatter(app_name, include_colors=True)) logger.addHandler(console_handler) + # set formatter for all handlers + for handler in logger.handlers: + handler.setFormatter( + DefaultFormatter( + app_name=app_name, + include_colors=isinstance(handler, logging.StreamHandler), + ) + ) + def get_logger(name: str) -> logging.Logger: logger = logging.getLogger(name) From 50151c9e175a844b4284f104d60faa596d72c992 Mon Sep 17 00:00:00 2001 From: Vishal Doshi Date: Tue, 13 Jan 2026 13:34:11 -0500 Subject: [PATCH 4/8] Improve readability of fix. --- modellogger/log_config.py | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/modellogger/log_config.py b/modellogger/log_config.py index c0a8f07..f54affb 100644 --- a/modellogger/log_config.py +++ b/modellogger/log_config.py @@ -68,12 +68,14 @@ def configure_logging( ): existing_stream_handler = True - if log_file and not existing_file_handler: - file_handler = logging.FileHandler(log_file) - logger.addHandler(file_handler) - elif not existing_stream_handler: - console_handler = logging.StreamHandler(stream=sys.stderr) - logger.addHandler(console_handler) + if log_file: + if not existing_file_handler: + file_handler = logging.FileHandler(log_file) + logger.addHandler(file_handler) + else: + if not existing_stream_handler: + console_handler = logging.StreamHandler(stream=sys.stderr) + logger.addHandler(console_handler) # set formatter for all handlers for handler in logger.handlers: From f2288f9a824e20e431e8b43bc23934bc087cbba6 Mon Sep 17 00:00:00 2001 From: Vishal Doshi Date: Tue, 13 Jan 2026 13:36:48 -0500 Subject: [PATCH 5/8] Fix tests so checks the last handler added. --- tests/test_log_config.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test_log_config.py b/tests/test_log_config.py index 207878a..c99585e 100644 --- a/tests/test_log_config.py +++ b/tests/test_log_config.py @@ -50,7 +50,7 @@ def test_configure_logging(): root_logger = logging.getLogger() assert root_logger.name == "root" assert root_logger.level == logging.INFO - assert isinstance(root_logger.handlers[0], logging.StreamHandler) + assert isinstance(root_logger.handlers[-1], logging.StreamHandler) def test_configure_logging_with_file(tmp_path): @@ -60,7 +60,7 @@ def test_configure_logging_with_file(tmp_path): root_logger = logging.getLogger() assert root_logger.name == "root" assert root_logger.level == logging.INFO - assert isinstance(root_logger.handlers[0], logging.FileHandler) + assert isinstance(root_logger.handlers[-1], logging.FileHandler) def test_get_logger_basic(): From 93399b2206486d3a906b63fc0479a9e809c68076 Mon Sep 17 00:00:00 2001 From: Vishal Doshi Date: Tue, 13 Jan 2026 13:54:51 -0500 Subject: [PATCH 6/8] Less magical checking. --- modellogger/log_config.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modellogger/log_config.py b/modellogger/log_config.py index f54affb..72c4a93 100644 --- a/modellogger/log_config.py +++ b/modellogger/log_config.py @@ -60,7 +60,7 @@ def configure_logging( if ( isinstance(handler, logging.FileHandler) and log_file is not None - and handler.baseFilename == os.path.abspath(os.fspath(log_file)) + and handler.baseFilename == logging.FileHandler(log_file).baseFilename ): existing_file_handler = True elif ( From d5443009876c09fbb6cea5895f32e08b3e917fda Mon Sep 17 00:00:00 2001 From: Vishal Doshi Date: Tue, 13 Jan 2026 16:16:29 -0500 Subject: [PATCH 7/8] Simplify. --- modellogger/log_config.py | 28 +++++++--------------------- 1 file changed, 7 insertions(+), 21 deletions(-) diff --git a/modellogger/log_config.py b/modellogger/log_config.py index 72c4a93..67d6fbb 100644 --- a/modellogger/log_config.py +++ b/modellogger/log_config.py @@ -45,6 +45,9 @@ def configure_logging( """Configures the root logger. Preserves existing handlers (if any), but sets their formatters to use the DefaultFormatter. + NOTE: if you call this function multiple times, you may end up with + duplicate log messages, since each call adds new handlers to the root logger. + Args: app_name: Name of the application to include in log messages. level: Logging level to set. @@ -53,29 +56,12 @@ def configure_logging( logger = logging.getLogger() logger.setLevel(level) - # check for existing matching handlers - existing_file_handler = False - existing_stream_handler = False - for handler in logger.handlers: - if ( - isinstance(handler, logging.FileHandler) - and log_file is not None - and handler.baseFilename == logging.FileHandler(log_file).baseFilename - ): - existing_file_handler = True - elif ( - isinstance(handler, logging.StreamHandler) and handler.stream == sys.stderr - ): - existing_stream_handler = True - if log_file: - if not existing_file_handler: - file_handler = logging.FileHandler(log_file) - logger.addHandler(file_handler) + file_handler = logging.FileHandler(log_file) + logger.addHandler(file_handler) else: - if not existing_stream_handler: - console_handler = logging.StreamHandler(stream=sys.stderr) - logger.addHandler(console_handler) + console_handler = logging.StreamHandler(stream=sys.stderr) + logger.addHandler(console_handler) # set formatter for all handlers for handler in logger.handlers: From c2061c41694b1514819a729e93d9aa559b02acc8 Mon Sep 17 00:00:00 2001 From: Vishal Doshi Date: Tue, 13 Jan 2026 21:07:25 -0500 Subject: [PATCH 8/8] yagni --- modellogger/log_config.py | 15 +++------------ 1 file changed, 3 insertions(+), 12 deletions(-) diff --git a/modellogger/log_config.py b/modellogger/log_config.py index 67d6fbb..86f6852 100644 --- a/modellogger/log_config.py +++ b/modellogger/log_config.py @@ -1,5 +1,4 @@ import logging -import os import sys import time from typing import Any, Dict, Optional @@ -42,8 +41,7 @@ def configure_logging( level: int = logging.INFO, log_file: Optional[str] = None, ) -> None: - """Configures the root logger. Preserves existing handlers (if any), but - sets their formatters to use the DefaultFormatter. + """Configures the root logger. Preserves existing handlers (if any). NOTE: if you call this function multiple times, you may end up with duplicate log messages, since each call adds new handlers to the root logger. @@ -58,20 +56,13 @@ def configure_logging( if log_file: file_handler = logging.FileHandler(log_file) + file_handler.setFormatter(DefaultFormatter(app_name, include_colors=False)) logger.addHandler(file_handler) else: console_handler = logging.StreamHandler(stream=sys.stderr) + console_handler.setFormatter(DefaultFormatter(app_name, include_colors=True)) logger.addHandler(console_handler) - # set formatter for all handlers - for handler in logger.handlers: - handler.setFormatter( - DefaultFormatter( - app_name=app_name, - include_colors=isinstance(handler, logging.StreamHandler), - ) - ) - def get_logger(name: str) -> logging.Logger: logger = logging.getLogger(name)