From 9d8d2147191e8e02a3114da44ff291f794b4db38 Mon Sep 17 00:00:00 2001 From: Nok Lam Chan Date: Tue, 18 Feb 2025 15:21:12 +0000 Subject: [PATCH 1/5] add a failing test case Signed-off-by: Nok Lam Chan --- tests/framework/project/test_logging.py | 26 +++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/tests/framework/project/test_logging.py b/tests/framework/project/test_logging.py index b98f14f6bc..fdfa1cb7bb 100644 --- a/tests/framework/project/test_logging.py +++ b/tests/framework/project/test_logging.py @@ -8,6 +8,7 @@ import yaml from kedro.framework.project import LOGGING, configure_logging, configure_project +from kedro.io.data_catalog import DataCatalog from kedro.utils import _format_rich, _has_rich_handler @@ -187,3 +188,28 @@ def test_default_logging_info_emission(monkeypatch, tmp_path, caplog): assert logging.getLogger("kedro").level == logging.DEBUG expected_message = "You can change this by setting the KEDRO_LOGGING_CONFIG environment variable accordingly." assert expected_message in "".join(caplog.messages).strip("\n") + + +def test_logger_without_rich_markup(): + class CustomHandler(logging.Handler): + def __init__(self): + super().__init__() + self.records = [] + + def emit(self, record): + self.records.append(record) + + data = ("dummy",) + catalog = DataCatalog.from_config({"dummy": {"type": "MemoryDataset"}}) + + # Add a custom handler + custom_handler = CustomHandler() + root_logger = logging.getLogger() + root_logger.addHandler(custom_handler) + + # Emit some logs + assert not custom_handler.records + catalog.save("dummy", data) + assert custom_handler.records + for record in custom_handler.records: + assert "[dark_orange]" not in record.message From 6800de4dab80d6b7302c7279c0051fe5d404b948 Mon Sep 17 00:00:00 2001 From: Nok Lam Chan Date: Tue, 18 Feb 2025 15:55:27 +0000 Subject: [PATCH 2/5] move format_rich to logging module Signed-off-by: Nok Lam Chan --- kedro/io/data_catalog.py | 3 ++- kedro/io/kedro_data_catalog.py | 3 ++- kedro/logging.py | 5 +++++ kedro/utils.py | 5 ----- tests/framework/project/test_logging.py | 3 ++- 5 files changed, 11 insertions(+), 8 deletions(-) diff --git a/kedro/io/data_catalog.py b/kedro/io/data_catalog.py index 42863c735f..364a81b2c6 100644 --- a/kedro/io/data_catalog.py +++ b/kedro/io/data_catalog.py @@ -29,7 +29,8 @@ generate_timestamp, ) from kedro.io.memory_dataset import MemoryDataset -from kedro.utils import _format_rich, _has_rich_handler +from kedro.logging import _format_rich +from kedro.utils import _has_rich_handler CATALOG_KEY = "catalog" # Kept to avoid the breaking change WORDS_REGEX_PATTERN = re.compile(r"\W+") diff --git a/kedro/io/kedro_data_catalog.py b/kedro/io/kedro_data_catalog.py index 49cfe99333..6d47306894 100644 --- a/kedro/io/kedro_data_catalog.py +++ b/kedro/io/kedro_data_catalog.py @@ -30,7 +30,8 @@ parse_dataset_definition, ) from kedro.io.memory_dataset import MemoryDataset, _is_memory_dataset -from kedro.utils import _format_rich, _has_rich_handler +from kedro.logging import _format_rich +from kedro.utils import _has_rich_handler class _LazyDataset: diff --git a/kedro/logging.py b/kedro/logging.py index 83e8204fec..c3197874ae 100644 --- a/kedro/logging.py +++ b/kedro/logging.py @@ -54,3 +54,8 @@ def __init__(self, *args: Any, **kwargs: Any): # fixed on their side at some point, but until then we disable it. # See https://github.com/Textualize/rich/issues/2455 rich.traceback.install(**traceback_install_kwargs) # type: ignore[arg-type] + + +def _format_rich(value: str, markup: str) -> str: + """Format string with rich markup""" + return f"[{markup}]{value}[/{markup}]" diff --git a/kedro/utils.py b/kedro/utils.py index 2d99d3d4e6..534c08a19e 100644 --- a/kedro/utils.py +++ b/kedro/utils.py @@ -91,8 +91,3 @@ def _has_rich_handler(logger: Optional[logging.Logger] = None) -> bool: except ImportError: return False return any(isinstance(handler, RichHandler) for handler in logger.handlers) - - -def _format_rich(value: str, markup: str) -> str: - """Format string with rich markup""" - return f"[{markup}]{value}[/{markup}]" diff --git a/tests/framework/project/test_logging.py b/tests/framework/project/test_logging.py index fdfa1cb7bb..d84bcc624d 100644 --- a/tests/framework/project/test_logging.py +++ b/tests/framework/project/test_logging.py @@ -1,6 +1,7 @@ import importlib import logging import sys +from logging import _format_rich from pathlib import Path from unittest import mock @@ -9,7 +10,7 @@ from kedro.framework.project import LOGGING, configure_logging, configure_project from kedro.io.data_catalog import DataCatalog -from kedro.utils import _format_rich, _has_rich_handler +from kedro.utils import _has_rich_handler @pytest.fixture From f55763f6f8cea21d9246c5d908e5755ee67bef1b Mon Sep 17 00:00:00 2001 From: Nok Lam Chan Date: Tue, 18 Feb 2025 16:16:22 +0000 Subject: [PATCH 3/5] move markup=True as default Signed-off-by: Nok Lam Chan --- kedro/io/data_catalog.py | 2 -- kedro/logging.py | 15 +++++++++++++++ 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/kedro/io/data_catalog.py b/kedro/io/data_catalog.py index 364a81b2c6..4396c085ee 100644 --- a/kedro/io/data_catalog.py +++ b/kedro/io/data_catalog.py @@ -392,7 +392,6 @@ def load(self, name: str, version: str | None = None) -> Any: "Loading data from %s (%s)...", _format_rich(name, "dark_orange") if self._use_rich_markup else name, type(dataset).__name__, - extra={"markup": True}, ) result = dataset.load() @@ -434,7 +433,6 @@ def save(self, name: str, data: Any) -> None: "Saving data to %s (%s)...", _format_rich(name, "dark_orange") if self._use_rich_markup else name, type(dataset).__name__, - extra={"markup": True}, ) dataset.save(data) diff --git a/kedro/logging.py b/kedro/logging.py index c3197874ae..059ebf27c1 100644 --- a/kedro/logging.py +++ b/kedro/logging.py @@ -1,5 +1,6 @@ import logging import sys +from logging import LogRecord from pathlib import Path from typing import Any @@ -26,6 +27,8 @@ class RichHandler(rich.logging.RichHandler): """ def __init__(self, *args: Any, **kwargs: Any): + if "markup" not in kwargs: + kwargs.update({"markup": True}) # Set markup as default super().__init__(*args, **kwargs) logging.captureWarnings(True) rich.pretty.install() @@ -55,6 +58,18 @@ def __init__(self, *args: Any, **kwargs: Any): # See https://github.com/Textualize/rich/issues/2455 rich.traceback.install(**traceback_install_kwargs) # type: ignore[arg-type] + def emit(self, record: LogRecord): + args = record.args + new_args = list(args) + if hasattr(record, "rich_format"): + # Add markup to the message + # if the list is shorter than the arg list, keep it unchanged + for i, color in enumerate(record.rich_format): + new_args[i] = _format_rich(args[i], color) + + record.args = tuple(new_args) + super().emit(record) + def _format_rich(value: str, markup: str) -> str: """Format string with rich markup""" From f05e525c08171eeee0334bb0c5003478bccde832 Mon Sep 17 00:00:00 2001 From: Nok Lam Chan Date: Tue, 18 Feb 2025 16:18:32 +0000 Subject: [PATCH 4/5] use a kedro specific "extra" to add markup language Signed-off-by: Nok Lam Chan --- kedro/io/data_catalog.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/kedro/io/data_catalog.py b/kedro/io/data_catalog.py index 4396c085ee..40fc1312a9 100644 --- a/kedro/io/data_catalog.py +++ b/kedro/io/data_catalog.py @@ -29,7 +29,6 @@ generate_timestamp, ) from kedro.io.memory_dataset import MemoryDataset -from kedro.logging import _format_rich from kedro.utils import _has_rich_handler CATALOG_KEY = "catalog" # Kept to avoid the breaking change @@ -390,8 +389,9 @@ def load(self, name: str, version: str | None = None) -> Any: self._logger.info( "Loading data from %s (%s)...", - _format_rich(name, "dark_orange") if self._use_rich_markup else name, + name, type(dataset).__name__, + extra={"rich_format": ["dark_orange"]}, ) result = dataset.load() @@ -431,8 +431,9 @@ def save(self, name: str, data: Any) -> None: self._logger.info( "Saving data to %s (%s)...", - _format_rich(name, "dark_orange") if self._use_rich_markup else name, + name, type(dataset).__name__, + extra={"rich_format": ["dark_orange"]}, ) dataset.save(data) From 4babd7fe25d376d14ddeaef526ad4e3b195c6d97 Mon Sep 17 00:00:00 2001 From: Nok Lam Chan Date: Wed, 19 Feb 2025 12:25:48 +0000 Subject: [PATCH 5/5] fix test Signed-off-by: Nok Lam Chan --- kedro/logging.py | 7 +++++-- tests/framework/project/test_logging.py | 3 ++- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/kedro/logging.py b/kedro/logging.py index 059ebf27c1..70b94af5c1 100644 --- a/kedro/logging.py +++ b/kedro/logging.py @@ -1,3 +1,4 @@ +import copy import logging import sys from logging import LogRecord @@ -61,14 +62,16 @@ def __init__(self, *args: Any, **kwargs: Any): def emit(self, record: LogRecord): args = record.args new_args = list(args) + # LogRecord is shared among all handler, created a new instance so it does not affect the rest. + updated_record = copy.copy(record) if hasattr(record, "rich_format"): # Add markup to the message # if the list is shorter than the arg list, keep it unchanged for i, color in enumerate(record.rich_format): new_args[i] = _format_rich(args[i], color) - record.args = tuple(new_args) - super().emit(record) + updated_record.args = tuple(new_args) + super().emit(updated_record) def _format_rich(value: str, markup: str) -> str: diff --git a/tests/framework/project/test_logging.py b/tests/framework/project/test_logging.py index d84bcc624d..ed28d8b2d7 100644 --- a/tests/framework/project/test_logging.py +++ b/tests/framework/project/test_logging.py @@ -1,7 +1,6 @@ import importlib import logging import sys -from logging import _format_rich from pathlib import Path from unittest import mock @@ -10,6 +9,7 @@ from kedro.framework.project import LOGGING, configure_logging, configure_project from kedro.io.data_catalog import DataCatalog +from kedro.logging import _format_rich from kedro.utils import _has_rich_handler @@ -212,5 +212,6 @@ def emit(self, record): assert not custom_handler.records catalog.save("dummy", data) assert custom_handler.records + for record in custom_handler.records: assert "[dark_orange]" not in record.message