From 816f41fe5156847f83a1ad43d00e5c21262422aa Mon Sep 17 00:00:00 2001 From: Laura Couto Date: Wed, 14 Aug 2024 12:25:26 -0300 Subject: [PATCH 1/5] Move rich detecting and formatting to utils.py Signed-off-by: Laura Couto --- kedro/io/data_catalog.py | 2 +- kedro/logging.py | 12 ------------ kedro/utils.py | 22 ++++++++++++++++++++++ tests/framework/project/test_logging.py | 3 ++- 4 files changed, 25 insertions(+), 14 deletions(-) diff --git a/kedro/io/data_catalog.py b/kedro/io/data_catalog.py index 9d090ce56f..141a3f47da 100644 --- a/kedro/io/data_catalog.py +++ b/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]] diff --git a/kedro/logging.py b/kedro/logging.py index d31ac41945..83e8204fec 100644 --- a/kedro/logging.py +++ b/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}]" diff --git a/kedro/utils.py b/kedro/utils.py index 0d4285e05c..cf559d95fc 100644 --- a/kedro/utils.py +++ b/kedro/utils.py @@ -2,10 +2,17 @@ of kedro package. """ import importlib +import logging import os +from functools import lru_cache from pathlib import Path from typing import Any, Union +try: + from kedro.logging import RichHandler +except ImportError: + pass + _PYPROJECT = "pyproject.toml" @@ -78,3 +85,18 @@ def _find_kedro_project(current_dir: Path) -> Any: # pragma: no cover if _is_project(parent_dir): return parent_dir return None + + +@lru_cache(maxsize=None) +def _has_rich_handler(logger: logging.Logger) -> bool: + """Returns true if the logger has a RichHandler attached.""" + try: + importlib.util.find_spec("rich") + 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 7a90405ecb..a2c7bc55b3 100644 --- a/tests/framework/project/test_logging.py +++ b/tests/framework/project/test_logging.py @@ -6,7 +6,8 @@ import yaml from kedro.framework.project import LOGGING, configure_logging, configure_project -from kedro.logging import RichHandler, _format_rich, _has_rich_handler +from kedro.logging import RichHandler +from kedro.utils import _format_rich, _has_rich_handler @pytest.fixture From 99486346cdcd6b10652e7c0ae86e55d953dbdc58 Mon Sep 17 00:00:00 2001 From: Laura Couto Date: Wed, 14 Aug 2024 16:40:54 -0300 Subject: [PATCH 2/5] Fix import on test for _has_rich_handler Signed-off-by: Laura Couto --- kedro/utils.py | 7 +------ tests/framework/project/test_logging.py | 9 ++++++--- 2 files changed, 7 insertions(+), 9 deletions(-) diff --git a/kedro/utils.py b/kedro/utils.py index cf559d95fc..91378ab0cc 100644 --- a/kedro/utils.py +++ b/kedro/utils.py @@ -8,11 +8,6 @@ from pathlib import Path from typing import Any, Union -try: - from kedro.logging import RichHandler -except ImportError: - pass - _PYPROJECT = "pyproject.toml" @@ -91,7 +86,7 @@ def _find_kedro_project(current_dir: Path) -> Any: # pragma: no cover def _has_rich_handler(logger: logging.Logger) -> bool: """Returns true if the logger has a RichHandler attached.""" try: - importlib.util.find_spec("rich") + from rich.logging import RichHandler except ImportError: return False return any(isinstance(handler, RichHandler) for handler in logger.handlers) diff --git a/tests/framework/project/test_logging.py b/tests/framework/project/test_logging.py index a2c7bc55b3..dc9a04315b 100644 --- a/tests/framework/project/test_logging.py +++ b/tests/framework/project/test_logging.py @@ -1,3 +1,4 @@ +import importlib import logging import sys from pathlib import Path @@ -6,7 +7,6 @@ import yaml from kedro.framework.project import LOGGING, configure_logging, configure_project -from kedro.logging import RichHandler from kedro.utils import _format_rich, _has_rich_handler @@ -159,8 +159,11 @@ 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) + if importlib.util.find_spec("rich"): + from kedro.logging import RichHandler + + test_logger.addHandler(RichHandler()) + assert _has_rich_handler(test_logger) def test_default_logging_info_emission(monkeypatch, tmp_path, caplog): From c871a08a865b65905775cdd466d083692fa912e6 Mon Sep 17 00:00:00 2001 From: Laura Couto Date: Wed, 14 Aug 2024 16:48:05 -0300 Subject: [PATCH 3/5] Adjust tests for coverage Signed-off-by: Laura Couto --- tests/framework/project/test_logging.py | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/tests/framework/project/test_logging.py b/tests/framework/project/test_logging.py index dc9a04315b..1b1a8d4fe5 100644 --- a/tests/framework/project/test_logging.py +++ b/tests/framework/project/test_logging.py @@ -155,15 +155,20 @@ def test_rich_format(): ) -def test_has_rich_handler(): +def test_has_rich_handler_true(): test_logger = logging.getLogger("test_logger") - assert not _has_rich_handler(test_logger) - _has_rich_handler.cache_clear() if importlib.util.find_spec("rich"): from kedro.logging import RichHandler test_logger.addHandler(RichHandler()) assert _has_rich_handler(test_logger) + else: + pass + + +def test_has_rich_handler_false(): + test_logger = logging.getLogger("test_logger") + assert not _has_rich_handler(test_logger) def test_default_logging_info_emission(monkeypatch, tmp_path, caplog): From 771e1a85f3225fdc4c0cfbca2faa21c6d54a56ca Mon Sep 17 00:00:00 2001 From: Laura Couto Date: Wed, 14 Aug 2024 16:57:45 -0300 Subject: [PATCH 4/5] Adjust tests for coverage Signed-off-by: Laura Couto --- tests/framework/project/test_logging.py | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/tests/framework/project/test_logging.py b/tests/framework/project/test_logging.py index 1b1a8d4fe5..f3f4cd4a2f 100644 --- a/tests/framework/project/test_logging.py +++ b/tests/framework/project/test_logging.py @@ -155,20 +155,17 @@ def test_rich_format(): ) -def test_has_rich_handler_true(): +def test_has_rich_handler(): test_logger = logging.getLogger("test_logger") + assert not _has_rich_handler(test_logger) + _has_rich_handler.cache_clear() if importlib.util.find_spec("rich"): from kedro.logging import RichHandler test_logger.addHandler(RichHandler()) assert _has_rich_handler(test_logger) else: - pass - - -def test_has_rich_handler_false(): - test_logger = logging.getLogger("test_logger") - assert not _has_rich_handler(test_logger) + assert not _has_rich_handler(test_logger) def test_default_logging_info_emission(monkeypatch, tmp_path, caplog): From f922b1522ebb1b6b8e14b4fac77db895692e149d Mon Sep 17 00:00:00 2001 From: Laura Couto Date: Wed, 14 Aug 2024 17:26:25 -0300 Subject: [PATCH 5/5] Add failed import case to test Signed-off-by: Laura Couto --- tests/framework/project/test_logging.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/tests/framework/project/test_logging.py b/tests/framework/project/test_logging.py index f3f4cd4a2f..4f5962efe9 100644 --- a/tests/framework/project/test_logging.py +++ b/tests/framework/project/test_logging.py @@ -2,6 +2,7 @@ import logging import sys from pathlib import Path +from unittest import mock import pytest import yaml @@ -157,10 +158,12 @@ def test_rich_format(): def test_has_rich_handler(): test_logger = logging.getLogger("test_logger") - assert not _has_rich_handler(test_logger) + with mock.patch("builtins.__import__", side_effect=ImportError): + assert not _has_rich_handler(test_logger) _has_rich_handler.cache_clear() + if importlib.util.find_spec("rich"): - from kedro.logging import RichHandler + from rich.logging import RichHandler test_logger.addHandler(RichHandler()) assert _has_rich_handler(test_logger)