From 15f97da739ce74698ed47818e7784877d716f4d6 Mon Sep 17 00:00:00 2001 From: Simon Brugman Date: Mon, 4 Mar 2024 18:40:02 +0100 Subject: [PATCH 01/17] Rich utils Signed-off-by: Simon Brugman --- kedro/logging.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/kedro/logging.py b/kedro/logging.py index 72d85d41c5..c1c8d5dc5f 100644 --- a/kedro/logging.py +++ b/kedro/logging.py @@ -58,3 +58,13 @@ 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] + + +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 rich_color(value: str, color: str) -> str: + """Format string with rich color""" + return f"[{color}]{value}[/{color}]" From 62ece11e83ede927404c0f1404828696773806dd Mon Sep 17 00:00:00 2001 From: Simon Brugman Date: Mon, 4 Mar 2024 18:41:36 +0100 Subject: [PATCH 02/17] Conditional logging data_catalog --- kedro/io/data_catalog.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/kedro/io/data_catalog.py b/kedro/io/data_catalog.py index 411cf14e09..d3075ce700 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 has_rich_handler, rich_color Patterns = Dict[str, Dict[str, Any]] @@ -481,8 +482,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)...", + rich_color(name, "dark_orange") if has_rich_handler(self._logger) else name, type(dataset).__name__, extra={"markup": True}, ) From 0a4edb568f7614cc032d7fbcf44be7bb8b678767 Mon Sep 17 00:00:00 2001 From: Simon Brugman Date: Mon, 4 Mar 2024 18:47:35 +0100 Subject: [PATCH 03/17] Update data_catalog.py --- kedro/io/data_catalog.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/kedro/io/data_catalog.py b/kedro/io/data_catalog.py index d3075ce700..7a16985e31 100644 --- a/kedro/io/data_catalog.py +++ b/kedro/io/data_catalog.py @@ -24,7 +24,7 @@ generate_timestamp, ) from kedro.io.memory_dataset import MemoryDataset -from kedro.logging import has_rich_handler, rich_color +from kedro.logging import fmt_rich, has_rich_handler Patterns = Dict[str, Dict[str, Any]] @@ -483,7 +483,7 @@ def load(self, name: str, version: str | None = None) -> Any: self._logger.info( "Loading data from %s (%s)...", - rich_color(name, "dark_orange") if has_rich_handler(self._logger) else name, + fmt_rich(name, "dark_orange") if has_rich_handler(self._logger) else name, type(dataset).__name__, extra={"markup": True}, ) @@ -524,8 +524,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}, ) From 86a8842cda02371bd364e1931bbc9bb901b8be5d Mon Sep 17 00:00:00 2001 From: Simon Brugman Date: Mon, 4 Mar 2024 18:48:25 +0100 Subject: [PATCH 04/17] Update logging.py --- kedro/logging.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/kedro/logging.py b/kedro/logging.py index c1c8d5dc5f..fa5f6ab9d9 100644 --- a/kedro/logging.py +++ b/kedro/logging.py @@ -65,6 +65,6 @@ def has_rich_handler(logger: logging.Logger) -> bool: return any(isinstance(handler, RichHandler) for handler in logger.handlers) -def rich_color(value: str, color: str) -> str: - """Format string with rich color""" - return f"[{color}]{value}[/{color}]" +def fmt_rich(value: str, markup: str) -> str: + """Format string with rich markup""" + return f"[{markup}]{value}[/{markup}]" From 0742860f969222917954e989c8a8f6c4f2b488e6 Mon Sep 17 00:00:00 2001 From: Simon Brugman Date: Mon, 4 Mar 2024 19:08:56 +0100 Subject: [PATCH 05/17] Add unit tests --- tests/framework/project/test_logging.py | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/tests/framework/project/test_logging.py b/tests/framework/project/test_logging.py index 3ce6020d65..c2a55559fa 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 fmt_rich, has_rich_handler @pytest.fixture @@ -144,3 +145,17 @@ def test_rich_traceback_disabled_on_databricks( rich_traceback_install.assert_not_called() 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) From 931235ea75e0673b0164472823a7d935019f94bc Mon Sep 17 00:00:00 2001 From: Simon Brugman Date: Mon, 4 Mar 2024 19:09:46 +0100 Subject: [PATCH 06/17] Update test_logging.py --- tests/framework/project/test_logging.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/framework/project/test_logging.py b/tests/framework/project/test_logging.py index c2a55559fa..df77655801 100644 --- a/tests/framework/project/test_logging.py +++ b/tests/framework/project/test_logging.py @@ -6,7 +6,7 @@ import yaml from kedro.framework.project import LOGGING, configure_logging, configure_project -from kedro.logging import fmt_rich, has_rich_handler +from kedro.logging import fmt_rich, has_rich_handler, RichHandler @pytest.fixture From c7bb278211a899c18ab33649eabe03106e74aad8 Mon Sep 17 00:00:00 2001 From: Simon Brugman Date: Mon, 4 Mar 2024 19:26:22 +0100 Subject: [PATCH 07/17] Lint --- tests/framework/project/test_logging.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/framework/project/test_logging.py b/tests/framework/project/test_logging.py index df77655801..c841dead1e 100644 --- a/tests/framework/project/test_logging.py +++ b/tests/framework/project/test_logging.py @@ -6,7 +6,7 @@ import yaml from kedro.framework.project import LOGGING, configure_logging, configure_project -from kedro.logging import fmt_rich, has_rich_handler, RichHandler +from kedro.logging import RichHandler, fmt_rich, has_rich_handler @pytest.fixture From b72677a88051ac0d97e41417c8d0059a13d141ae Mon Sep 17 00:00:00 2001 From: Simon Brugman Date: Fri, 8 Mar 2024 11:45:53 +0100 Subject: [PATCH 08/17] Cache `has_rich_logger` --- kedro/logging.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/kedro/logging.py b/kedro/logging.py index fa5f6ab9d9..9e7181585d 100644 --- a/kedro/logging.py +++ b/kedro/logging.py @@ -4,6 +4,7 @@ import logging import sys +from functools import cache from pathlib import Path from typing import Any @@ -60,6 +61,7 @@ def __init__(self, *args: Any, **kwargs: Any): rich.traceback.install(**traceback_install_kwargs) # type: ignore[arg-type] +@cache 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) From 6caf7e84d66ef5dcb8885a6c81f1957006159538 Mon Sep 17 00:00:00 2001 From: Simon Brugman Date: Fri, 8 Mar 2024 11:49:54 +0100 Subject: [PATCH 09/17] Backwards compatibility 3.8 --- kedro/logging.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/kedro/logging.py b/kedro/logging.py index 9e7181585d..13b6174c77 100644 --- a/kedro/logging.py +++ b/kedro/logging.py @@ -4,7 +4,7 @@ import logging import sys -from functools import cache +from functools import lru_cache from pathlib import Path from typing import Any @@ -61,7 +61,8 @@ def __init__(self, *args: Any, **kwargs: Any): rich.traceback.install(**traceback_install_kwargs) # type: ignore[arg-type] -@cache +# @cache is supported from 3.9 onwards +@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) From 802ad2607f27c5e68ec11b5ca3dbe5c03995230b Mon Sep 17 00:00:00 2001 From: Simon Brugman Date: Tue, 28 May 2024 12:58:48 +0200 Subject: [PATCH 10/17] Update test_logging.py --- tests/framework/project/test_logging.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/framework/project/test_logging.py b/tests/framework/project/test_logging.py index 3fa6a20709..90178a8a54 100644 --- a/tests/framework/project/test_logging.py +++ b/tests/framework/project/test_logging.py @@ -161,7 +161,7 @@ def test_has_rich_handler(): assert has_rich_handler(test_logger) - def test_default_logging_info_emission(monkeypatch, capsys): +def test_default_logging_info_emission(monkeypatch, capsys): # Expected path and logging configuration expected_path = Path("conf/logging.yml") dummy_logging_config = yaml.dump( From e2908719bb75d7371e3c109d4a94f8b3f7d50475 Mon Sep 17 00:00:00 2001 From: lrcouto Date: Tue, 2 Jul 2024 15:25:39 -0300 Subject: [PATCH 11/17] Revert "Update test_logging.py" This reverts commit 802ad2607f27c5e68ec11b5ca3dbe5c03995230b. --- tests/framework/project/test_logging.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/framework/project/test_logging.py b/tests/framework/project/test_logging.py index ef08d8b9e1..858ba4c850 100644 --- a/tests/framework/project/test_logging.py +++ b/tests/framework/project/test_logging.py @@ -163,7 +163,7 @@ def test_has_rich_handler(): assert has_rich_handler(test_logger) -def test_default_logging_info_emission(monkeypatch, capsys): + def test_default_logging_info_emission(monkeypatch, capsys): # Expected path and logging configuration expected_path = Path("conf/logging.yml") dummy_logging_config = yaml.dump( From 19e17fea5bc527326cff8c4a33a86708307c0e0d Mon Sep 17 00:00:00 2001 From: lrcouto Date: Tue, 2 Jul 2024 15:33:13 -0300 Subject: [PATCH 12/17] Revert merge Signed-off-by: lrcouto --- tests/framework/project/test_logging.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/framework/project/test_logging.py b/tests/framework/project/test_logging.py index 858ba4c850..a23f75884a 100644 --- a/tests/framework/project/test_logging.py +++ b/tests/framework/project/test_logging.py @@ -181,7 +181,7 @@ def test_default_logging_info_emission(monkeypatch, capsys): 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) From d3dadfc3b9d3e91d8dc02eb10c9af13a4280eb04 Mon Sep 17 00:00:00 2001 From: lrcouto Date: Tue, 2 Jul 2024 16:13:41 -0300 Subject: [PATCH 13/17] revert test_logging.py to commit 802ad26 Signed-off-by: lrcouto --- tests/framework/project/test_logging.py | 36 +++++++++---------------- 1 file changed, 13 insertions(+), 23 deletions(-) diff --git a/tests/framework/project/test_logging.py b/tests/framework/project/test_logging.py index a23f75884a..90178a8a54 100644 --- a/tests/framework/project/test_logging.py +++ b/tests/framework/project/test_logging.py @@ -36,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 @@ -47,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(): @@ -163,7 +161,7 @@ def test_has_rich_handler(): assert has_rich_handler(test_logger) - def test_default_logging_info_emission(monkeypatch, capsys): +def test_default_logging_info_emission(monkeypatch, capsys): # Expected path and logging configuration expected_path = Path("conf/logging.yml") dummy_logging_config = yaml.dump( @@ -181,21 +179,13 @@ def test_default_logging_info_emission(monkeypatch, capsys): 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) - logging_config = {"version": 1, "loggers": {"kedro": {"level": "DEBUG"}}} - with config_path.open("w", encoding="utf-8") as f: - yaml.dump(logging_config, f) - import os - from kedro.framework.project import _ProjectLogging - os.chdir(tmp_path) - LOGGING = _ProjectLogging() + _ProjectLogging() - assert LOGGING.data == logging_config - assert logging.getLogger("kedro").level == logging.DEBUG - expected_message = "You can change this by setting the KEDRO_LOGGING_CONFIG environment variable accordingly." - assert expected_message in "".join(caplog.messages).strip("\n") + captured = capsys.readouterr() + + expected_message = f"Using `{expected_path}`" + assert ( + expected_message in captured.out + ), f"Expected message not found in logs: {captured.out}" From d14576c4fa5a753dbdd71877637db17fb5a6fd53 Mon Sep 17 00:00:00 2001 From: lrcouto Date: Thu, 4 Jul 2024 02:37:23 -0300 Subject: [PATCH 14/17] Fix test, apply small fixes Signed-off-by: lrcouto --- kedro/io/data_catalog.py | 10 +++++++--- kedro/logging.py | 5 ++--- tests/framework/project/test_logging.py | 9 +++++---- 3 files changed, 14 insertions(+), 10 deletions(-) diff --git a/kedro/io/data_catalog.py b/kedro/io/data_catalog.py index 3cad97b155..6b71a918ef 100644 --- a/kedro/io/data_catalog.py +++ b/kedro/io/data_catalog.py @@ -24,7 +24,7 @@ generate_timestamp, ) from kedro.io.memory_dataset import MemoryDataset -from kedro.logging import fmt_rich, has_rich_handler +from kedro.logging import _format_rich, _has_rich_handler Patterns = Dict[str, Dict[str, Any]] @@ -508,7 +508,9 @@ def load(self, name: str, version: str | None = None) -> Any: self._logger.info( "Loading data from %s (%s)...", - fmt_rich(name, "dark_orange") if has_rich_handler(self._logger) else name, + _format_rich(name, "dark_orange") + if _has_rich_handler(self._logger) + else name, type(dataset).__name__, extra={"markup": True}, ) @@ -550,7 +552,9 @@ def save(self, name: str, data: Any) -> None: self._logger.info( "Saving data to %s (%s)...", - fmt_rich(name, "dark_orange") if has_rich_handler(self._logger) else name, + _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 958717d3ba..d31ac41945 100644 --- a/kedro/logging.py +++ b/kedro/logging.py @@ -57,13 +57,12 @@ def __init__(self, *args: Any, **kwargs: Any): rich.traceback.install(**traceback_install_kwargs) # type: ignore[arg-type] -# @cache is supported from 3.9 onwards @lru_cache(maxsize=None) -def has_rich_handler(logger: logging.Logger) -> bool: +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 fmt_rich(value: str, markup: str) -> str: +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 90178a8a54..824ea69df4 100644 --- a/tests/framework/project/test_logging.py +++ b/tests/framework/project/test_logging.py @@ -6,7 +6,7 @@ import yaml from kedro.framework.project import LOGGING, configure_logging, configure_project -from kedro.logging import RichHandler, fmt_rich, has_rich_handler +from kedro.logging import RichHandler, _format_rich, _has_rich_handler @pytest.fixture @@ -149,16 +149,17 @@ def test_rich_traceback_disabled_on_databricks( def test_rich_format(): assert ( - fmt_rich("Hello World!", "dark_orange") + _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) + assert not _has_rich_handler(test_logger) + _has_rich_handler.cache_clear() test_logger.addHandler(RichHandler()) - assert has_rich_handler(test_logger) + assert _has_rich_handler(test_logger) def test_default_logging_info_emission(monkeypatch, capsys): From 505a593ede57926902517e6f3dac55854893d193 Mon Sep 17 00:00:00 2001 From: lrcouto Date: Thu, 4 Jul 2024 02:50:08 -0300 Subject: [PATCH 15/17] Fix non-matching character in expected_message Signed-off-by: lrcouto --- tests/framework/project/test_logging.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/framework/project/test_logging.py b/tests/framework/project/test_logging.py index 824ea69df4..290bc61adf 100644 --- a/tests/framework/project/test_logging.py +++ b/tests/framework/project/test_logging.py @@ -185,8 +185,8 @@ def test_default_logging_info_emission(monkeypatch, capsys): _ProjectLogging() captured = capsys.readouterr() - - expected_message = f"Using `{expected_path}`" + print(captured.out) + expected_message = f"Using '{expected_path}'" assert ( expected_message in captured.out ), f"Expected message not found in logs: {captured.out}" From 956ceea8c6f25556129f00c2a17d7cf98561733a Mon Sep 17 00:00:00 2001 From: lrcouto Date: Thu, 4 Jul 2024 03:08:38 -0300 Subject: [PATCH 16/17] Remove leftover print Signed-off-by: lrcouto --- tests/framework/project/test_logging.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/framework/project/test_logging.py b/tests/framework/project/test_logging.py index 290bc61adf..30eb74045d 100644 --- a/tests/framework/project/test_logging.py +++ b/tests/framework/project/test_logging.py @@ -185,7 +185,6 @@ def test_default_logging_info_emission(monkeypatch, capsys): _ProjectLogging() captured = capsys.readouterr() - print(captured.out) expected_message = f"Using '{expected_path}'" assert ( expected_message in captured.out From e4968d364437984db0baaead01833fee3fad62b2 Mon Sep 17 00:00:00 2001 From: Nok Date: Fri, 5 Jul 2024 10:33:36 +0000 Subject: [PATCH 17/17] fix test Signed-off-by: Nok --- tests/framework/project/test_logging.py | 36 +++++++++---------------- 1 file changed, 13 insertions(+), 23 deletions(-) diff --git a/tests/framework/project/test_logging.py b/tests/framework/project/test_logging.py index 30eb74045d..7a90405ecb 100644 --- a/tests/framework/project/test_logging.py +++ b/tests/framework/project/test_logging.py @@ -162,30 +162,20 @@ def test_has_rich_handler(): 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_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"}}} + with config_path.open("w", encoding="utf-8") as f: + yaml.dump(logging_config, f) + import os from kedro.framework.project import _ProjectLogging - _ProjectLogging() + os.chdir(tmp_path) + LOGGING = _ProjectLogging() - captured = capsys.readouterr() - expected_message = f"Using '{expected_path}'" - assert ( - expected_message in captured.out - ), f"Expected message not found in logs: {captured.out}" + assert LOGGING.data == logging_config + assert logging.getLogger("kedro").level == logging.DEBUG + expected_message = "You can change this by setting the KEDRO_LOGGING_CONFIG environment variable accordingly." + assert expected_message in "".join(caplog.messages).strip("\n")