From 957e8e4f649b310222c6dcf98d2c08f1989fb57e Mon Sep 17 00:00:00 2001 From: noklam Date: Tue, 12 Apr 2022 15:48:58 +0100 Subject: [PATCH 1/8] Removed nested session test and add tests to make sure multiple sessions can be created. Signed-off-by: noklam --- kedro/framework/session/session.py | 2 +- tests/framework/session/test_session.py | 28 ++++++++++++------------- 2 files changed, 15 insertions(+), 15 deletions(-) diff --git a/kedro/framework/session/session.py b/kedro/framework/session/session.py index c40a7b112d..1f92c49d2a 100644 --- a/kedro/framework/session/session.py +++ b/kedro/framework/session/session.py @@ -34,7 +34,7 @@ def _activate_session(session: "KedroSession", force: bool = False) -> None: global _active_session - if _active_session and not force and session is not _active_session: + if not force and session is not _active_session: raise RuntimeError( "Cannot activate the session as another active session already exists." ) diff --git a/tests/framework/session/test_session.py b/tests/framework/session/test_session.py index 2a559ffe70..b25ea84e70 100644 --- a/tests/framework/session/test_session.py +++ b/tests/framework/session/test_session.py @@ -8,6 +8,7 @@ import pytest import toml +import kedro from kedro import __version__ as kedro_version from kedro.config import ConfigLoader from kedro.framework.context import KedroContext @@ -16,9 +17,9 @@ Validator, _IsSubclassValidator, _ProjectSettings, - configure_project, ) from kedro.framework.session import KedroSession +from kedro.framework.session.session import _activate_session from kedro.framework.session.store import BaseSessionStore, ShelveStore _FAKE_PROJECT_NAME = "fake_project" @@ -284,6 +285,18 @@ def test_create( assert session.load_context() is mock_context_class.return_value assert isinstance(session._get_config_loader(), ConfigLoader) + @pytest.mark.usefixtures("mock_settings") + def test_create_session_with_unclosed_session( + self, fake_project, mock_package_name + ): + first_session = KedroSession.create(mock_package_name, fake_project) + _activate_session(first_session) + assert kedro.framework.session.session._active_session is first_session + + new_session = KedroSession.create(mock_package_name, fake_project) + _activate_session(new_session) + assert kedro.framework.session.session._active_session is new_session + @pytest.mark.usefixtures("mock_settings_context_class") def test_create_no_env_extra_params( self, @@ -518,19 +531,6 @@ def test_log_error(self, fake_project, mock_package_name): "raise FakeException" in tb_line for tb_line in exception["traceback"] ) - @pytest.mark.usefixtures("mock_settings") - def test_nested_sessions(self, fake_project, mock_package_name): - configure_project(mock_package_name) - session1 = KedroSession.create(mock_package_name, fake_project) - session2 = KedroSession.create(mock_package_name, fake_project) - - with session1: - pattern = ( - "Cannot activate the session as another active session already exists" - ) - with pytest.raises(RuntimeError, match=pattern), session2: - pass # pragma: no cover - @pytest.mark.usefixtures("mock_settings_context_class") @pytest.mark.parametrize("fake_pipeline_name", [None, _FAKE_PIPELINE_NAME]) def test_run( From dc422f3f3f4b8119b87f40e98bb252b5979d3163 Mon Sep 17 00:00:00 2001 From: noklam Date: Tue, 12 Apr 2022 16:21:57 +0100 Subject: [PATCH 2/8] Remove _active_session global and activate/deactivate mechanism. Signed-off-by: noklam --- kedro/extras/extensions/ipython.py | 2 -- kedro/framework/session/session.py | 24 ------------------------ tests/extras/extensions/test_ipython.py | 2 -- tests/framework/session/test_session.py | 16 ++++------------ 4 files changed, 4 insertions(+), 40 deletions(-) diff --git a/kedro/extras/extensions/ipython.py b/kedro/extras/extensions/ipython.py index b29746b271..19a57fec85 100644 --- a/kedro/extras/extensions/ipython.py +++ b/kedro/extras/extensions/ipython.py @@ -44,7 +44,6 @@ def reload_kedro( from kedro.framework.cli import load_entry_points from kedro.framework.project import configure_project, pipelines from kedro.framework.session import KedroSession - from kedro.framework.session.session import _activate_session from kedro.framework.startup import bootstrap_project # If a path is provided, set it as default for subsequent calls @@ -63,7 +62,6 @@ def reload_kedro( session = KedroSession.create( metadata.package_name, default_project_path, env=env, extra_params=extra_params ) - _activate_session(session, force=True) logging.debug("Loading the context from %s", default_project_path) context = session.load_context() catalog = context.catalog diff --git a/kedro/framework/session/session.py b/kedro/framework/session/session.py index 1f92c49d2a..8179354412 100644 --- a/kedro/framework/session/session.py +++ b/kedro/framework/session/session.py @@ -28,24 +28,6 @@ from kedro.io.core import generate_timestamp from kedro.runner import AbstractRunner, SequentialRunner -_active_session = None - - -def _activate_session(session: "KedroSession", force: bool = False) -> None: - global _active_session - - if not force and session is not _active_session: - raise RuntimeError( - "Cannot activate the session as another active session already exists." - ) - - _active_session = session - - -def _deactivate_session() -> None: - global _active_session - _active_session = None - def _describe_git(project_path: Path) -> Dict[str, Dict[str, Any]]: project_path = str(project_path) @@ -278,18 +260,12 @@ def close(self): if self.save_on_close: self._store.save() - if _active_session is self: - _deactivate_session() - def __enter__(self): - if _active_session is not self: - _activate_session(self) return self def __exit__(self, exc_type, exc_value, tb_): if exc_type: self._log_exception(exc_type, exc_value, tb_) - self.close() def run( # pylint: disable=too-many-arguments,too-many-locals self, diff --git a/tests/extras/extensions/test_ipython.py b/tests/extras/extensions/test_ipython.py index 6771c04b01..f343538393 100644 --- a/tests/extras/extensions/test_ipython.py +++ b/tests/extras/extensions/test_ipython.py @@ -2,7 +2,6 @@ import pytest from kedro.extras.extensions.ipython import load_ipython_extension, reload_kedro -from kedro.framework.session.session import _deactivate_session from kedro.framework.startup import ProjectMetadata from kedro.pipeline import Pipeline @@ -23,7 +22,6 @@ def mocked_logging(mocker): @pytest.fixture(autouse=True) def cleanup_session(): yield - _deactivate_session() @pytest.fixture() diff --git a/tests/framework/session/test_session.py b/tests/framework/session/test_session.py index b25ea84e70..34debe9289 100644 --- a/tests/framework/session/test_session.py +++ b/tests/framework/session/test_session.py @@ -8,7 +8,6 @@ import pytest import toml -import kedro from kedro import __version__ as kedro_version from kedro.config import ConfigLoader from kedro.framework.context import KedroContext @@ -19,7 +18,6 @@ _ProjectSettings, ) from kedro.framework.session import KedroSession -from kedro.framework.session.session import _activate_session from kedro.framework.session.store import BaseSessionStore, ShelveStore _FAKE_PROJECT_NAME = "fake_project" @@ -286,16 +284,10 @@ def test_create( assert isinstance(session._get_config_loader(), ConfigLoader) @pytest.mark.usefixtures("mock_settings") - def test_create_session_with_unclosed_session( - self, fake_project, mock_package_name - ): - first_session = KedroSession.create(mock_package_name, fake_project) - _activate_session(first_session) - assert kedro.framework.session.session._active_session is first_session - - new_session = KedroSession.create(mock_package_name, fake_project) - _activate_session(new_session) - assert kedro.framework.session.session._active_session is new_session + def test_create_multiple_sessions(self, fake_project, mock_package_name): + KedroSession.create(mock_package_name, fake_project) + with KedroSession.create(mock_package_name, fake_project): + pass @pytest.mark.usefixtures("mock_settings_context_class") def test_create_no_env_extra_params( From 1d90518698307a6651bbfb66918406072121c021 Mon Sep 17 00:00:00 2001 From: noklam Date: Thu, 14 Apr 2022 16:30:31 +0100 Subject: [PATCH 3/8] Revert "Remove _active_session global and activate/deactivate mechanism." This reverts commit dc422f3f3f4b8119b87f40e98bb252b5979d3163. Signed-off-by: noklam --- kedro/extras/extensions/ipython.py | 2 ++ kedro/framework/session/session.py | 24 ++++++++++++++++++++++++ tests/extras/extensions/test_ipython.py | 2 ++ tests/framework/session/test_session.py | 16 ++++++++++++---- 4 files changed, 40 insertions(+), 4 deletions(-) diff --git a/kedro/extras/extensions/ipython.py b/kedro/extras/extensions/ipython.py index 19a57fec85..b29746b271 100644 --- a/kedro/extras/extensions/ipython.py +++ b/kedro/extras/extensions/ipython.py @@ -44,6 +44,7 @@ def reload_kedro( from kedro.framework.cli import load_entry_points from kedro.framework.project import configure_project, pipelines from kedro.framework.session import KedroSession + from kedro.framework.session.session import _activate_session from kedro.framework.startup import bootstrap_project # If a path is provided, set it as default for subsequent calls @@ -62,6 +63,7 @@ def reload_kedro( session = KedroSession.create( metadata.package_name, default_project_path, env=env, extra_params=extra_params ) + _activate_session(session, force=True) logging.debug("Loading the context from %s", default_project_path) context = session.load_context() catalog = context.catalog diff --git a/kedro/framework/session/session.py b/kedro/framework/session/session.py index 8179354412..1f92c49d2a 100644 --- a/kedro/framework/session/session.py +++ b/kedro/framework/session/session.py @@ -28,6 +28,24 @@ from kedro.io.core import generate_timestamp from kedro.runner import AbstractRunner, SequentialRunner +_active_session = None + + +def _activate_session(session: "KedroSession", force: bool = False) -> None: + global _active_session + + if not force and session is not _active_session: + raise RuntimeError( + "Cannot activate the session as another active session already exists." + ) + + _active_session = session + + +def _deactivate_session() -> None: + global _active_session + _active_session = None + def _describe_git(project_path: Path) -> Dict[str, Dict[str, Any]]: project_path = str(project_path) @@ -260,12 +278,18 @@ def close(self): if self.save_on_close: self._store.save() + if _active_session is self: + _deactivate_session() + def __enter__(self): + if _active_session is not self: + _activate_session(self) return self def __exit__(self, exc_type, exc_value, tb_): if exc_type: self._log_exception(exc_type, exc_value, tb_) + self.close() def run( # pylint: disable=too-many-arguments,too-many-locals self, diff --git a/tests/extras/extensions/test_ipython.py b/tests/extras/extensions/test_ipython.py index f343538393..6771c04b01 100644 --- a/tests/extras/extensions/test_ipython.py +++ b/tests/extras/extensions/test_ipython.py @@ -2,6 +2,7 @@ import pytest from kedro.extras.extensions.ipython import load_ipython_extension, reload_kedro +from kedro.framework.session.session import _deactivate_session from kedro.framework.startup import ProjectMetadata from kedro.pipeline import Pipeline @@ -22,6 +23,7 @@ def mocked_logging(mocker): @pytest.fixture(autouse=True) def cleanup_session(): yield + _deactivate_session() @pytest.fixture() diff --git a/tests/framework/session/test_session.py b/tests/framework/session/test_session.py index 34debe9289..b25ea84e70 100644 --- a/tests/framework/session/test_session.py +++ b/tests/framework/session/test_session.py @@ -8,6 +8,7 @@ import pytest import toml +import kedro from kedro import __version__ as kedro_version from kedro.config import ConfigLoader from kedro.framework.context import KedroContext @@ -18,6 +19,7 @@ _ProjectSettings, ) from kedro.framework.session import KedroSession +from kedro.framework.session.session import _activate_session from kedro.framework.session.store import BaseSessionStore, ShelveStore _FAKE_PROJECT_NAME = "fake_project" @@ -284,10 +286,16 @@ def test_create( assert isinstance(session._get_config_loader(), ConfigLoader) @pytest.mark.usefixtures("mock_settings") - def test_create_multiple_sessions(self, fake_project, mock_package_name): - KedroSession.create(mock_package_name, fake_project) - with KedroSession.create(mock_package_name, fake_project): - pass + def test_create_session_with_unclosed_session( + self, fake_project, mock_package_name + ): + first_session = KedroSession.create(mock_package_name, fake_project) + _activate_session(first_session) + assert kedro.framework.session.session._active_session is first_session + + new_session = KedroSession.create(mock_package_name, fake_project) + _activate_session(new_session) + assert kedro.framework.session.session._active_session is new_session @pytest.mark.usefixtures("mock_settings_context_class") def test_create_no_env_extra_params( From d533bfb06039a464f02771e4764390b0e2671095 Mon Sep 17 00:00:00 2001 From: noklam Date: Tue, 19 Apr 2022 13:42:04 +0100 Subject: [PATCH 4/8] remove session checking Signed-off-by: noklam --- kedro/framework/session/session.py | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/kedro/framework/session/session.py b/kedro/framework/session/session.py index 6d895b97e9..47b12d38b8 100644 --- a/kedro/framework/session/session.py +++ b/kedro/framework/session/session.py @@ -31,14 +31,10 @@ _active_session = None -def _activate_session(session: "KedroSession", force: bool = False) -> None: +def _activate_session( + session: "KedroSession", force: bool = False # pylint: disable=unused-argument +) -> None: global _active_session - - if not force and session is not _active_session: - raise RuntimeError( - "Cannot activate the session as another active session already exists." - ) - _active_session = session From c5422552ded7152afe71c2f382d3ebe17e9c0fd0 Mon Sep 17 00:00:00 2001 From: noklam Date: Wed, 27 Apr 2022 12:21:52 +0100 Subject: [PATCH 5/8] Clean up activate deactivate session Signed-off-by: noklam --- RELEASE.md | 1 + kedro/extras/extensions/ipython.py | 2 -- kedro/framework/session/session.py | 19 ------------------- tests/extras/extensions/test_ipython.py | 2 -- tests/framework/session/test_session.py | 16 ++++------------ 5 files changed, 5 insertions(+), 35 deletions(-) diff --git a/RELEASE.md b/RELEASE.md index 3cd2a2712e..74d0f61ddf 100644 --- a/RELEASE.md +++ b/RELEASE.md @@ -22,6 +22,7 @@ * Introduced `after_command_run` CLI hook. * Update sections on visualisation, namespacing, and experiment tracking in the spaceflight tutorial to correspond to the complete spaceflights starter. * Fixed `Jinja2` syntax loading with `TemplatedConfigLoader` using `globals.yml`. +* Deprecated global `_active_session`, `_activate_session and `_deactivate_session` ## Upcoming deprecations for Kedro 0.19.0 diff --git a/kedro/extras/extensions/ipython.py b/kedro/extras/extensions/ipython.py index b29746b271..19a57fec85 100644 --- a/kedro/extras/extensions/ipython.py +++ b/kedro/extras/extensions/ipython.py @@ -44,7 +44,6 @@ def reload_kedro( from kedro.framework.cli import load_entry_points from kedro.framework.project import configure_project, pipelines from kedro.framework.session import KedroSession - from kedro.framework.session.session import _activate_session from kedro.framework.startup import bootstrap_project # If a path is provided, set it as default for subsequent calls @@ -63,7 +62,6 @@ def reload_kedro( session = KedroSession.create( metadata.package_name, default_project_path, env=env, extra_params=extra_params ) - _activate_session(session, force=True) logging.debug("Loading the context from %s", default_project_path) context = session.load_context() catalog = context.catalog diff --git a/kedro/framework/session/session.py b/kedro/framework/session/session.py index 1cc7f822a4..1618438a09 100644 --- a/kedro/framework/session/session.py +++ b/kedro/framework/session/session.py @@ -28,20 +28,6 @@ from kedro.io.core import generate_timestamp from kedro.runner import AbstractRunner, SequentialRunner -_active_session = None - - -def _activate_session( - session: "KedroSession", force: bool = False # pylint: disable=unused-argument -) -> None: - global _active_session - _active_session = session - - -def _deactivate_session() -> None: - global _active_session - _active_session = None - def _describe_git(project_path: Path) -> Dict[str, Dict[str, Any]]: project_path = str(project_path) @@ -278,12 +264,7 @@ def close(self): if self.save_on_close: self._store.save() - if _active_session is self: - _deactivate_session() - def __enter__(self): - if _active_session is not self: - _activate_session(self) return self def __exit__(self, exc_type, exc_value, tb_): diff --git a/tests/extras/extensions/test_ipython.py b/tests/extras/extensions/test_ipython.py index 6771c04b01..f343538393 100644 --- a/tests/extras/extensions/test_ipython.py +++ b/tests/extras/extensions/test_ipython.py @@ -2,7 +2,6 @@ import pytest from kedro.extras.extensions.ipython import load_ipython_extension, reload_kedro -from kedro.framework.session.session import _deactivate_session from kedro.framework.startup import ProjectMetadata from kedro.pipeline import Pipeline @@ -23,7 +22,6 @@ def mocked_logging(mocker): @pytest.fixture(autouse=True) def cleanup_session(): yield - _deactivate_session() @pytest.fixture() diff --git a/tests/framework/session/test_session.py b/tests/framework/session/test_session.py index 91ac55c176..1099f1e269 100644 --- a/tests/framework/session/test_session.py +++ b/tests/framework/session/test_session.py @@ -8,7 +8,6 @@ import pytest import toml -import kedro from kedro import __version__ as kedro_version from kedro.config import AbstractConfigLoader, ConfigLoader from kedro.framework.context import KedroContext @@ -20,7 +19,6 @@ _ProjectSettings, ) from kedro.framework.session import KedroSession -from kedro.framework.session.session import _activate_session from kedro.framework.session.store import BaseSessionStore, ShelveStore _FAKE_PROJECT_NAME = "fake_project" @@ -287,16 +285,10 @@ def test_create( assert isinstance(session._get_config_loader(), ConfigLoader) @pytest.mark.usefixtures("mock_settings") - def test_create_session_with_unclosed_session( - self, fake_project, mock_package_name - ): - first_session = KedroSession.create(mock_package_name, fake_project) - _activate_session(first_session) - assert kedro.framework.session.session._active_session is first_session - - new_session = KedroSession.create(mock_package_name, fake_project) - _activate_session(new_session) - assert kedro.framework.session.session._active_session is new_session + def test_create_multiple_sessions(self, fake_project, mock_package_name): + with KedroSession.create(mock_package_name, fake_project): + with KedroSession.create(mock_package_name, fake_project): + ... @pytest.mark.usefixtures("mock_settings_context_class") def test_create_no_env_extra_params( From 6e5da8c3add44666711b89b3f15ec212ea0062d4 Mon Sep 17 00:00:00 2001 From: Nok Date: Thu, 5 May 2022 17:18:47 +0100 Subject: [PATCH 6/8] Update RELEASE.md Co-authored-by: Antony Milne <49395058+AntonyMilneQB@users.noreply.github.com> --- RELEASE.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/RELEASE.md b/RELEASE.md index 74d0f61ddf..54bdea2134 100644 --- a/RELEASE.md +++ b/RELEASE.md @@ -22,7 +22,7 @@ * Introduced `after_command_run` CLI hook. * Update sections on visualisation, namespacing, and experiment tracking in the spaceflight tutorial to correspond to the complete spaceflights starter. * Fixed `Jinja2` syntax loading with `TemplatedConfigLoader` using `globals.yml`. -* Deprecated global `_active_session`, `_activate_session and `_deactivate_session` +* Removed global `_active_session`, `_activate_session` and `_deactivate_session`. Plugins that need to access objects such as the config loader should now do so through `context` in the new `after_context_created` hook. ## Upcoming deprecations for Kedro 0.19.0 From 5bbbc7d87c14b9f6eae3ab81c8710a3f43381f04 Mon Sep 17 00:00:00 2001 From: noklam Date: Mon, 9 May 2022 10:03:41 +0100 Subject: [PATCH 7/8] Fix merged conflict Signed-off-by: noklam --- kedro/extras/extensions/ipython.py | 1 - 1 file changed, 1 deletion(-) diff --git a/kedro/extras/extensions/ipython.py b/kedro/extras/extensions/ipython.py index 6b1bd18ca8..03dffb04cf 100644 --- a/kedro/extras/extensions/ipython.py +++ b/kedro/extras/extensions/ipython.py @@ -63,7 +63,6 @@ def reload_kedro( session = KedroSession.create( metadata.package_name, default_project_path, env=env, extra_params=extra_params ) - _activate_session(session, force=True) logger.debug("Loading the context from %s", default_project_path) context = session.load_context() catalog = context.catalog From 2be1e22220d27d5afbd40e016ecb04992a8f08ba Mon Sep 17 00:00:00 2001 From: noklam Date: Mon, 9 May 2022 11:37:33 +0100 Subject: [PATCH 8/8] Replace the ellipsis syntax with pass Signed-off-by: noklam --- tests/framework/session/test_session.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/framework/session/test_session.py b/tests/framework/session/test_session.py index 1099f1e269..f712489041 100644 --- a/tests/framework/session/test_session.py +++ b/tests/framework/session/test_session.py @@ -288,7 +288,7 @@ def test_create( def test_create_multiple_sessions(self, fake_project, mock_package_name): with KedroSession.create(mock_package_name, fake_project): with KedroSession.create(mock_package_name, fake_project): - ... + pass @pytest.mark.usefixtures("mock_settings_context_class") def test_create_no_env_extra_params(