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

Disable rich markup if rich handler is not present #3682

Merged
merged 22 commits into from
Jul 5, 2024
Merged
Show file tree
Hide file tree
Changes from 13 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions kedro/io/data_catalog.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
generate_timestamp,
)
from kedro.io.memory_dataset import MemoryDataset
from kedro.logging import fmt_rich, has_rich_handler

Patterns = Dict[str, Dict[str, Any]]

Expand Down Expand Up @@ -513,8 +514,8 @@ 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)...",
fmt_rich(name, "dark_orange") if has_rich_handler(self._logger) else name,
type(dataset).__name__,
extra={"markup": True},
)
Expand Down Expand Up @@ -555,8 +556,8 @@ 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)...",
fmt_rich(name, "dark_orange") if has_rich_handler(self._logger) else name,
type(dataset).__name__,
extra={"markup": True},
)
Expand Down
13 changes: 13 additions & 0 deletions kedro/logging.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import logging
import sys
from functools import lru_cache
from pathlib import Path
from typing import Any

Expand Down Expand Up @@ -54,3 +55,15 @@ 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]


# @cache is supported from 3.9 onwards
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: I would just leave out this comment; pyupgrade will take care of it when drop 3.8.

@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)

sbrugman marked this conversation as resolved.
Show resolved Hide resolved

def fmt_rich(value: str, markup: str) -> str:
"""Format string with rich markup"""
return f"[{markup}]{value}[/{markup}]"
34 changes: 34 additions & 0 deletions tests/framework/project/test_logging.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import yaml

from kedro.framework.project import LOGGING, configure_logging, configure_project
from kedro.logging import RichHandler, fmt_rich, has_rich_handler


@pytest.fixture
Expand Down Expand Up @@ -148,6 +149,39 @@ def test_rich_traceback_disabled_on_databricks(
rich_pretty_install.assert_called()


def test_rich_format():
assert (
fmt_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)
test_logger.addHandler(RichHandler())
assert has_rich_handler(test_logger)


def test_default_logging_info_emission(monkeypatch, capsys):
# Expected path and logging configuration
expected_path = Path("conf/logging.yml")
dummy_logging_config = yaml.dump(
{"version": 1, "loggers": {"kedro": {"level": "INFO"}}}
)

# Setup environment and path mocks
monkeypatch.delenv("KEDRO_LOGGING_CONFIG", raising=False)
monkeypatch.setattr(Path, "exists", lambda x: x == expected_path)
monkeypatch.setattr(
Path,
"read_text",
lambda x, encoding="utf-8": dummy_logging_config
if x == expected_path
else FileNotFoundError("File not found"),
)


def test_environment_variable_logging_config2(monkeypatch, tmp_path, caplog):
config_path = (Path(tmp_path) / "conf" / "logging.yml").absolute()
config_path.parent.mkdir(parents=True)
Expand Down
Loading