From e2a00f556ff5393c05c42e200ee9dfab033640e1 Mon Sep 17 00:00:00 2001 From: Merel Theisen Date: Thu, 5 Jan 2023 13:49:06 +0000 Subject: [PATCH 1/5] Only convert omegaconf DictConfig to primitive type for logging Signed-off-by: Merel Theisen --- kedro/config/omegaconf_config.py | 5 ++--- kedro/framework/session/session.py | 3 ++- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/kedro/config/omegaconf_config.py b/kedro/config/omegaconf_config.py index 3b4f9c2be6..ce4efe386f 100644 --- a/kedro/config/omegaconf_config.py +++ b/kedro/config/omegaconf_config.py @@ -233,9 +233,8 @@ def load_and_merge_dir_config(self, conf_path: str, patterns: Iterable[str]): if aggregate_config: if len(aggregate_config) > 1: - merged_config = OmegaConf.merge(*aggregate_config) - return OmegaConf.to_container(merged_config) - return OmegaConf.to_container(list(aggregate_config)[0]) + return dict(OmegaConf.merge(*aggregate_config)) + return list(aggregate_config)[0] return {} @staticmethod diff --git a/kedro/framework/session/session.py b/kedro/framework/session/session.py index 6b751b910c..e32f4458b1 100644 --- a/kedro/framework/session/session.py +++ b/kedro/framework/session/session.py @@ -10,6 +10,7 @@ from typing import Any, Dict, Iterable, Optional, Union import click +from omegaconf import OmegaConf from kedro import __version__ as kedro_version from kedro.config import ConfigLoader, MissingConfigException @@ -191,7 +192,7 @@ def create( # pylint: disable=too-many-arguments return session def _get_logging_config(self) -> Dict[str, Any]: - logging_config = self._get_config_loader()["logging"] + logging_config = OmegaConf.to_container(self._get_config_loader()["logging"]) # turn relative paths in logging config into absolute path # before initialising loggers logging_config = _convert_paths_to_absolute_posix( From ab003c1bb1dfc86e3164286882986142293e8a8a Mon Sep 17 00:00:00 2001 From: Merel Theisen Date: Thu, 5 Jan 2023 14:10:27 +0000 Subject: [PATCH 2/5] Check if dealing with omegaconf dict before converting Signed-off-by: Merel Theisen --- kedro/framework/session/session.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/kedro/framework/session/session.py b/kedro/framework/session/session.py index e32f4458b1..a435ffd582 100644 --- a/kedro/framework/session/session.py +++ b/kedro/framework/session/session.py @@ -10,7 +10,7 @@ from typing import Any, Dict, Iterable, Optional, Union import click -from omegaconf import OmegaConf +from omegaconf import OmegaConf, omegaconf from kedro import __version__ as kedro_version from kedro.config import ConfigLoader, MissingConfigException @@ -192,7 +192,9 @@ def create( # pylint: disable=too-many-arguments return session def _get_logging_config(self) -> Dict[str, Any]: - logging_config = OmegaConf.to_container(self._get_config_loader()["logging"]) + logging_config = self._get_config_loader()["logging"] + if isinstance(logging_config, omegaconf.DictConfig): + logging_config = OmegaConf.to_container(logging_config) # turn relative paths in logging config into absolute path # before initialising loggers logging_config = _convert_paths_to_absolute_posix( From 67ff24d0ead127c389bee181ce5828f20375a448 Mon Sep 17 00:00:00 2001 From: Merel Theisen Date: Thu, 5 Jan 2023 14:33:27 +0000 Subject: [PATCH 3/5] Add test to check logging file is parsed correctly with omegaconfloader Signed-off-by: Merel Theisen --- tests/framework/session/test_session.py | 29 ++++++++++++++++++++++++- 1 file changed, 28 insertions(+), 1 deletion(-) diff --git a/tests/framework/session/test_session.py b/tests/framework/session/test_session.py index af8c734179..24b6652be2 100644 --- a/tests/framework/session/test_session.py +++ b/tests/framework/session/test_session.py @@ -9,7 +9,7 @@ import yaml from kedro import __version__ as kedro_version -from kedro.config import AbstractConfigLoader, ConfigLoader +from kedro.config import AbstractConfigLoader, ConfigLoader, OmegaConfLoader from kedro.framework.context import KedroContext from kedro.framework.project import ( ValidationError, @@ -91,6 +91,16 @@ class MockSettings(_ProjectSettings): return _mock_imported_settings_paths(mocker, MockSettings()) +@pytest.fixture +def mock_settings_omega_config_loader_class(mocker): + class MockSettings(_ProjectSettings): + _CONFIG_LOADER_CLASS = _HasSharedParentClassValidator( + "CONFIG_LOADER_CLASS", default=lambda *_: OmegaConfLoader + ) + + return _mock_imported_settings_paths(mocker, MockSettings()) + + @pytest.fixture def mock_settings_config_loader_args(mocker): class MockSettings(_ProjectSettings): @@ -889,3 +899,20 @@ def test_setup_logging_using_absolute_path( ).as_posix() actual_log_filepath = call_args["handlers"]["info_file_handler"]["filename"] assert actual_log_filepath == expected_log_filepath + + +@pytest.mark.usefixtures("mock_settings_omega_config_loader_class") +def test_setup_logging_using_omega_config_loader_class( + fake_project_with_logging_file_handler, mocker, mock_package_name +): + mocked_logging = mocker.patch("logging.config.dictConfig") + KedroSession.create(mock_package_name, fake_project_with_logging_file_handler) + + mocked_logging.assert_called_once() + call_args = mocked_logging.call_args[0][0] + + expected_log_filepath = ( + fake_project_with_logging_file_handler / "logs" / "info.log" + ).as_posix() + actual_log_filepath = call_args["handlers"]["info_file_handler"]["filename"] + assert actual_log_filepath == expected_log_filepath From f8172a23e4db4ac5f36a3544ab84749221ce437f Mon Sep 17 00:00:00 2001 From: Merel Theisen Date: Tue, 10 Jan 2023 10:27:54 +0000 Subject: [PATCH 4/5] Address review suggestion Signed-off-by: Merel Theisen --- kedro/config/omegaconf_config.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/kedro/config/omegaconf_config.py b/kedro/config/omegaconf_config.py index ce4efe386f..5d954c19af 100644 --- a/kedro/config/omegaconf_config.py +++ b/kedro/config/omegaconf_config.py @@ -231,11 +231,11 @@ def load_and_merge_dir_config(self, conf_path: str, patterns: Iterable[str]): aggregate_config = config_per_file.values() self._check_duplicates(seen_file_to_keys) - if aggregate_config: - if len(aggregate_config) > 1: - return dict(OmegaConf.merge(*aggregate_config)) + if not aggregate_config: + return {} + if len(aggregate_config) == 1: return list(aggregate_config)[0] - return {} + return dict(OmegaConf.merge(*aggregate_config)) @staticmethod def _is_valid_config_path(path): From e87c32039809634900bb90ab3b71ef774b810120 Mon Sep 17 00:00:00 2001 From: Merel Theisen Date: Tue, 10 Jan 2023 10:42:25 +0000 Subject: [PATCH 5/5] Fix lint Signed-off-by: Merel Theisen --- docs/source/hooks/examples.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/source/hooks/examples.md b/docs/source/hooks/examples.md index 9b2d490cce..f5008da75e 100644 --- a/docs/source/hooks/examples.md +++ b/docs/source/hooks/examples.md @@ -178,7 +178,7 @@ class DataValidationHooks: great_expectations checkpoint new raw_companies_dataset_checkpoint ``` -* Remove `data_connector_query` from the `batch_request` in the checkpoint config file: +* Remove `data_connector_query` from the `batch_request` in the checkpoint config file: ```python yaml_config = f""" @@ -249,7 +249,7 @@ class DataValidationHooks: }, "batch_identifiers": { "runtime_batch_identifier_name": dataset_name - } + }, }, run_name=session_id, )