From 48064c5ef0528122650ff982541a30357bddc1e2 Mon Sep 17 00:00:00 2001 From: lrcouto Date: Wed, 29 May 2024 01:29:57 -0300 Subject: [PATCH 01/21] Add fallback to starters pull on kedro new Signed-off-by: lrcouto --- kedro/framework/cli/starters.py | 32 ++++++++++++++++++++++++++++++-- 1 file changed, 30 insertions(+), 2 deletions(-) diff --git a/kedro/framework/cli/starters.py b/kedro/framework/cli/starters.py index 5a4b76a347..99311eafc2 100644 --- a/kedro/framework/cli/starters.py +++ b/kedro/framework/cli/starters.py @@ -17,6 +17,7 @@ from typing import Any, Callable import click +import requests import yaml from attrs import define, field from importlib_metadata import EntryPoints @@ -95,7 +96,30 @@ class KedroStarterSpec: KEDRO_PATH = Path(kedro.__file__).parent TEMPLATE_PATH = KEDRO_PATH / "templates" / "project" -_STARTERS_REPO = "git+https://github.com/kedro-org/kedro-starters.git" + +def _kedro_and_starters_version_identical() -> bool: + response = requests.get( + "https://api.github.com/repos/kedro-org/kedro-starters/releases/latest", + timeout=10, + ) + http_success: int = 200 + + if response.status_code == http_success: + latest_release = response.json() + else: + raise RuntimeError( + f"Error fetching release information: {response.status_code}" + ) + return True if version == latest_release["tag_name"] else False + + +_STARTERS_REPO = ( + "git+https://github.com/kedro-org/kedro-starters.git" + if _kedro_and_starters_version_identical() + else "https://github.com/kedro-org/kedro-starters.git@main" +) + + _OFFICIAL_STARTER_SPECS = [ KedroStarterSpec("astro-airflow-iris", _STARTERS_REPO, "astro-airflow-iris"), KedroStarterSpec("spaceflights-pandas", _STARTERS_REPO, "spaceflights-pandas"), @@ -773,7 +797,11 @@ def _make_cookiecutter_args_and_fetch_template( tools = config["tools"] example_pipeline = config["example_pipeline"] - starter_path = "git+https://github.com/kedro-org/kedro-starters.git" + starter_path = ( + "git+https://github.com/kedro-org/kedro-starters.git" + if _kedro_and_starters_version_identical() + else "https://github.com/kedro-org/kedro-starters.git@main" + ) if "PySpark" in tools and "Kedro Viz" in tools: # Use the spaceflights-pyspark-viz starter if both PySpark and Kedro Viz are chosen. From 2ffa441b7cee5387ea20ac1b675d5acfc34088cf Mon Sep 17 00:00:00 2001 From: lrcouto Date: Wed, 29 May 2024 01:45:30 -0300 Subject: [PATCH 02/21] Lint Signed-off-by: lrcouto --- kedro/framework/cli/starters.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/kedro/framework/cli/starters.py b/kedro/framework/cli/starters.py index 99311eafc2..1daa92ed8d 100644 --- a/kedro/framework/cli/starters.py +++ b/kedro/framework/cli/starters.py @@ -102,14 +102,15 @@ def _kedro_and_starters_version_identical() -> bool: "https://api.github.com/repos/kedro-org/kedro-starters/releases/latest", timeout=10, ) - http_success: int = 200 + http_success = 200 if response.status_code == http_success: latest_release = response.json() else: raise RuntimeError( - f"Error fetching release information: {response.status_code}" + f"Error fetching kedro-starters latest release version: {response.status_code}" ) + return True if version == latest_release["tag_name"] else False From 4bd7ec951e9ac19dd723b5ff02f7a01dcaa14597 Mon Sep 17 00:00:00 2001 From: lrcouto Date: Wed, 29 May 2024 14:45:50 -0300 Subject: [PATCH 03/21] Add types-requests for mypy Signed-off-by: lrcouto --- pyproject.toml | 1 + 1 file changed, 1 insertion(+) diff --git a/pyproject.toml b/pyproject.toml index 6ece4851cd..2c19074c75 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -77,6 +77,7 @@ test = [ "pandas-stubs", "types-PyYAML", "types-cachetools", + "types-requests", "types-toml" ] docs = [ From 41c7a60a023698b253efdbfc188f4f278dd08041 Mon Sep 17 00:00:00 2001 From: lrcouto Date: Mon, 3 Jun 2024 21:45:01 -0300 Subject: [PATCH 04/21] allow checkout flag to be used for different starters version Signed-off-by: lrcouto --- kedro/framework/cli/starters.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/kedro/framework/cli/starters.py b/kedro/framework/cli/starters.py index 1daa92ed8d..f568a0487a 100644 --- a/kedro/framework/cli/starters.py +++ b/kedro/framework/cli/starters.py @@ -163,7 +163,12 @@ def _kedro_and_starters_version_identical() -> bool: def _validate_flag_inputs(flag_inputs: dict[str, Any]) -> None: - if flag_inputs.get("checkout") and not flag_inputs.get("starter"): + version_tag = r"^\d+(\.\d+)*$" + if ( + flag_inputs.get("checkout") + and not flag_inputs.get("starter") + and not re.match(str(flag_inputs.get("starter")), version_tag) + ): raise KedroCliError("Cannot use the --checkout flag without a --starter value.") if flag_inputs.get("directory") and not flag_inputs.get("starter"): From aa42e543012482b43f224c311ec6c94558fa485c Mon Sep 17 00:00:00 2001 From: lrcouto Date: Mon, 3 Jun 2024 22:46:46 -0300 Subject: [PATCH 05/21] Attempt to fix e2e tests Signed-off-by: lrcouto --- kedro/framework/cli/starters.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/kedro/framework/cli/starters.py b/kedro/framework/cli/starters.py index f568a0487a..6a83a00f61 100644 --- a/kedro/framework/cli/starters.py +++ b/kedro/framework/cli/starters.py @@ -98,8 +98,14 @@ class KedroStarterSpec: def _kedro_and_starters_version_identical() -> bool: + GITHUB_TOKEN = os.getenv("GITHUB_TOKEN") + headers = {} + if GITHUB_TOKEN: + headers["Authorization"] = f"token {GITHUB_TOKEN}" + response = requests.get( "https://api.github.com/repos/kedro-org/kedro-starters/releases/latest", + headers=headers, timeout=10, ) http_success = 200 From 11e599b26b66e9fea3990c2973feedc65b9351d0 Mon Sep 17 00:00:00 2001 From: lrcouto Date: Tue, 4 Jun 2024 22:01:53 -0300 Subject: [PATCH 06/21] Store starters version on env variable Signed-off-by: lrcouto --- kedro/framework/cli/starters.py | 51 +++++++++++++++++---------------- 1 file changed, 27 insertions(+), 24 deletions(-) diff --git a/kedro/framework/cli/starters.py b/kedro/framework/cli/starters.py index 6a83a00f61..e1624ba36b 100644 --- a/kedro/framework/cli/starters.py +++ b/kedro/framework/cli/starters.py @@ -97,27 +97,35 @@ class KedroStarterSpec: TEMPLATE_PATH = KEDRO_PATH / "templates" / "project" -def _kedro_and_starters_version_identical() -> bool: - GITHUB_TOKEN = os.getenv("GITHUB_TOKEN") - headers = {} - if GITHUB_TOKEN: - headers["Authorization"] = f"token {GITHUB_TOKEN}" - - response = requests.get( - "https://api.github.com/repos/kedro-org/kedro-starters/releases/latest", - headers=headers, - timeout=10, - ) - http_success = 200 +def _get_latest_starters_version() -> str: + if "STARTERS_VERSION" not in os.environ: + GITHUB_TOKEN = os.getenv("GITHUB_TOKEN") + headers = {} + if GITHUB_TOKEN: + headers["Authorization"] = f"token {GITHUB_TOKEN}" + + response = requests.get( + "https://api.github.com/repos/kedro-org/kedro-starters/releases/latest", + headers=headers, + timeout=10, + ) + http_success = 200 - if response.status_code == http_success: - latest_release = response.json() + if response.status_code == http_success: + latest_release = response.json() + else: + raise RuntimeError( + f"Error fetching kedro-starters latest release version: {response.status_code}" + ) + os.environ["STARTERS_VERSION"] = latest_release["tag_name"] + return str(latest_release["tag_name"]) else: - raise RuntimeError( - f"Error fetching kedro-starters latest release version: {response.status_code}" - ) + return str(os.getenv("STARTERS_VERSION")) + - return True if version == latest_release["tag_name"] else False +def _kedro_and_starters_version_identical() -> bool: + starters_version = _get_latest_starters_version() + return True if version == starters_version else False _STARTERS_REPO = ( @@ -169,12 +177,7 @@ def _kedro_and_starters_version_identical() -> bool: def _validate_flag_inputs(flag_inputs: dict[str, Any]) -> None: - version_tag = r"^\d+(\.\d+)*$" - if ( - flag_inputs.get("checkout") - and not flag_inputs.get("starter") - and not re.match(str(flag_inputs.get("starter")), version_tag) - ): + if flag_inputs.get("checkout") and not flag_inputs.get("starter"): raise KedroCliError("Cannot use the --checkout flag without a --starter value.") if flag_inputs.get("directory") and not flag_inputs.get("starter"): From a53b459ef07aca4ad1cf90b004e7751be69dd73f Mon Sep 17 00:00:00 2001 From: lrcouto Date: Tue, 4 Jun 2024 23:31:24 -0300 Subject: [PATCH 07/21] Add unit tests Signed-off-by: lrcouto --- tests/framework/cli/test_starters.py | 48 ++++++++++++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/tests/framework/cli/test_starters.py b/tests/framework/cli/test_starters.py index 5d00fa5401..3ff212a607 100644 --- a/tests/framework/cli/test_starters.py +++ b/tests/framework/cli/test_starters.py @@ -2,6 +2,7 @@ """ from __future__ import annotations +import os import shutil from pathlib import Path @@ -18,6 +19,7 @@ KedroStarterSpec, _convert_tool_short_names_to_numbers, _fetch_validate_parse_config_from_user_prompts, + _get_latest_starters_version, _make_cookiecutter_args_and_fetch_template, _parse_tools_input, _parse_yes_no_to_bool, @@ -1687,3 +1689,49 @@ def test_flag_is_absent(self, fake_kedro_cli): telemetry_file_path = Path(repo_name + "/.telemetry") assert not telemetry_file_path.exists() + + +@pytest.fixture +def mock_env_vars(mocker): + """Fixture to mock environment variables""" + mocker.patch.dict(os.environ, {"GITHUB_TOKEN": "fake_token"}, clear=True) + + +class TestGetLatestStartersVersion: + @pytest.fixture(autouse=True) + def setup_class(self, mock_env_vars, requests_mock): + self.requests_mock = requests_mock + + def test_get_latest_starters_version(self): + latest_version = "1.2.3" + + self.requests_mock.get( + "https://api.github.com/repos/kedro-org/kedro-starters/releases/latest", + json={"tag_name": latest_version}, + status_code=200, + ) + + result = _get_latest_starters_version() + + assert result == latest_version + assert os.getenv("STARTERS_VERSION") == latest_version + + def test_get_latest_starters_version_with_env_set(self, mocker): + expected_version = "1.2.3" + mocker.patch.dict(os.environ, {"STARTERS_VERSION": expected_version}) + + result = _get_latest_starters_version() + + assert result == expected_version + + def test_get_latest_starters_version_error(self): + self.requests_mock.get( + "https://api.github.com/repos/kedro-org/kedro-starters/releases/latest", + status_code=403, + ) + + with pytest.raises( + RuntimeError, + match="Error fetching kedro-starters latest release version: 403", + ): + _get_latest_starters_version() From b1b03ea24209743d4a757803ae669ce66f83a21d Mon Sep 17 00:00:00 2001 From: lrcouto Date: Wed, 5 Jun 2024 00:01:57 -0300 Subject: [PATCH 08/21] Fix unit tests Signed-off-by: lrcouto --- tests/framework/cli/test_starters.py | 22 +++++++++------------- 1 file changed, 9 insertions(+), 13 deletions(-) diff --git a/tests/framework/cli/test_starters.py b/tests/framework/cli/test_starters.py index 3ff212a607..c6016dd55b 100644 --- a/tests/framework/cli/test_starters.py +++ b/tests/framework/cli/test_starters.py @@ -1698,14 +1698,12 @@ def mock_env_vars(mocker): class TestGetLatestStartersVersion: - @pytest.fixture(autouse=True) - def setup_class(self, mock_env_vars, requests_mock): - self.requests_mock = requests_mock - - def test_get_latest_starters_version(self): + def test_get_latest_starters_version(self, mock_env_vars, requests_mock): + """Test _get_latest_starters_version when STARTERS_VERSION is not set""" latest_version = "1.2.3" - self.requests_mock.get( + # Mock the GitHub API response + requests_mock.get( "https://api.github.com/repos/kedro-org/kedro-starters/releases/latest", json={"tag_name": latest_version}, status_code=200, @@ -1717,6 +1715,7 @@ def test_get_latest_starters_version(self): assert os.getenv("STARTERS_VERSION") == latest_version def test_get_latest_starters_version_with_env_set(self, mocker): + """Test _get_latest_starters_version when STARTERS_VERSION is already set""" expected_version = "1.2.3" mocker.patch.dict(os.environ, {"STARTERS_VERSION": expected_version}) @@ -1724,14 +1723,11 @@ def test_get_latest_starters_version_with_env_set(self, mocker): assert result == expected_version - def test_get_latest_starters_version_error(self): - self.requests_mock.get( + def test_get_latest_starters_version_error(self, mock_env_vars, requests_mock): + requests_mock.get( "https://api.github.com/repos/kedro-org/kedro-starters/releases/latest", - status_code=403, + exc=RuntimeError("Error message"), ) - with pytest.raises( - RuntimeError, - match="Error fetching kedro-starters latest release version: 403", - ): + with pytest.raises(RuntimeError, match="Error message"): _get_latest_starters_version() From 6aca1f43749faf6fa85f28f3612452352f4cef98 Mon Sep 17 00:00:00 2001 From: lrcouto Date: Wed, 5 Jun 2024 01:00:28 -0300 Subject: [PATCH 09/21] Add requests_mock Signed-off-by: lrcouto --- pyproject.toml | 1 + 1 file changed, 1 insertion(+) diff --git a/pyproject.toml b/pyproject.toml index 2c19074c75..429a19836e 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -72,6 +72,7 @@ test = [ "pytest-xdist[psutil]~=2.2.1", "pytest>=7.2,<9.0", "s3fs>=2021.4, <2024.6", # Upper bound set arbitrarily, to be reassessed in early 2024 + "requests_mock", "trufflehog~=2.1", # mypy related dependencies "pandas-stubs", From 2572d3d404804938a406dcb1ee85fcd2fa423a10 Mon Sep 17 00:00:00 2001 From: lrcouto Date: Wed, 5 Jun 2024 01:19:54 -0300 Subject: [PATCH 10/21] Catch exception if request fails Signed-off-by: lrcouto --- kedro/framework/cli/starters.py | 22 ++++++++++------------ tests/framework/cli/test_starters.py | 15 +++++++++++++++ 2 files changed, 25 insertions(+), 12 deletions(-) diff --git a/kedro/framework/cli/starters.py b/kedro/framework/cli/starters.py index e1624ba36b..e0f0b742d4 100644 --- a/kedro/framework/cli/starters.py +++ b/kedro/framework/cli/starters.py @@ -104,19 +104,17 @@ def _get_latest_starters_version() -> str: if GITHUB_TOKEN: headers["Authorization"] = f"token {GITHUB_TOKEN}" - response = requests.get( - "https://api.github.com/repos/kedro-org/kedro-starters/releases/latest", - headers=headers, - timeout=10, - ) - http_success = 200 - - if response.status_code == http_success: - latest_release = response.json() - else: - raise RuntimeError( - f"Error fetching kedro-starters latest release version: {response.status_code}" + try: + response = requests.get( + "https://api.github.com/repos/kedro-org/kedro-starters/releases/latest", + headers=headers, + timeout=10, ) + response.raise_for_status() # Raise an HTTPError for bad status codes + latest_release = response.json() + except requests.exceptions.RequestException as e: + raise RuntimeError(f"Error fetching kedro-starters latest release version: {e}") + os.environ["STARTERS_VERSION"] = latest_release["tag_name"] return str(latest_release["tag_name"]) else: diff --git a/tests/framework/cli/test_starters.py b/tests/framework/cli/test_starters.py index c6016dd55b..23c29b7b94 100644 --- a/tests/framework/cli/test_starters.py +++ b/tests/framework/cli/test_starters.py @@ -7,6 +7,7 @@ from pathlib import Path import pytest +import requests import toml import yaml from click.testing import CliRunner @@ -1731,3 +1732,17 @@ def test_get_latest_starters_version_error(self, mock_env_vars, requests_mock): with pytest.raises(RuntimeError, match="Error message"): _get_latest_starters_version() + + def test_get_latest_starters_version_request_exception( + self, mock_env_vars, requests_mock + ): + """Test _get_latest_starters_version when the request raises an exception""" + requests_mock.get( + "https://api.github.com/repos/kedro-org/kedro-starters/releases/latest", + exc=requests.exceptions.RequestException("Request failed"), + ) + + with pytest.raises( + RuntimeError, match="Error fetching kedro-starters latest release version" + ): + _get_latest_starters_version() From 6867f923860b9c48a8cd4fd174f65a949ba47fba Mon Sep 17 00:00:00 2001 From: lrcouto Date: Wed, 5 Jun 2024 01:22:29 -0300 Subject: [PATCH 11/21] Lint Signed-off-by: lrcouto --- kedro/framework/cli/starters.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/kedro/framework/cli/starters.py b/kedro/framework/cli/starters.py index e0f0b742d4..6578070b33 100644 --- a/kedro/framework/cli/starters.py +++ b/kedro/framework/cli/starters.py @@ -113,7 +113,9 @@ def _get_latest_starters_version() -> str: response.raise_for_status() # Raise an HTTPError for bad status codes latest_release = response.json() except requests.exceptions.RequestException as e: - raise RuntimeError(f"Error fetching kedro-starters latest release version: {e}") + raise RuntimeError( + f"Error fetching kedro-starters latest release version: {e}" + ) os.environ["STARTERS_VERSION"] = latest_release["tag_name"] return str(latest_release["tag_name"]) From 5088d84f88d151ac5c9da24079fbb49dfd6e63e9 Mon Sep 17 00:00:00 2001 From: lrcouto Date: Wed, 5 Jun 2024 12:13:23 -0300 Subject: [PATCH 12/21] Change error condition on _get_latest_starters_version Signed-off-by: lrcouto --- kedro/framework/cli/starters.py | 12 ++++------ tests/framework/cli/test_starters.py | 35 +++++++++++++--------------- 2 files changed, 20 insertions(+), 27 deletions(-) diff --git a/kedro/framework/cli/starters.py b/kedro/framework/cli/starters.py index 6578070b33..8fa8a66bcb 100644 --- a/kedro/framework/cli/starters.py +++ b/kedro/framework/cli/starters.py @@ -5,6 +5,7 @@ """ from __future__ import annotations +import logging import os import re import shutil @@ -113,9 +114,8 @@ def _get_latest_starters_version() -> str: response.raise_for_status() # Raise an HTTPError for bad status codes latest_release = response.json() except requests.exceptions.RequestException as e: - raise RuntimeError( - f"Error fetching kedro-starters latest release version: {e}" - ) + logging.error(f"Error fetching kedro-starters latest release version: {e}") + return "" os.environ["STARTERS_VERSION"] = latest_release["tag_name"] return str(latest_release["tag_name"]) @@ -812,11 +812,7 @@ def _make_cookiecutter_args_and_fetch_template( tools = config["tools"] example_pipeline = config["example_pipeline"] - starter_path = ( - "git+https://github.com/kedro-org/kedro-starters.git" - if _kedro_and_starters_version_identical() - else "https://github.com/kedro-org/kedro-starters.git@main" - ) + starter_path = _STARTERS_REPO if "PySpark" in tools and "Kedro Viz" in tools: # Use the spaceflights-pyspark-viz starter if both PySpark and Kedro Viz are chosen. diff --git a/tests/framework/cli/test_starters.py b/tests/framework/cli/test_starters.py index 23c29b7b94..415a0a0bf4 100644 --- a/tests/framework/cli/test_starters.py +++ b/tests/framework/cli/test_starters.py @@ -2,6 +2,7 @@ """ from __future__ import annotations +import logging import os import shutil from pathlib import Path @@ -1724,25 +1725,21 @@ def test_get_latest_starters_version_with_env_set(self, mocker): assert result == expected_version - def test_get_latest_starters_version_error(self, mock_env_vars, requests_mock): - requests_mock.get( - "https://api.github.com/repos/kedro-org/kedro-starters/releases/latest", - exc=RuntimeError("Error message"), - ) - - with pytest.raises(RuntimeError, match="Error message"): - _get_latest_starters_version() - - def test_get_latest_starters_version_request_exception( - self, mock_env_vars, requests_mock + def test_get_latest_starters_version_error( + self, mock_env_vars, requests_mock, caplog ): """Test _get_latest_starters_version when the request raises an exception""" - requests_mock.get( - "https://api.github.com/repos/kedro-org/kedro-starters/releases/latest", - exc=requests.exceptions.RequestException("Request failed"), - ) + with caplog.at_level(logging.ERROR): + # Mock the request to raise a RequestException + requests_mock.get( + "https://api.github.com/repos/kedro-org/kedro-starters/releases/latest", + exc=requests.exceptions.RequestException("Request failed"), + ) - with pytest.raises( - RuntimeError, match="Error fetching kedro-starters latest release version" - ): - _get_latest_starters_version() + result = _get_latest_starters_version() + + assert result == "" + assert ( + "Error fetching kedro-starters latest release version: Request failed" + in caplog.text + ) From 15ba4dae69231b5f92921ddab879f4df43233e61 Mon Sep 17 00:00:00 2001 From: lrcouto Date: Thu, 6 Jun 2024 12:54:01 -0300 Subject: [PATCH 13/21] Change env variable name to be more specific Signed-off-by: lrcouto --- kedro/framework/cli/starters.py | 6 +++--- tests/framework/cli/test_starters.py | 8 ++++---- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/kedro/framework/cli/starters.py b/kedro/framework/cli/starters.py index 8fa8a66bcb..565e5bfd2f 100644 --- a/kedro/framework/cli/starters.py +++ b/kedro/framework/cli/starters.py @@ -99,7 +99,7 @@ class KedroStarterSpec: def _get_latest_starters_version() -> str: - if "STARTERS_VERSION" not in os.environ: + if "KEDRO_STARTERS_VERSION" not in os.environ: GITHUB_TOKEN = os.getenv("GITHUB_TOKEN") headers = {} if GITHUB_TOKEN: @@ -117,10 +117,10 @@ def _get_latest_starters_version() -> str: logging.error(f"Error fetching kedro-starters latest release version: {e}") return "" - os.environ["STARTERS_VERSION"] = latest_release["tag_name"] + os.environ["KEDRO_STARTERS_VERSION"] = latest_release["tag_name"] return str(latest_release["tag_name"]) else: - return str(os.getenv("STARTERS_VERSION")) + return str(os.getenv("KEDRO_STARTERS_VERSION")) def _kedro_and_starters_version_identical() -> bool: diff --git a/tests/framework/cli/test_starters.py b/tests/framework/cli/test_starters.py index 415a0a0bf4..b0999abf3e 100644 --- a/tests/framework/cli/test_starters.py +++ b/tests/framework/cli/test_starters.py @@ -1701,7 +1701,7 @@ def mock_env_vars(mocker): class TestGetLatestStartersVersion: def test_get_latest_starters_version(self, mock_env_vars, requests_mock): - """Test _get_latest_starters_version when STARTERS_VERSION is not set""" + """Test _get_latest_starters_version when KEDRO_STARTERS_VERSION is not set""" latest_version = "1.2.3" # Mock the GitHub API response @@ -1714,12 +1714,12 @@ def test_get_latest_starters_version(self, mock_env_vars, requests_mock): result = _get_latest_starters_version() assert result == latest_version - assert os.getenv("STARTERS_VERSION") == latest_version + assert os.getenv("KEDRO_STARTERS_VERSION") == latest_version def test_get_latest_starters_version_with_env_set(self, mocker): - """Test _get_latest_starters_version when STARTERS_VERSION is already set""" + """Test _get_latest_starters_version when KEDRO_STARTERS_VERSION is already set""" expected_version = "1.2.3" - mocker.patch.dict(os.environ, {"STARTERS_VERSION": expected_version}) + mocker.patch.dict(os.environ, {"KEDRO_STARTERS_VERSION": expected_version}) result = _get_latest_starters_version() From 2a3572fb2e730a8fe59b1ac8001151365671d94c Mon Sep 17 00:00:00 2001 From: lrcouto Date: Fri, 14 Jun 2024 16:41:34 -0300 Subject: [PATCH 14/21] Set all starter options to pull from main if versions don't match Signed-off-by: lrcouto --- kedro/framework/cli/starters.py | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/kedro/framework/cli/starters.py b/kedro/framework/cli/starters.py index 565e5bfd2f..4ce3187997 100644 --- a/kedro/framework/cli/starters.py +++ b/kedro/framework/cli/starters.py @@ -805,6 +805,8 @@ def _make_cookiecutter_args_and_fetch_template( "extra_context": config, } + kedro_version_match_starters = _kedro_and_starters_version_identical() + if checkout: cookiecutter_args["checkout"] = checkout if directory: @@ -812,27 +814,35 @@ def _make_cookiecutter_args_and_fetch_template( tools = config["tools"] example_pipeline = config["example_pipeline"] - starter_path = _STARTERS_REPO + starter_path = "git+https://github.com/kedro-org/kedro-starters.git" if "PySpark" in tools and "Kedro Viz" in tools: # Use the spaceflights-pyspark-viz starter if both PySpark and Kedro Viz are chosen. cookiecutter_args["directory"] = "spaceflights-pyspark-viz" # Ensures we use the same tag version of kedro for kedro-starters - cookiecutter_args["checkout"] = version + cookiecutter_args["checkout"] = ( + version if kedro_version_match_starters else "main" + ) elif "PySpark" in tools: # Use the spaceflights-pyspark starter if only PySpark is chosen. cookiecutter_args["directory"] = "spaceflights-pyspark" - cookiecutter_args["checkout"] = version + cookiecutter_args["checkout"] = ( + version if kedro_version_match_starters else "main" + ) elif "Kedro Viz" in tools: # Use the spaceflights-pandas-viz starter if only Kedro Viz is chosen. cookiecutter_args["directory"] = "spaceflights-pandas-viz" elif example_pipeline == "True": # Use spaceflights-pandas starter if example was selected, but PySpark or Viz wasn't cookiecutter_args["directory"] = "spaceflights-pandas" - cookiecutter_args["checkout"] = version + cookiecutter_args["checkout"] = ( + version if kedro_version_match_starters else "main" + ) else: # Use the default template path for non PySpark, Viz or example options: starter_path = template_path + if not kedro_version_match_starters: + cookiecutter_args["checkout"] = "main" return cookiecutter_args, starter_path From 4a11ed266075d63e9ebea51271fae594cc7a83de Mon Sep 17 00:00:00 2001 From: lrcouto Date: Fri, 14 Jun 2024 17:54:24 -0300 Subject: [PATCH 15/21] Add tests to cover changes on _make_cookiecutter_args_and_fetch_template Signed-off-by: lrcouto --- tests/framework/cli/test_starters.py | 119 +++++++++++++++++++++++++++ 1 file changed, 119 insertions(+) diff --git a/tests/framework/cli/test_starters.py b/tests/framework/cli/test_starters.py index b0999abf3e..12b2726287 100644 --- a/tests/framework/cli/test_starters.py +++ b/tests/framework/cli/test_starters.py @@ -1743,3 +1743,122 @@ def test_get_latest_starters_version_error( "Error fetching kedro-starters latest release version: Request failed" in caplog.text ) + + +class TestFetchCookiecutterArgsWhenKedroVersionDifferentFromStarters: + @pytest.fixture + def config(self): + return { + "tools": [], + "example_pipeline": "False", + "output_dir": "/my/output/dir", + } + + def test_kedro_version_match_starters_true(self, config, mocker): + mocker.patch( + "kedro.framework.cli.starters._kedro_and_starters_version_identical", + return_value=True, + ) + + config["tools"] = ["PySpark", "Kedro Viz"] + checkout = "" + directory = "my-directory" + template_path = "my/template/path" + + expected_args = { + "output_dir": "/my/output/dir", + "no_input": True, + "extra_context": config, + "checkout": version, + "directory": "spaceflights-pyspark-viz", + } + expected_path = "git+https://github.com/kedro-org/kedro-starters.git" + + result_args, result_path = _make_cookiecutter_args_and_fetch_template( + config, checkout, directory, template_path + ) + + assert result_args == expected_args + assert result_path == expected_path + + def test_kedro_version_match_starters_false(self, config, mocker): + mocker.patch( + "kedro.framework.cli.starters._kedro_and_starters_version_identical", + return_value=False, + ) + + config["tools"] = ["PySpark", "Kedro Viz"] + checkout = "my-checkout" + directory = "my-directory" + template_path = "my/template/path" + + expected_args = { + "output_dir": "/my/output/dir", + "no_input": True, + "extra_context": config, + "checkout": "main", + "directory": "spaceflights-pyspark-viz", + } + expected_path = "git+https://github.com/kedro-org/kedro-starters.git" + + result_args, result_path = _make_cookiecutter_args_and_fetch_template( + config, checkout, directory, template_path + ) + + assert result_args == expected_args + assert result_path == expected_path + + def test_no_tools_and_example_pipeline(self, config, mocker): + mocker.patch( + "kedro.framework.cli.starters._kedro_and_starters_version_identical", + return_value=False, + ) + + config["tools"] = [] + config["example_pipeline"] = "True" + checkout = "" + directory = "" + template_path = "my/template/path" + + expected_args = { + "output_dir": "/my/output/dir", + "no_input": True, + "extra_context": config, + "checkout": "main", + "directory": "spaceflights-pandas", + } + expected_path = "git+https://github.com/kedro-org/kedro-starters.git" + + result_args, result_path = _make_cookiecutter_args_and_fetch_template( + config, checkout, directory, template_path + ) + + assert result_args == expected_args + assert result_path == expected_path + + def test_no_tools_no_example_pipeline(self, config, mocker): + mocker.patch( + "kedro.framework.cli.starters._kedro_and_starters_version_identical", + return_value=False, + ) + + config["tools"] = [] + config["example_pipeline"] = "False" + checkout = "" + directory = "" + template_path = "my/template/path" + + expected_args = { + "output_dir": "/my/output/dir", + "no_input": True, + "extra_context": config, + "checkout": "main", + } + expected_path = "my/template/path" + + result_args, result_path = _make_cookiecutter_args_and_fetch_template( + config, checkout, directory, template_path + ) + + assert result_args == expected_args + assert result_path == expected_path From 7f658dd7d965762c41601d0044776fd6b7dd212b Mon Sep 17 00:00:00 2001 From: lrcouto Date: Fri, 21 Jun 2024 00:36:46 -0300 Subject: [PATCH 16/21] remove redundant assignments Signed-off-by: lrcouto --- kedro/framework/cli/starters.py | 22 ++++++++-------------- 1 file changed, 8 insertions(+), 14 deletions(-) diff --git a/kedro/framework/cli/starters.py b/kedro/framework/cli/starters.py index 4ce3187997..7794e8e5f6 100644 --- a/kedro/framework/cli/starters.py +++ b/kedro/framework/cli/starters.py @@ -807,43 +807,37 @@ def _make_cookiecutter_args_and_fetch_template( kedro_version_match_starters = _kedro_and_starters_version_identical() - if checkout: - cookiecutter_args["checkout"] = checkout if directory: cookiecutter_args["directory"] = directory tools = config["tools"] example_pipeline = config["example_pipeline"] starter_path = "git+https://github.com/kedro-org/kedro-starters.git" + checkout_version = version if kedro_version_match_starters else "main" if "PySpark" in tools and "Kedro Viz" in tools: # Use the spaceflights-pyspark-viz starter if both PySpark and Kedro Viz are chosen. cookiecutter_args["directory"] = "spaceflights-pyspark-viz" # Ensures we use the same tag version of kedro for kedro-starters - cookiecutter_args["checkout"] = ( - version if kedro_version_match_starters else "main" - ) + cookiecutter_args["checkout"] = checkout_version elif "PySpark" in tools: # Use the spaceflights-pyspark starter if only PySpark is chosen. cookiecutter_args["directory"] = "spaceflights-pyspark" - cookiecutter_args["checkout"] = ( - version if kedro_version_match_starters else "main" - ) + cookiecutter_args["checkout"] = checkout_version elif "Kedro Viz" in tools: # Use the spaceflights-pandas-viz starter if only Kedro Viz is chosen. cookiecutter_args["directory"] = "spaceflights-pandas-viz" + cookiecutter_args["checkout"] = checkout_version elif example_pipeline == "True": # Use spaceflights-pandas starter if example was selected, but PySpark or Viz wasn't cookiecutter_args["directory"] = "spaceflights-pandas" - cookiecutter_args["checkout"] = ( - version if kedro_version_match_starters else "main" - ) + cookiecutter_args["checkout"] = checkout_version else: # Use the default template path for non PySpark, Viz or example options: starter_path = template_path - if not kedro_version_match_starters: - cookiecutter_args["checkout"] = "main" - + cookiecutter_args["checkout"] = ( + checkout if checkout and kedro_version_match_starters else "main" + ) return cookiecutter_args, starter_path From 4077bad903f6ae9e83c05310bd1a5f1cd0bbef62 Mon Sep 17 00:00:00 2001 From: lrcouto Date: Mon, 1 Jul 2024 12:10:18 -0300 Subject: [PATCH 17/21] Changes on logic for fetching templates Signed-off-by: lrcouto --- kedro/framework/cli/starters.py | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/kedro/framework/cli/starters.py b/kedro/framework/cli/starters.py index 7794e8e5f6..4a14234c6f 100644 --- a/kedro/framework/cli/starters.py +++ b/kedro/framework/cli/starters.py @@ -813,12 +813,17 @@ def _make_cookiecutter_args_and_fetch_template( tools = config["tools"] example_pipeline = config["example_pipeline"] starter_path = "git+https://github.com/kedro-org/kedro-starters.git" - checkout_version = version if kedro_version_match_starters else "main" - + + if checkout: + checkout_version = checkout + elif kedro_version_match_starters: + checkout_version = version + else: + checkout_version = "main" + if "PySpark" in tools and "Kedro Viz" in tools: # Use the spaceflights-pyspark-viz starter if both PySpark and Kedro Viz are chosen. cookiecutter_args["directory"] = "spaceflights-pyspark-viz" - # Ensures we use the same tag version of kedro for kedro-starters cookiecutter_args["checkout"] = checkout_version elif "PySpark" in tools: # Use the spaceflights-pyspark starter if only PySpark is chosen. From c7b261d307002aa892ba840371807ef2b0f0185f Mon Sep 17 00:00:00 2001 From: lrcouto Date: Mon, 1 Jul 2024 12:11:57 -0300 Subject: [PATCH 18/21] Lint Signed-off-by: lrcouto --- kedro/framework/cli/starters.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/kedro/framework/cli/starters.py b/kedro/framework/cli/starters.py index 4a14234c6f..5a3790f9db 100644 --- a/kedro/framework/cli/starters.py +++ b/kedro/framework/cli/starters.py @@ -813,14 +813,14 @@ def _make_cookiecutter_args_and_fetch_template( tools = config["tools"] example_pipeline = config["example_pipeline"] starter_path = "git+https://github.com/kedro-org/kedro-starters.git" - + if checkout: checkout_version = checkout elif kedro_version_match_starters: checkout_version = version else: checkout_version = "main" - + if "PySpark" in tools and "Kedro Viz" in tools: # Use the spaceflights-pyspark-viz starter if both PySpark and Kedro Viz are chosen. cookiecutter_args["directory"] = "spaceflights-pyspark-viz" From 4ee258782d7addbdb2a4f478553fb97554c1c117 Mon Sep 17 00:00:00 2001 From: lrcouto Date: Mon, 1 Jul 2024 12:37:17 -0300 Subject: [PATCH 19/21] Update test to match changes on template fetching Signed-off-by: lrcouto --- tests/framework/cli/test_starters.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/framework/cli/test_starters.py b/tests/framework/cli/test_starters.py index 12b2726287..47ac8b1923 100644 --- a/tests/framework/cli/test_starters.py +++ b/tests/framework/cli/test_starters.py @@ -1796,7 +1796,7 @@ def test_kedro_version_match_starters_false(self, config, mocker): "output_dir": "/my/output/dir", "no_input": True, "extra_context": config, - "checkout": "main", + "checkout": "my-checkout", "directory": "spaceflights-pyspark-viz", } expected_path = "git+https://github.com/kedro-org/kedro-starters.git" From c279c6af372afc8a587c4eb47e89ae3aa69afb48 Mon Sep 17 00:00:00 2001 From: lrcouto Date: Mon, 8 Jul 2024 18:46:08 -0300 Subject: [PATCH 20/21] Change logic of version comparison, add tests Signed-off-by: lrcouto --- kedro/framework/cli/starters.py | 9 +++--- tests/framework/cli/test_starters.py | 44 +++++++++++++++++++++++++--- 2 files changed, 45 insertions(+), 8 deletions(-) diff --git a/kedro/framework/cli/starters.py b/kedro/framework/cli/starters.py index 5a3790f9db..a7477b5981 100644 --- a/kedro/framework/cli/starters.py +++ b/kedro/framework/cli/starters.py @@ -22,6 +22,7 @@ import yaml from attrs import define, field from importlib_metadata import EntryPoints +from packaging.version import parse import kedro from kedro import __version__ as version @@ -123,14 +124,14 @@ def _get_latest_starters_version() -> str: return str(os.getenv("KEDRO_STARTERS_VERSION")) -def _kedro_and_starters_version_identical() -> bool: +def _kedro_version_equal_or_lower_to_starters(version: str) -> bool: starters_version = _get_latest_starters_version() - return True if version == starters_version else False + return parse(version) <= parse(starters_version) _STARTERS_REPO = ( "git+https://github.com/kedro-org/kedro-starters.git" - if _kedro_and_starters_version_identical() + if _kedro_version_equal_or_lower_to_starters(version) else "https://github.com/kedro-org/kedro-starters.git@main" ) @@ -805,7 +806,7 @@ def _make_cookiecutter_args_and_fetch_template( "extra_context": config, } - kedro_version_match_starters = _kedro_and_starters_version_identical() + kedro_version_match_starters = _kedro_version_equal_or_lower_to_starters(version) if directory: cookiecutter_args["directory"] = directory diff --git a/tests/framework/cli/test_starters.py b/tests/framework/cli/test_starters.py index 47ac8b1923..3800a2a1a8 100644 --- a/tests/framework/cli/test_starters.py +++ b/tests/framework/cli/test_starters.py @@ -22,6 +22,7 @@ _convert_tool_short_names_to_numbers, _fetch_validate_parse_config_from_user_prompts, _get_latest_starters_version, + _kedro_version_equal_or_lower_to_starters, _make_cookiecutter_args_and_fetch_template, _parse_tools_input, _parse_yes_no_to_bool, @@ -1745,6 +1746,41 @@ def test_get_latest_starters_version_error( ) +class TestStartersVersionComparison: + def test_kedro_version_equal_or_lower_to_starters_equal(self, mocker): + """Test when version is equal to starters_version""" + mocker.patch( + "kedro.framework.cli.starters._get_latest_starters_version", + return_value="1.2.3", + ) + version = "1.2.3" + + result = _kedro_version_equal_or_lower_to_starters(version) + assert result is True + + def test_kedro_version_equal_or_lower_to_starters_lower(self, mocker): + """Test when version is lower than starters_version""" + mocker.patch( + "kedro.framework.cli.starters._get_latest_starters_version", + return_value="1.3.0", + ) + version = "1.2.3" + + result = _kedro_version_equal_or_lower_to_starters(version) + assert result is True + + def test_kedro_version_equal_or_lower_to_starters_higher(self, mocker): + """Test when version is higher than starters_version""" + mocker.patch( + "kedro.framework.cli.starters._get_latest_starters_version", + return_value="1.2.3", + ) + version = "1.2.4" + + result = _kedro_version_equal_or_lower_to_starters(version) + assert result is False + + class TestFetchCookiecutterArgsWhenKedroVersionDifferentFromStarters: @pytest.fixture def config(self): @@ -1756,7 +1792,7 @@ def config(self): def test_kedro_version_match_starters_true(self, config, mocker): mocker.patch( - "kedro.framework.cli.starters._kedro_and_starters_version_identical", + "kedro.framework.cli.starters._kedro_version_equal_or_lower_to_starters", return_value=True, ) @@ -1783,7 +1819,7 @@ def test_kedro_version_match_starters_true(self, config, mocker): def test_kedro_version_match_starters_false(self, config, mocker): mocker.patch( - "kedro.framework.cli.starters._kedro_and_starters_version_identical", + "kedro.framework.cli.starters._kedro_version_equal_or_lower_to_starters", return_value=False, ) @@ -1810,7 +1846,7 @@ def test_kedro_version_match_starters_false(self, config, mocker): def test_no_tools_and_example_pipeline(self, config, mocker): mocker.patch( - "kedro.framework.cli.starters._kedro_and_starters_version_identical", + "kedro.framework.cli.starters._kedro_version_equal_or_lower_to_starters", return_value=False, ) @@ -1838,7 +1874,7 @@ def test_no_tools_and_example_pipeline(self, config, mocker): def test_no_tools_no_example_pipeline(self, config, mocker): mocker.patch( - "kedro.framework.cli.starters._kedro_and_starters_version_identical", + "kedro.framework.cli.starters._kedro_version_equal_or_lower_to_starters", return_value=False, ) From 8e4c8f09cf025cb738abcd07fb9341b9268f80c0 Mon Sep 17 00:00:00 2001 From: lrcouto Date: Wed, 10 Jul 2024 19:55:39 -0300 Subject: [PATCH 21/21] Extract checkout logic to its own function Signed-off-by: lrcouto --- kedro/framework/cli/starters.py | 36 +++++++++++----------------- tests/framework/cli/test_starters.py | 35 ++++++++++++++++++++++++--- 2 files changed, 46 insertions(+), 25 deletions(-) diff --git a/kedro/framework/cli/starters.py b/kedro/framework/cli/starters.py index a7477b5981..bb07cfab2e 100644 --- a/kedro/framework/cli/starters.py +++ b/kedro/framework/cli/starters.py @@ -129,11 +129,7 @@ def _kedro_version_equal_or_lower_to_starters(version: str) -> bool: return parse(version) <= parse(starters_version) -_STARTERS_REPO = ( - "git+https://github.com/kedro-org/kedro-starters.git" - if _kedro_version_equal_or_lower_to_starters(version) - else "https://github.com/kedro-org/kedro-starters.git@main" -) +_STARTERS_REPO = "git+https://github.com/kedro-org/kedro-starters.git" _OFFICIAL_STARTER_SPECS = [ @@ -342,10 +338,10 @@ def new( # noqa: PLR0913 # "directory" is an optional key for starters from plugins, so if the key is # not present we will use "None". directory = spec.directory # type: ignore[assignment] - checkout = checkout or version + checkout = _select_checkout_branch_for_cookiecutter(checkout) elif starter_alias is not None: template_path = starter_alias - checkout = checkout or version + checkout = _select_checkout_branch_for_cookiecutter(checkout) else: template_path = str(TEMPLATE_PATH) @@ -775,6 +771,15 @@ def _make_cookiecutter_context_for_prompts(cookiecutter_dir: Path) -> OrderedDic return cookiecutter_context.get("cookiecutter", {}) # type: ignore[no-any-return] +def _select_checkout_branch_for_cookiecutter(checkout: str | None) -> str: + if checkout: + return checkout + elif _kedro_version_equal_or_lower_to_starters(version): + return version + else: + return "main" + + def _make_cookiecutter_args_and_fetch_template( config: dict[str, str], checkout: str, @@ -806,8 +811,6 @@ def _make_cookiecutter_args_and_fetch_template( "extra_context": config, } - kedro_version_match_starters = _kedro_version_equal_or_lower_to_starters(version) - if directory: cookiecutter_args["directory"] = directory @@ -815,35 +818,24 @@ def _make_cookiecutter_args_and_fetch_template( example_pipeline = config["example_pipeline"] starter_path = "git+https://github.com/kedro-org/kedro-starters.git" - if checkout: - checkout_version = checkout - elif kedro_version_match_starters: - checkout_version = version - else: - checkout_version = "main" + cookiecutter_args["checkout"] = checkout if "PySpark" in tools and "Kedro Viz" in tools: # Use the spaceflights-pyspark-viz starter if both PySpark and Kedro Viz are chosen. cookiecutter_args["directory"] = "spaceflights-pyspark-viz" - cookiecutter_args["checkout"] = checkout_version elif "PySpark" in tools: # Use the spaceflights-pyspark starter if only PySpark is chosen. cookiecutter_args["directory"] = "spaceflights-pyspark" - cookiecutter_args["checkout"] = checkout_version elif "Kedro Viz" in tools: # Use the spaceflights-pandas-viz starter if only Kedro Viz is chosen. cookiecutter_args["directory"] = "spaceflights-pandas-viz" - cookiecutter_args["checkout"] = checkout_version elif example_pipeline == "True": # Use spaceflights-pandas starter if example was selected, but PySpark or Viz wasn't cookiecutter_args["directory"] = "spaceflights-pandas" - cookiecutter_args["checkout"] = checkout_version else: # Use the default template path for non PySpark, Viz or example options: starter_path = template_path - cookiecutter_args["checkout"] = ( - checkout if checkout and kedro_version_match_starters else "main" - ) + return cookiecutter_args, starter_path diff --git a/tests/framework/cli/test_starters.py b/tests/framework/cli/test_starters.py index 2d9b80ca84..b114c818de 100644 --- a/tests/framework/cli/test_starters.py +++ b/tests/framework/cli/test_starters.py @@ -26,6 +26,7 @@ _make_cookiecutter_args_and_fetch_template, _parse_tools_input, _parse_yes_no_to_bool, + _select_checkout_branch_for_cookiecutter, _validate_tool_selection, ) @@ -1797,7 +1798,7 @@ def test_kedro_version_match_starters_true(self, config, mocker): ) config["tools"] = ["PySpark", "Kedro Viz"] - checkout = "" + checkout = version directory = "my-directory" template_path = "my/template/path" @@ -1852,7 +1853,7 @@ def test_no_tools_and_example_pipeline(self, config, mocker): config["tools"] = [] config["example_pipeline"] = "True" - checkout = "" + checkout = "main" directory = "" template_path = "my/template/path" @@ -1880,7 +1881,7 @@ def test_no_tools_no_example_pipeline(self, config, mocker): config["tools"] = [] config["example_pipeline"] = "False" - checkout = "" + checkout = "main" directory = "" template_path = "my/template/path" @@ -1898,3 +1899,31 @@ def test_no_tools_no_example_pipeline(self, config, mocker): assert result_args == expected_args assert result_path == expected_path + + +class TestSelectCheckoutBranch: + def test_select_checkout_branch_with_checkout(self): + checkout = "user_selected_branch" + result = _select_checkout_branch_for_cookiecutter(checkout) + + assert result == checkout + + def test_select_checkout_branch_kedro_version_equal_or_lower(self, mocker): + mocker.patch( + "kedro.framework.cli.starters._kedro_version_equal_or_lower_to_starters", + return_value=True, + ) + checkout = None + result = _select_checkout_branch_for_cookiecutter(checkout) + + assert result == version + + def test_select_checkout_branch_main(self, mocker): + mocker.patch( + "kedro.framework.cli.starters._kedro_version_equal_or_lower_to_starters", + return_value=False, + ) + checkout = None + result = _select_checkout_branch_for_cookiecutter(checkout) + + assert result == "main"