From 14ac2e714d68645951df2692231168f915b6b8bb Mon Sep 17 00:00:00 2001 From: Aman Gupta Date: Wed, 5 Mar 2025 10:41:13 -0800 Subject: [PATCH] [BugFix] Fix logging disabling bug and add tests (#1218) SUMMARY: Fixed logging and clear loggers enabling/disabling bug. Previously, any value on the right environment variables would disable logging. Now, we explicitly check for `true` TEST PLAN: Added unit tests for enabling logging. `make test` passes --------- Signed-off-by: Aman Gupta Co-authored-by: Dipika Sikka --- src/llmcompressor/logger.py | 8 ++++---- tests/unit/test_logger.py | 13 +++++++++++++ 2 files changed, 17 insertions(+), 4 deletions(-) diff --git a/src/llmcompressor/logger.py b/src/llmcompressor/logger.py index 332daeb3d..686da7564 100644 --- a/src/llmcompressor/logger.py +++ b/src/llmcompressor/logger.py @@ -53,9 +53,9 @@ class LoggerConfig: metrics_disabled: bool = False -def configure_logger(config: Optional[LoggerConfig] = None): +def configure_logger(config: Optional[LoggerConfig] = None) -> None: """ - Configure the metrics for LLM Compressor. + Configure the logger for LLM Compressor. This function sets up the console and file logging as per the specified or default parameters. @@ -68,9 +68,9 @@ def configure_logger(config: Optional[LoggerConfig] = None): # env vars get priority if (disabled := os.getenv("LLM_COMPRESSOR_LOG_DISABLED")) is not None: - logger_config.disabled = disabled.lower() + logger_config.disabled = disabled.lower() == "true" if (clear_loggers := os.getenv("LLM_COMPRESSOR_CLEAR_LOGGERS")) is not None: - logger_config.clear_loggers = clear_loggers.lower() + logger_config.clear_loggers = clear_loggers.lower() == "true" if (console_log_level := os.getenv("LLM_COMPRESSOR_LOG_LEVEL")) is not None: logger_config.console_log_level = console_log_level.upper() if (log_file := os.getenv("LLM_COMPRESSOR_LOG_FILE")) is not None: diff --git a/tests/unit/test_logger.py b/tests/unit/test_logger.py index 0e0ac8925..1796293f7 100644 --- a/tests/unit/test_logger.py +++ b/tests/unit/test_logger.py @@ -103,3 +103,16 @@ def test_environment_variable_disable_logging(monkeypatch, capsys): captured = capsys.readouterr() assert captured.out == "" assert captured.err == "" + + +def test_environment_variable_enable_logging(monkeypatch, capsys): + # Test environment variable to enable logging + monkeypatch.setenv("LLM_COMPRESSOR_LOG_DISABLED", "false") + + configure_logger(config=LoggerConfig()) + logger.info("Info message") + logger.error("Error message") + + captured = capsys.readouterr() + assert captured.out.count("Info message") == 1 + assert captured.out.count("Error message") == 1