From 28ca764a1b9f641af207e7fe4ff13ed15ff0aebc Mon Sep 17 00:00:00 2001 From: Merel Theisen Date: Tue, 26 Oct 2021 16:53:15 +0100 Subject: [PATCH 1/4] Include nested pipeline configuration in packaging --- kedro/framework/cli/pipeline.py | 3 +- .../cli/pipeline/test_pipeline_package.py | 46 +++++++++++++++++++ 2 files changed, 48 insertions(+), 1 deletion(-) diff --git a/kedro/framework/cli/pipeline.py b/kedro/framework/cli/pipeline.py index 2e3daa4b68..eec0cf703c 100644 --- a/kedro/framework/cli/pipeline.py +++ b/kedro/framework/cli/pipeline.py @@ -681,7 +681,7 @@ def _package_pipeline( # pylint: disable=too-many-arguments # config files not to confuse users and avoid useless file copies configs_to_package = _find_config_files( artifacts_to_package.pipeline_conf, - [f"parameters*/**/{pipeline_name}.yml", f"parameters*/**/{pipeline_name}/*"], + [f"parameters*/**/{pipeline_name}.yml", f"parameters*/**/{pipeline_name}/**/*"], ) source_paths = ( @@ -942,6 +942,7 @@ def _generate_setup_file( "config/**/parameters*", "config/parameters*/**", "config/parameters*/**/*", + "config/parameters*/**/**/*", ] } setup_file_context = dict( diff --git a/tests/framework/cli/pipeline/test_pipeline_package.py b/tests/framework/cli/pipeline/test_pipeline_package.py index 7bdf31306c..9854efeae7 100644 --- a/tests/framework/cli/pipeline/test_pipeline_package.py +++ b/tests/framework/cli/pipeline/test_pipeline_package.py @@ -312,6 +312,52 @@ def test_package_modular_pipeline_with_nested_parameters( assert "retail/config/parameters/retail.yml" in wheel_contents assert "retail/config/parameters/retail_banking.yml" not in wheel_contents + def test_package_pipeline_with_deep_nested_parameters( + self, fake_repo_path, fake_project_cli, fake_metadata + ): + CliRunner().invoke( + fake_project_cli, ["pipeline", "create", "retail"], obj=fake_metadata + ) + deep_nested_param_path = Path( + fake_repo_path / "conf" / "base" / "parameters" / "deep" / "retail" + ) + deep_nested_param_path.mkdir(parents=True, exist_ok=True) + (deep_nested_param_path / "params1.yml").touch() + + deep_nested_param_path2 = Path( + fake_repo_path / "conf" / "base" / "parameters" / "retail" / "deep" + ) + deep_nested_param_path2.mkdir(parents=True, exist_ok=True) + (deep_nested_param_path2 / "params1.yml").touch() + + deep_nested_param_path3 = Path( + fake_repo_path / "conf" / "base" / "parameters" / "deep" + ) + deep_nested_param_path3.mkdir(parents=True, exist_ok=True) + (deep_nested_param_path3 / "retail.yml").touch() + + result = CliRunner().invoke( + fake_project_cli, ["pipeline", "package", "retail"], obj=fake_metadata + ) + + assert result.exit_code == 0 + assert "Pipeline `retail` packaged!" in result.output + + wheel_location = fake_repo_path / "src" / "dist" + assert f"Location: {wheel_location}" in result.output + + wheel_name = _get_wheel_name(name="retail", version="0.1") + wheel_file = wheel_location / wheel_name + assert wheel_file.is_file() + assert len(list(wheel_location.iterdir())) == 1 + + # pylint: disable=consider-using-with + wheel_contents = set(ZipFile(str(wheel_file)).namelist()) + assert "retail/config/parameters/deep/retail/params1.yml" in wheel_contents + assert "retail/config/parameters/retail/deep/params1.yml" in wheel_contents + assert "retail/config/parameters/retail.yml" in wheel_contents + assert "retail/config/parameters/deep/retail.yml" in wheel_contents + def test_pipeline_package_version( self, fake_repo_path, fake_package_path, fake_project_cli, fake_metadata ): From 3d44b1067fa10f9b0748bdcee4c72d3044a831d9 Mon Sep 17 00:00:00 2001 From: Merel Theisen Date: Tue, 26 Oct 2021 17:16:22 +0100 Subject: [PATCH 2/4] Update release notes --- RELEASE.md | 1 + 1 file changed, 1 insertion(+) diff --git a/RELEASE.md b/RELEASE.md index f3c425b6be..13812814ab 100644 --- a/RELEASE.md +++ b/RELEASE.md @@ -20,6 +20,7 @@ * Relaxed the bounds on the `plotly` requirement for `plotly.PlotlyDataSet`. * `kedro pipeline package ` now raises an error if the `` argument doesn't look like a valid Python module path (e.g. has `/` instead of `.`). * Fixed slow startup because of catalog processing by reducing the exponential growth of extra processing during `_FrozenDatasets` creations. +* Fixed an issue where nested pipeline configuration was not included in the packaged pipeline. ## Minor breaking changes to the API From ccbf69d588425372514dc1c91a8fffbef8214ad2 Mon Sep 17 00:00:00 2001 From: Merel Theisen Date: Mon, 1 Nov 2021 11:32:51 +0000 Subject: [PATCH 3/4] Remove glob from setuptool matching and instead provide list of config files --- kedro/framework/cli/pipeline.py | 21 +++++++++------------ 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/kedro/framework/cli/pipeline.py b/kedro/framework/cli/pipeline.py index eec0cf703c..3a57332f9b 100644 --- a/kedro/framework/cli/pipeline.py +++ b/kedro/framework/cli/pipeline.py @@ -908,8 +908,9 @@ def _generate_wheel_file( cls = exc.__class__ raise KedroCliError(f"{cls.__module__}.{cls.__qualname__}: {exc}") from exc + config_files = [str(file) for file in conf_target.rglob("*") if file.is_file()] setup_file = _generate_setup_file( - package_name, version, install_requires, temp_dir_path + package_name, version, install_requires, temp_dir_path, config_files ) package_file = destination / _get_wheel_name(name=package_name, version=version) @@ -932,19 +933,15 @@ def _generate_wheel_file( def _generate_setup_file( - package_name: str, version: str, install_requires: List[str], output_dir: Path + package_name: str, + version: str, + install_requires: List[str], + output_dir: Path, + config_files: List[str], ) -> Path: setup_file = output_dir / "setup.py" - package_data = { - package_name: [ - "README.md", - "config/parameters*", - "config/**/parameters*", - "config/parameters*/**", - "config/parameters*/**/*", - "config/parameters*/**/**/*", - ] - } + + package_data = {package_name: ["README.md"] + config_files} setup_file_context = dict( name=package_name, version=version, From 0ffcb499928a7e8b5082f655db7a31909b7b23e7 Mon Sep 17 00:00:00 2001 From: Merel Theisen Date: Tue, 2 Nov 2021 10:22:25 +0000 Subject: [PATCH 4/4] Add super nested test case --- .../cli/pipeline/test_pipeline_package.py | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/tests/framework/cli/pipeline/test_pipeline_package.py b/tests/framework/cli/pipeline/test_pipeline_package.py index 6ddc9852fe..f1894de041 100644 --- a/tests/framework/cli/pipeline/test_pipeline_package.py +++ b/tests/framework/cli/pipeline/test_pipeline_package.py @@ -358,6 +358,19 @@ def test_package_pipeline_with_deep_nested_parameters( deep_nested_param_path3.mkdir(parents=True, exist_ok=True) (deep_nested_param_path3 / "retail.yml").touch() + super_deep_nested_param_path = Path( + fake_repo_path + / "conf" + / "base" + / "parameters" + / "a" + / "b" + / "c" + / "d" + / "retail" + ) + super_deep_nested_param_path.mkdir(parents=True, exist_ok=True) + (super_deep_nested_param_path / "params3.yml").touch() result = CliRunner().invoke( fake_project_cli, ["pipeline", "package", "retail"], obj=fake_metadata ) @@ -379,6 +392,7 @@ def test_package_pipeline_with_deep_nested_parameters( assert "retail/config/parameters/retail/deep/params1.yml" in wheel_contents assert "retail/config/parameters/retail.yml" in wheel_contents assert "retail/config/parameters/deep/retail.yml" in wheel_contents + assert "retail/config/parameters/a/b/c/d/retail/params3.yml" in wheel_contents def test_pipeline_package_version( self, fake_repo_path, fake_package_path, fake_project_cli, fake_metadata