Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix rich markup regression in 0.19.7 #4097

Merged
merged 13 commits into from
Aug 16, 2024
2 changes: 1 addition & 1 deletion RELEASE.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
11 changes: 4 additions & 7 deletions kedro/io/data_catalog.py
Original file line number Diff line number Diff line change
Expand Up @@ -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]]

Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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},
)
Expand Down Expand Up @@ -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},
)
Expand Down
12 changes: 0 additions & 12 deletions kedro/logging.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import logging
import sys
from functools import lru_cache
from pathlib import Path
from typing import Any

Expand Down Expand Up @@ -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}]"
19 changes: 18 additions & 1 deletion kedro/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down Expand Up @@ -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}]"
18 changes: 13 additions & 5 deletions tests/framework/project/test_logging.py
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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):
Expand Down
6 changes: 6 additions & 0 deletions tests/framework/session/test_session.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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):
Expand Down