From a179f87f0f170075ed3694d8a154937a9a96254c Mon Sep 17 00:00:00 2001 From: Simon Brugman Date: Fri, 5 Jul 2024 15:14:33 +0200 Subject: [PATCH] Disable rich markup if rich handler is not present (#3682) * Rich utils Signed-off-by: Simon Brugman * Conditional logging data_catalog * Update data_catalog.py * Update logging.py * Add unit tests * Update test_logging.py * Lint * Cache `has_rich_logger` * Backwards compatibility 3.8 * Update test_logging.py * Revert "Update test_logging.py" This reverts commit 802ad2607f27c5e68ec11b5ca3dbe5c03995230b. * Revert merge Signed-off-by: lrcouto * revert test_logging.py to commit 802ad26 Signed-off-by: lrcouto * Fix test, apply small fixes Signed-off-by: lrcouto * Fix non-matching character in expected_message Signed-off-by: lrcouto * Remove leftover print Signed-off-by: lrcouto * fix test Signed-off-by: Nok --------- Signed-off-by: Simon Brugman Signed-off-by: L. R. Couto <57910428+lrcouto@users.noreply.github.com> Signed-off-by: lrcouto Signed-off-by: Nok Co-authored-by: Nok Lam Chan Co-authored-by: L. R. Couto <57910428+lrcouto@users.noreply.github.com> Co-authored-by: lrcouto --- kedro/io/data_catalog.py | 13 +++++++---- kedro/logging.py | 12 ++++++++++ tests/framework/project/test_logging.py | 30 ++++++++++++++++++------- 3 files changed, 43 insertions(+), 12 deletions(-) diff --git a/kedro/io/data_catalog.py b/kedro/io/data_catalog.py index f9f74f8fbe..6c0c87f522 100644 --- a/kedro/io/data_catalog.py +++ b/kedro/io/data_catalog.py @@ -24,6 +24,7 @@ generate_timestamp, ) from kedro.io.memory_dataset import MemoryDataset +from kedro.logging import _format_rich, _has_rich_handler Patterns = Dict[str, Dict[str, Any]] @@ -521,8 +522,10 @@ def load(self, name: str, version: str | None = None) -> Any: dataset = self._get_dataset(name, version=load_version) self._logger.info( - "Loading data from [dark_orange]%s[/dark_orange] (%s)...", - name, + "Loading data from %s (%s)...", + _format_rich(name, "dark_orange") + if _has_rich_handler(self._logger) + else name, type(dataset).__name__, extra={"markup": True}, ) @@ -563,8 +566,10 @@ def save(self, name: str, data: Any) -> None: dataset = self._get_dataset(name) self._logger.info( - "Saving data to [dark_orange]%s[/dark_orange] (%s)...", - name, + "Saving data to %s (%s)...", + _format_rich(name, "dark_orange") + if _has_rich_handler(self._logger) + else name, type(dataset).__name__, extra={"markup": True}, ) diff --git a/kedro/logging.py b/kedro/logging.py index 83e8204fec..d31ac41945 100644 --- a/kedro/logging.py +++ b/kedro/logging.py @@ -1,5 +1,6 @@ import logging import sys +from functools import lru_cache from pathlib import Path from typing import Any @@ -54,3 +55,14 @@ 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] + + +@lru_cache(maxsize=None) +def _has_rich_handler(logger: logging.Logger) -> bool: + """Returns true if the logger has a RichHandler attached.""" + 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 dfac58313e..7a90405ecb 100644 --- a/tests/framework/project/test_logging.py +++ b/tests/framework/project/test_logging.py @@ -6,6 +6,7 @@ import yaml from kedro.framework.project import LOGGING, configure_logging, configure_project +from kedro.logging import RichHandler, _format_rich, _has_rich_handler @pytest.fixture @@ -35,10 +36,10 @@ def test_project_logging_in_default_logging_config(default_logging_config_with_p assert logging.getLogger("test_project").level == logging.INFO -def test_environment_variable_logging_config(monkeypatch, tmp_path, caplog): - config_path = (Path(tmp_path) / "logging.yml").absolute() - monkeypatch.setenv("KEDRO_LOGGING_CONFIG", config_path) - logging_config = {"version": 1, "loggers": {"kedro": {"level": "DEBUG"}}} +def test_environment_variable_logging_config(monkeypatch, tmp_path): + config_path = Path(tmp_path) / "logging.yml" + monkeypatch.setenv("KEDRO_LOGGING_CONFIG", config_path.absolute()) + logging_config = {"version": 1, "loggers": {"kedro": {"level": "WARNING"}}} with config_path.open("w", encoding="utf-8") as f: yaml.dump(logging_config, f) from kedro.framework.project import _ProjectLogging @@ -46,9 +47,7 @@ def test_environment_variable_logging_config(monkeypatch, tmp_path, caplog): LOGGING = _ProjectLogging() assert LOGGING.data == logging_config - assert logging.getLogger("kedro").level == logging.DEBUG - expected_message = f"Using '{config_path}'" - assert expected_message in "".join(caplog.messages).strip("\n") + assert logging.getLogger("kedro").level == logging.WARNING def test_configure_logging(): @@ -148,7 +147,22 @@ def test_rich_traceback_disabled_on_databricks( rich_pretty_install.assert_called() -def test_environment_variable_logging_config2(monkeypatch, tmp_path, caplog): +def test_rich_format(): + assert ( + _format_rich("Hello World!", "dark_orange") + == "[dark_orange]Hello World![/dark_orange]" + ) + + +def test_has_rich_handler(): + test_logger = logging.getLogger("test_logger") + assert not _has_rich_handler(test_logger) + _has_rich_handler.cache_clear() + test_logger.addHandler(RichHandler()) + assert _has_rich_handler(test_logger) + + +def test_default_logging_info_emission(monkeypatch, tmp_path, caplog): config_path = (Path(tmp_path) / "conf" / "logging.yml").absolute() config_path.parent.mkdir(parents=True) logging_config = {"version": 1, "loggers": {"kedro": {"level": "DEBUG"}}}