fix rich markup regression in 0.19.7 (#4097)

* Move rich detecting and formatting to utils.py

Signed-off-by: Laura Couto <laurarccouto@gmail.com>

* Fix import on test for _has_rich_handler

Signed-off-by: Laura Couto <laurarccouto@gmail.com>

* Adjust tests for coverage

Signed-off-by: Laura Couto <laurarccouto@gmail.com>

* Adjust tests for coverage

Signed-off-by: Laura Couto <laurarccouto@gmail.com>

* Add failed import case to test

Signed-off-by: Laura Couto <laurarccouto@gmail.com>

* fix rich markup regression åçin 0.19.7

Signed-off-by: Nok <nok.lam.chan@quantumblack.com>

* Fix linter error

Signed-off-by: Laura Couto <laurarccouto@gmail.com>

* move test to session tests as rich handler has nothing to do with DataCatalog now

Signed-off-by: Nok <nok.lam.chan@quantumblack.com>

* format

Signed-off-by: Nok <nok.lam.chan@quantumblack.com>

---------

Signed-off-by: Laura Couto <laurarccouto@gmail.com>
Signed-off-by: Nok <nok.lam.chan@quantumblack.com>
Co-authored-by: Laura Couto <laurarccouto@gmail.com>
Co-authored-by: L. R. Couto <57910428+lrcouto@users.noreply.github.com>

RELEASE.md

@@ -6,7 +6,7 @@
 ## Bug fixes and other changes
 * Moved `_find_run_command()` and `_find_run_command_in_plugins()` from `__main__.py` in the project template to the framework itself.
 * Fixed a bug where `%load_node` breaks with multi-lines import statements.
-
+* Fixed a regression where `rich` mark up logs stop showing since 0.19.7.
 ## Breaking changes to the API
 
 ## Documentation changes

kedro/io/data_catalog.py

@@ -25,7 +25,7 @@
     generate_timestamp,
 )
 from kedro.io.memory_dataset import MemoryDataset
-from kedro.logging import _format_rich, _has_rich_handler
+from kedro.utils import _format_rich, _has_rich_handler
 
 Patterns = Dict[str, Dict[str, Any]]
 
@@ -213,6 +213,7 @@ def __init__(  # noqa: PLR0913
         self._load_versions = load_versions or {}
         self._save_version = save_version
         self._default_pattern = default_pattern or {}
+        self._use_rich_markup = _has_rich_handler()
 
         if feed_dict:
             self.add_feed_dict(feed_dict)
@@ -536,9 +537,7 @@ def load(self, name: str, version: str | None = None) -> Any:
 
         self._logger.info(
             "Loading data from %s (%s)...",
-            _format_rich(name, "dark_orange")
-            if _has_rich_handler(self._logger)
-            else name,
+            _format_rich(name, "dark_orange") if self._use_rich_markup else name,
             type(dataset).__name__,
             extra={"markup": True},
         )
@@ -580,9 +579,7 @@ def save(self, name: str, data: Any) -> None:
 
         self._logger.info(
             "Saving data to %s (%s)...",
-            _format_rich(name, "dark_orange")
-            if _has_rich_handler(self._logger)
-            else name,
+            _format_rich(name, "dark_orange") if self._use_rich_markup else name,
             type(dataset).__name__,
             extra={"markup": True},
         )

kedro/logging.py

@@ -1,6 +1,5 @@
 import logging
 import sys
-from functools import lru_cache
 from pathlib import Path
 from typing import Any
 
@@ -55,14 +54,3 @@ 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}]"

kedro/utils.py

@@ -2,9 +2,10 @@
 of kedro package.
 """
 import importlib
+import logging
 import os
 from pathlib import Path
-from typing import Any, Union
+from typing import Any, Optional, Union
 
 _PYPROJECT = "pyproject.toml"
 
@@ -78,3 +79,19 @@ def _find_kedro_project(current_dir: Path) -> Any:  # pragma: no cover
         if _is_project(parent_dir):
             return parent_dir
     return None
+
+
+def _has_rich_handler(logger: Optional[logging.Logger] = None) -> bool:
+    """Returns true if the logger has a RichHandler attached."""
+    if not logger:
+        logger = logging.getLogger()  # User root by default
+    try:
+        from rich.logging import RichHandler
+    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}]"

tests/framework/project/test_logging.py

@@ -1,12 +1,14 @@
+import importlib
 import logging
 import sys
 from pathlib import Path
+from unittest import mock
 
 import pytest
 import yaml
 
 from kedro.framework.project import LOGGING, configure_logging, configure_project
-from kedro.logging import RichHandler, _format_rich, _has_rich_handler
+from kedro.utils import _format_rich, _has_rich_handler
 
 
 @pytest.fixture
@@ -156,10 +158,16 @@ def test_rich_format():
 
 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)
+    with mock.patch("builtins.__import__", side_effect=ImportError):
+        assert not _has_rich_handler(test_logger)
+
+    if importlib.util.find_spec("rich"):
+        from rich.logging import RichHandler
+
+        test_logger.addHandler(RichHandler())
+        assert _has_rich_handler(test_logger)
+    else:
+        assert not _has_rich_handler(test_logger)
 
 
 def test_default_logging_info_emission(monkeypatch, tmp_path, caplog):

tests/framework/session/test_session.py

@@ -29,6 +29,7 @@
 from kedro.framework.session.session import KedroSessionError
 from kedro.framework.session.shelvestore import ShelveStore
 from kedro.framework.session.store import BaseSessionStore
+from kedro.utils import _has_rich_handler
 
 _FAKE_PROJECT_NAME = "fake_project"
 _FAKE_PIPELINE_NAME = "fake_pipeline"
@@ -948,6 +949,11 @@ def test_session_raise_error_with_invalid_runner_instance(
             # Execute run with SequentialRunner class instead of SequentialRunner()
             session.run(runner=mock_runner_class)
 
+    def test_logging_rich_markup(self, fake_project):
+        # Make sure RichHandler is registered as root's handlers as in a Kedro Project.
+        KedroSession.create(fake_project)
+        assert _has_rich_handler()
+
 
 @pytest.fixture
 def fake_project_with_logging_file_handler(fake_project):