From 00ea4ac06092d8a05cf9346b873d1f8b16109089 Mon Sep 17 00:00:00 2001 From: Andrew Snare Date: Thu, 15 May 2025 14:41:14 +0200 Subject: [PATCH 01/10] Remove unnnecessry linter suppression. --- src/databricks/labs/blueprint/logger.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/databricks/labs/blueprint/logger.py b/src/databricks/labs/blueprint/logger.py index 2c734b6..6ec1a13 100644 --- a/src/databricks/labs/blueprint/logger.py +++ b/src/databricks/labs/blueprint/logger.py @@ -37,7 +37,7 @@ def _bold(self, text): """Return text in bold.""" return f"{self.BOLD}{text}{self.RESET}" - def format(self, record: logging.LogRecord): # noqa: A003 + def format(self, record: logging.LogRecord): """Format the log record. If colors are enabled, use them.""" if not self.colors: return super().format(record) From aabb9765e16839cea5f563c8f3f4e3a0907e1a1c Mon Sep 17 00:00:00 2001 From: Andrew Snare Date: Thu, 15 May 2025 14:43:45 +0200 Subject: [PATCH 02/10] Type hints for return values. --- src/databricks/labs/blueprint/logger.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/databricks/labs/blueprint/logger.py b/src/databricks/labs/blueprint/logger.py index 6ec1a13..3d66aa1 100644 --- a/src/databricks/labs/blueprint/logger.py +++ b/src/databricks/labs/blueprint/logger.py @@ -33,11 +33,11 @@ def __init__(self, *, probe_tty: bool = False) -> None: # show colors in runtime, github actions, and while debugging self.colors = sys.stdout.isatty() if probe_tty else True - def _bold(self, text): + def _bold(self, text: str) -> str: """Return text in bold.""" return f"{self.BOLD}{text}{self.RESET}" - def format(self, record: logging.LogRecord): + def format(self, record: logging.LogRecord) -> str: """Format the log record. If colors are enabled, use them.""" if not self.colors: return super().format(record) @@ -68,7 +68,7 @@ def format(self, record: logging.LogRecord): return f"{self.GRAY}{timestamp}{self.RESET} {level} {color_marker}[{name}]{thread_name} {msg}{self.RESET}" -def install_logger(level="DEBUG"): +def install_logger(level="DEBUG") -> logging.StreamHandler: """Install a console logger with a nice formatter.""" for handler in logging.root.handlers: logging.root.removeHandler(handler) From a80894a6f34d06e1aede2db283bebf94e5e129d4 Mon Sep 17 00:00:00 2001 From: Andrew Snare Date: Thu, 15 May 2025 14:45:06 +0200 Subject: [PATCH 03/10] Type hint for instance attribute. --- src/databricks/labs/blueprint/logger.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/databricks/labs/blueprint/logger.py b/src/databricks/labs/blueprint/logger.py index 3d66aa1..6595cda 100644 --- a/src/databricks/labs/blueprint/logger.py +++ b/src/databricks/labs/blueprint/logger.py @@ -17,6 +17,9 @@ class NiceFormatter(logging.Formatter): MAGENTA = "\033[35m" GRAY = "\033[90m" + colors: bool + """Whether this formatter is formatting with colors or not.""" + def __init__(self, *, probe_tty: bool = False) -> None: """Create a new instance of the formatter. If probe_tty is True, then the formatter will attempt to detect if the console supports colors. If probe_tty is False, colors will be From 09d36e0c16ce8dbdcd0b62844a98636effebff37 Mon Sep 17 00:00:00 2001 From: Andrew Snare Date: Thu, 15 May 2025 14:46:52 +0200 Subject: [PATCH 04/10] Allow the stream to-be-probed to be passed in as an argument. --- src/databricks/labs/blueprint/logger.py | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/src/databricks/labs/blueprint/logger.py b/src/databricks/labs/blueprint/logger.py index 6595cda..54ed65a 100644 --- a/src/databricks/labs/blueprint/logger.py +++ b/src/databricks/labs/blueprint/logger.py @@ -2,6 +2,7 @@ import logging import sys +from typing import TextIO class NiceFormatter(logging.Formatter): @@ -20,10 +21,13 @@ class NiceFormatter(logging.Formatter): colors: bool """Whether this formatter is formatting with colors or not.""" - def __init__(self, *, probe_tty: bool = False) -> None: - """Create a new instance of the formatter. If probe_tty is True, then the formatter will - attempt to detect if the console supports colors. If probe_tty is False, colors will be - enabled by default.""" + def __init__(self, *, probe_tty: bool = False, stream: TextIO = sys.stdout) -> None: + """Create a new instance of the formatter. + + Args: + stream: the output stream to which the formatter will write, used to check if it is a console. + probe_tty: If true, the formatter will enable color support if the output stream appears to be a console. + """ super().__init__(fmt="%(asctime)s %(levelname)s [%(name)s] %(message)s", datefmt="%H:%M") self._levels = { logging.NOTSET: self._bold("TRACE"), @@ -34,7 +38,7 @@ def __init__(self, *, probe_tty: bool = False) -> None: logging.CRITICAL: self._bold(f"{self.MAGENTA}FATAL"), } # show colors in runtime, github actions, and while debugging - self.colors = sys.stdout.isatty() if probe_tty else True + self.colors = stream.isatty() if probe_tty else True def _bold(self, text: str) -> str: """Return text in bold.""" From 6dc364ad7e1ec951175fda4d23dc06164195eb0a Mon Sep 17 00:00:00 2001 From: Andrew Snare Date: Thu, 15 May 2025 14:49:24 +0200 Subject: [PATCH 05/10] Mark in the code that the internal documentation doesn't seem to match the behaviour. --- src/databricks/labs/blueprint/logger.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/databricks/labs/blueprint/logger.py b/src/databricks/labs/blueprint/logger.py index 54ed65a..8530616 100644 --- a/src/databricks/labs/blueprint/logger.py +++ b/src/databricks/labs/blueprint/logger.py @@ -8,6 +8,8 @@ class NiceFormatter(logging.Formatter): """A nice formatter for logging. It uses colors and bold text if the console supports it.""" + # TODO: Actually detect if the console supports colors. Currently, it just assumes that it does. + BOLD = "\033[1m" RESET = "\033[0m" GREEN = "\033[32m" From 58110059a256e6dfb4d79ce17abb0bbeae1c243c Mon Sep 17 00:00:00 2001 From: Andrew Snare Date: Thu, 15 May 2025 14:49:54 +0200 Subject: [PATCH 06/10] Update install_logger() so that it can be tested. In addition, pass through stderr as the stream for the nice formatter to probe because that's where output goes, and it might be different to stdout. --- src/databricks/labs/blueprint/logger.py | 32 +++++++++++++++++++------ 1 file changed, 25 insertions(+), 7 deletions(-) diff --git a/src/databricks/labs/blueprint/logger.py b/src/databricks/labs/blueprint/logger.py index 8530616..ae1485a 100644 --- a/src/databricks/labs/blueprint/logger.py +++ b/src/databricks/labs/blueprint/logger.py @@ -77,12 +77,30 @@ def format(self, record: logging.LogRecord) -> str: return f"{self.GRAY}{timestamp}{self.RESET} {level} {color_marker}[{name}]{thread_name} {msg}{self.RESET}" -def install_logger(level="DEBUG") -> logging.StreamHandler: - """Install a console logger with a nice formatter.""" - for handler in logging.root.handlers: - logging.root.removeHandler(handler) - console_handler = logging.StreamHandler(sys.stderr) - console_handler.setFormatter(NiceFormatter()) +def install_logger( + level: int | str = logging.DEBUG, *, stream: TextIO = sys.stderr, root: logging.Logger = logging.root +) -> logging.StreamHandler: + """Install a console logger with a nice formatter. + + The root logger will be modified: + + - Its logging level will be left as-is. + - All existing handlers will be removed. + - A new handler will be installed with our custom formatter. It will be configured to emit logs at the given level + (default: DEBUG) or higher, to the specified stream (default: sys.stderr). + + Args: + level: The logging level to set for the console handler. + stream: The stream to which the logger will write. Defaults to sys.stderr. + root: The root logger to modify. Defaults to the system root logger. (Mainly useful in tests.) + + Returns: + The logging handler that was installed. + """ + for handler in root.handlers: + root.removeHandler(handler) + console_handler = logging.StreamHandler(stream) + console_handler.setFormatter(NiceFormatter(stream=stream)) console_handler.setLevel(level) - logging.root.addHandler(console_handler) + root.addHandler(console_handler) return console_handler From 7a5ed7bca19bc2fb0ae968141a2b10cd73a75129 Mon Sep 17 00:00:00 2001 From: Andrew Snare Date: Thu, 15 May 2025 14:54:19 +0200 Subject: [PATCH 07/10] Verify what install_logger() does, and update existing logger tests to confirm they work. --- tests/unit/test_logger.py | 103 ++++++++++++++++++++++++++++++++++++-- 1 file changed, 100 insertions(+), 3 deletions(-) diff --git a/tests/unit/test_logger.py b/tests/unit/test_logger.py index c0cc261..b2b62eb 100644 --- a/tests/unit/test_logger.py +++ b/tests/unit/test_logger.py @@ -1,10 +1,107 @@ +from __future__ import annotations + +import io import logging +from collections.abc import Generator +from contextlib import contextmanager + +import pytest + +from databricks.labs.blueprint.logger import NiceFormatter, install_logger + + +class LogCaptureHandler(logging.Handler): + """Custom logging handler to capture log records.""" + + records: list[logging.LogRecord] + + def __init__(self) -> None: + super().__init__() + self.records = [] + + def emit(self, record: logging.LogRecord) -> None: + """Capture the log record.""" + self.records.append(record) + + @classmethod + @contextmanager + def record_capturing(cls, logger: logging.Logger) -> Generator[LogCaptureHandler, None, None]: + """Temporarily capture all log records, in addition to existing handling.""" + handler = LogCaptureHandler() + logger.addHandler(handler) + try: + yield handler + finally: + logger.removeHandler(handler) + + +class LoggingSystemFixture: + """A logging system, independent of the system logger.""" + + output_buffer: io.StringIO + root: logging.RootLogger + manager: logging.Manager + + def __init__(self) -> None: + self.output_buffer = io.StringIO() + self.root = logging.RootLogger(logging.WARNING) + self.root.addHandler(logging.StreamHandler(self.output_buffer)) + self.manager = logging.Manager(self.root) + def getLogger(self, name: str) -> logging.Logger: + """Get a logger that is part of this logging system.""" + return self.manager.getLogger(name) -def test_logger(): - logger = logging.getLogger(__name__) + def text(self) -> str: + """Get the formatted text that has been logged by this system so far.""" + return self.output_buffer.getvalue() + + +@pytest.fixture +def logging_system() -> LoggingSystemFixture: + """Fixture to provide a logging system independent of the system logger.""" + return LoggingSystemFixture() + + +def test_install_logger(logging_system) -> None: + """Test installing the logger. + + This involves verifying that: + + - The existing handlers on the root logger are replaced with a new handler, and it uses the nice formatter. + - The handler log-level is set, but the root is left as-is. + """ + root = logging_system.root + root.setLevel(logging.FATAL) + + # Install the logger and log some things. + handler = install_logger(logging.INFO, root=root, stream=logging_system.output_buffer) + + # Verify that the root logger was configured as expected. + assert root.level == logging.FATAL # remains unchanged + assert root.handlers == [handler] + assert handler.level == logging.INFO + assert isinstance(handler.formatter, NiceFormatter) + + +def test_installed_logger_logging(logging_system) -> None: + """Test that logging basics work with the installed logger.""" + root = logging_system.root + root.setLevel(logging.DEBUG) + install_logger(stream=logging_system.output_buffer, root=root) + + # Log some messages. + logger = logging_system.getLogger(__file__) logger.debug("This is a debug message") - logger.info("This is an table message") + logger.info("This is an info message") logger.warning("This is a warning message") logger.error("This is an error message", exc_info=KeyError(123)) logger.critical("This is a critical message") + + # Verify the messages were logged correctly. + output = logging_system.text() + assert "This is a debug message" in output + assert "This is an info message" in output + assert "This is a warning message" in output + assert "This is an error message: KeyError: 123" in output + assert "This is a critical message" in output From 78c2633ce76ff4b5173c809fd566a8f17557280d Mon Sep 17 00:00:00 2001 From: Andrew Snare Date: Thu, 15 May 2025 14:55:32 +0200 Subject: [PATCH 08/10] Unit tests for the customized logging formatter (NiceFormatter) that we install. This includes several tests marked as expected failures due to issues in the current implementation of the formatter. --- tests/unit/test_logger.py | 296 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 296 insertions(+) diff --git a/tests/unit/test_logger.py b/tests/unit/test_logger.py index b2b62eb..a4c7e88 100644 --- a/tests/unit/test_logger.py +++ b/tests/unit/test_logger.py @@ -1,8 +1,12 @@ from __future__ import annotations +import datetime as dt +import inspect import io import logging +import re from collections.abc import Generator +from concurrent.futures import ThreadPoolExecutor from contextlib import contextmanager import pytest @@ -105,3 +109,295 @@ def test_installed_logger_logging(logging_system) -> None: assert "This is a warning message" in output assert "This is an error message: KeyError: 123" in output assert "This is a critical message" in output + + +# Regex that matches the SGR escape sequence to set text attributes (including colors) on terminals/consoles. SGR is: +# CSI Ps [; ... ; Ps] m +# Where: +# - CSI: Control Sequence Introducer, ESC + '[' +# - Ps: A number, indicating the attribute to set, 0 to reset. A sequence is allowed, separated by ';'. +# - m: A literal 'm' character (which indicates the end of SGR sequence). +# Examples: +# - '\x1b[0m' (reset) +# - '\x1b[1m' (bold) +# - '\x1b[1;31m' (bold red) +# - '\x1b[31;1m' (also bold red) +# These are often referred to as ANSI escape codes. +_SGR_ESCAPE_SEQ = re.compile(r"\x1b\[[\d;]+m") + + +def _strip_sgr_sequences(text: str) -> str: + """Strip SGR escape sequences from the text.""" + return _SGR_ESCAPE_SEQ.sub("", text) + + +# Call signature matches logger.log(), except we return the record. +def create_record(level: int, msg: str, *args, name: str = __name__, **kwargs) -> logging.LogRecord: + """Create a log record with the given level and message.""" + logger = logging.getLogger(name) + + # Capture existing configuration. + old_handlers = tuple(logger.handlers) + old_propagate = logger.propagate + old_level = logger.level + + try: + # Ensure the logger actually emits the record to its handler, but doesn't propagate to its parent. + logger.setLevel(logging.DEBUG) + logger.propagate = False + for handler in old_handlers: + logger.removeHandler(handler) + + with LogCaptureHandler.record_capturing(logger) as capture_handler: + # Create the log record. + logger.log(level, msg, *args, **kwargs) + + # Return the captured log record. + records = capture_handler.records + assert records + produced_record = capture_handler.records.pop() + return produced_record + finally: + # Restore the logger configuration. + logger.setLevel(old_level) + for handler in old_handlers: + logger.addHandler(handler) + logger.propagate = old_propagate + + +def test_formatter_color_if_enabled() -> None: + """Ensure the formatter includes color codes if colors are enabled.""" + formatter = NiceFormatter() + formatter.colors = True + + record = create_record(logging.DEBUG, "Arbitrary message.") + formatted = formatter.format(record) + stripped = _strip_sgr_sequences(formatted) + + assert stripped != formatted + + +def test_formatter_skips_colors() -> None: + """Ensure the formatter does not include color codes if colors are disabled.""" + formatter = NiceFormatter() + formatter.colors = False + + record = create_record(logging.DEBUG, "Arbitrary message.") + formatted = formatter.format(record) + stripped = _strip_sgr_sequences(formatted) + + assert stripped == formatted + + +@pytest.mark.parametrize("use_colors", (True, False), ids=("with_colors", "without_colors")) +def test_formatter_format_simple_msg(use_colors: bool) -> None: + """Ensure the formatter formats a simple message correctly.""" + formatter = NiceFormatter() + formatter.colors = use_colors + + record = create_record(logging.DEBUG, "This is a test message.") + formatted = formatter.format(record) + stripped = _strip_sgr_sequences(formatted) if use_colors else formatted + + # H:M:S LEVEL [logger_name] message + assert stripped.endswith(" This is a test message.") + + +@pytest.mark.parametrize( + "use_colors", + ( + pytest.param( + True, marks=pytest.mark.xfail(reason="Argument interpolation when colorizing doesn't work.", strict=True) + ), + False, + ), + ids=("with_colors", "without_colors"), +) +def test_formatter_format_msg_with_args(use_colors: bool) -> None: + """Ensure the formatter correctly formats a message with arguments that need to be interpolated.""" + formatter = NiceFormatter() + formatter.colors = use_colors + + record = create_record(logging.DEBUG, "This is a %s message with %d arguments.", "test", 2) + formatted = formatter.format(record) + stripped = _strip_sgr_sequences(formatted) if use_colors else formatted + + # H:M:S LEVEL [logger_name] message + assert stripped.endswith(" This is a test message with 2 arguments.") + + +@pytest.mark.parametrize( + "use_colors", + ( + True, + pytest.param( + False, + marks=pytest.mark.xfail(reason="Non-colorized logs currently missing second-granularity.", strict=True), + ), + ), + ids=["with_colors", "without_colors"], +) +def test_formatter_timestamp(use_colors: bool) -> None: + """Ensure the formatter starts with the timestamp.""" + formatter = NiceFormatter() + formatter.colors = use_colors + + record = create_record(logging.DEBUG, "Whatever") + + formatted = formatter.format(record) + + # Deliberately naive: we want the local time rather than UTC. + record_timestamp = dt.datetime.fromtimestamp(record.created, tz=None) + stripped = _strip_sgr_sequences(formatted) if use_colors else formatted + + # H:M:S LEVEL [logger_name] message + formatted_timestamp = record_timestamp.strftime("%H:%M:%S") + assert stripped.startswith(f"{formatted_timestamp} ") + + +@pytest.mark.parametrize( + "level", + (logging.DEBUG, logging.INFO, logging.WARNING, logging.ERROR, logging.CRITICAL), + ids=lambda level: logging.getLevelName(level), +) +@pytest.mark.parametrize("use_colors", (True, False), ids=("with_colors", "without_colors")) +def test_formatter_format_log_level(level: int, use_colors: bool) -> None: + """Ensure the formatter formats a simple message correctly.""" + formatter = NiceFormatter() + formatter.colors = use_colors + + # Create a log record with the specified level. + record = create_record(level, "Whatever") + formatted = formatter.format(record) + stripped = _strip_sgr_sequences(formatted) if use_colors else formatted + + # Can't mark combinations of parameters for xfail, so we simulate it here. + expected_failure = use_colors and level in (logging.WARNING, logging.CRITICAL) + try: + # H:M:S LEVEL [logger_name] message + assert f" {logging.getLevelName(level)} " in stripped + if expected_failure: + msg = ( + f"Unexpected success: colorized log-level for {logging.getLevelName(level)} is thought to be incorrect." + ) + pytest.fail(msg) + except AssertionError: + if not expected_failure: + raise + pytest.xfail(f"Colorized log-level formatting for {logging.getLevelName(level)} is known to be incorrect.") + + +# Logger names, and their abbreviated forms. +_logger_names = { + "foo": "foo", + "foo.bar": "foo.bar", + "woo.foo.bar": "w.foo.bar", + # Canonical example. + "databricks.labs.ucx.foo.bar": "d.l.u.foo.bar", + # Corner-case. + "....foo.bar": "....foo.bar", +} + + +@pytest.mark.parametrize(("logger_name", "formatted_name"), tuple(_logger_names.items())) +def test_formatter_format_colorized_logger_name_abbreviated(logger_name: str, formatted_name: str) -> None: + """Ensure the logger name is abbreviated in colorized formatting.""" + formatter = NiceFormatter() + formatter.colors = True + + # Create a log record with the specified level. + record = create_record(logging.DEBUG, "Whatever", name=logger_name) + # Can't easily mark this as known to sometimes faili, so we simulate it here. + expected_failure = ".." in logger_name + try: + formatted = formatter.format(record) + if expected_failure: + pytest.fail("Unexpected success: colorized logger name abbreviation is though to fail when .. is present.") + except IndexError: + if not expected_failure: + raise + pytest.xfail("Colorized logger name abbreviation is known to fail when .. is present.") + return + stripped = _strip_sgr_sequences(formatted) + + # H:M:S LEVEL [logger_name] message + assert f" [{formatted_name}] " in stripped + + +@pytest.mark.parametrize("logger_name", tuple(_logger_names.keys())) +def test_formatter_format_non_colorized_logger_name_as_is(logger_name: str) -> None: + """Ensure the logger name is left as-is for non-colorized formatting.""" + formatter = NiceFormatter() + formatter.colors = False + + # Create a log record with the specified level. + record = create_record(logging.DEBUG, "Whatever", name=logger_name) + formatted = formatter.format(record) + + # H:M:S LEVEL [logger_name] message + assert f" [{logger_name}] " in formatted + + +def test_formatter_format_colorized_thread_name() -> None: + """The colorized formatter includes the thread name if non-main.""" + formatter = NiceFormatter() + formatter.colors = True + + # Create a log record with the specified level. + main_record = create_record(logging.DEBUG, "Record from main thread") + assert main_record.threadName == "MainThread" + assert " [MainThread] " not in _strip_sgr_sequences(formatter.format(main_record)) + + # Create a log record on a different thread. + with ThreadPoolExecutor(max_workers=1, thread_name_prefix="temporary-test-worker") as executor: + future = executor.submit(create_record, logging.DEBUG, "Record from worker thread") + thread_record = future.result() + assert thread_record.threadName and thread_record.threadName.startswith("temporary-test-worker") + # H:M:S LEVEL [logger_name][thread_name] message + assert f"][{thread_record.threadName}] " in _strip_sgr_sequences(formatter.format(thread_record)) + + +@pytest.mark.parametrize( + "use_colors", + ( + pytest.param( + True, + marks=pytest.mark.xfail( + reason="Colorized exception formatting is inconsistent with system logging.", strict=True + ), + ), + False, + ), + ids=("with_colors", "without_colors"), +) +def test_formatter_format_exception(use_colors: bool) -> None: + """The colorized formatter includes the thread name if non-main.""" + formatter = NiceFormatter() + formatter.colors = use_colors + + # Create a log record that includes attached exception information. + try: + exception_message = "Test exception." + currentframe = inspect.currentframe() + assert currentframe + exception_line = inspect.getframeinfo(currentframe).lineno + 1 + raise RuntimeError(exception_message) + except RuntimeError: + record = create_record(logging.DEBUG, "Record with exception", exc_info=True) + formatted = formatter.format(record) + stripped = _strip_sgr_sequences(formatted) if use_colors else formatted + + # H:M:S LEVEL [logger_name] message\n + # Traceback (most recent call last):\n + # File "PATH", line X, in \n + # source_of_line + # exc_type: exc_message + lines = stripped.splitlines() + msg, *traceback, exception = lines + assert msg.endswith(" Record with exception") + assert traceback == [ + "Traceback (most recent call last):", + f' File "{__file__}", line {exception_line}, in test_formatter_format_exception', + " raise RuntimeError(exception_message)", + ] + assert exception == "RuntimeError: Test exception." From 4a2e9a2165df6ae297f525235d939d4c1a111057 Mon Sep 17 00:00:00 2001 From: Andrew Snare Date: Thu, 15 May 2025 15:06:32 +0200 Subject: [PATCH 09/10] Fix formatting. --- tests/unit/test_logger.py | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/tests/unit/test_logger.py b/tests/unit/test_logger.py index a4c7e88..989b2f3 100644 --- a/tests/unit/test_logger.py +++ b/tests/unit/test_logger.py @@ -206,10 +206,10 @@ def test_formatter_format_simple_msg(use_colors: bool) -> None: @pytest.mark.parametrize( "use_colors", ( - pytest.param( - True, marks=pytest.mark.xfail(reason="Argument interpolation when colorizing doesn't work.", strict=True) - ), - False, + pytest.param( + True, marks=pytest.mark.xfail(reason="Argument interpolation when colorizing doesn't work.", strict=True) + ), + False, ), ids=("with_colors", "without_colors"), ) @@ -229,11 +229,11 @@ def test_formatter_format_msg_with_args(use_colors: bool) -> None: @pytest.mark.parametrize( "use_colors", ( - True, - pytest.param( - False, - marks=pytest.mark.xfail(reason="Non-colorized logs currently missing second-granularity.", strict=True), - ), + True, + pytest.param( + False, + marks=pytest.mark.xfail(reason="Non-colorized logs currently missing second-granularity.", strict=True), + ), ), ids=["with_colors", "without_colors"], ) @@ -360,13 +360,13 @@ def test_formatter_format_colorized_thread_name() -> None: @pytest.mark.parametrize( "use_colors", ( - pytest.param( - True, - marks=pytest.mark.xfail( - reason="Colorized exception formatting is inconsistent with system logging.", strict=True - ), + pytest.param( + True, + marks=pytest.mark.xfail( + reason="Colorized exception formatting is inconsistent with system logging.", strict=True ), - False, + ), + False, ), ids=("with_colors", "without_colors"), ) From d97b95f012c32343288083b5eaf4b80852de328b Mon Sep 17 00:00:00 2001 From: Andrew Snare Date: Thu, 15 May 2025 15:35:08 +0200 Subject: [PATCH 10/10] Simplify assignment. --- src/databricks/labs/blueprint/logger.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/databricks/labs/blueprint/logger.py b/src/databricks/labs/blueprint/logger.py index ae1485a..13e73d9 100644 --- a/src/databricks/labs/blueprint/logger.py +++ b/src/databricks/labs/blueprint/logger.py @@ -71,9 +71,8 @@ def format(self, record: logging.LogRecord) -> str: color_marker = self.BOLD elif record.levelno in (logging.ERROR, logging.FATAL): color_marker = self.RED + self.BOLD - thread_name = "" - if record.threadName != "MainThread": - thread_name = f"[{record.threadName}]" + + thread_name = f"[{record.threadName}]" if record.threadName != "MainThread" else "" return f"{self.GRAY}{timestamp}{self.RESET} {level} {color_marker}[{name}]{thread_name} {msg}{self.RESET}"