Skip to content

Commit

Permalink
Fix Kedro-Viz waiting for valid Kedro Project (#1871)
Browse files Browse the repository at this point in the history
* Try remove bootstrap_project()

Signed-off-by: Merel Theisen <[email protected]>

* Try set the package name for spawn process

Signed-off-by: Merel Theisen <[email protected]>

* move configure project before kedro integration

Signed-off-by: ravi-kumar-pilla <[email protected]>

* include package name in jupyter and dev run

Signed-off-by: ravi-kumar-pilla <[email protected]>

* fix tests and lint

Signed-off-by: ravi-kumar-pilla <[email protected]>

* update release note

Signed-off-by: ravi-kumar-pilla <[email protected]>

* revert permission change

Signed-off-by: ravi-kumar-pilla <[email protected]>

* fix PR comments

Signed-off-by: ravi-kumar-pilla <[email protected]>

* working draft with parent project search

Signed-off-by: ravi-kumar-pilla <[email protected]>

* update error message to be consistent with other viz commands

Signed-off-by: ravi-kumar-pilla <[email protected]>

* update release note:

Signed-off-by: ravi-kumar-pilla <[email protected]>

* update release note

Signed-off-by: ravi-kumar-pilla <[email protected]>

* update error message

Signed-off-by: ravi-kumar-pilla <[email protected]>

---------

Signed-off-by: Merel Theisen <[email protected]>
Signed-off-by: ravi-kumar-pilla <[email protected]>
Co-authored-by: Merel Theisen <[email protected]>
  • Loading branch information
ravi-kumar-pilla and merelcht authored Apr 30, 2024
1 parent be7236e commit 329f13b
Show file tree
Hide file tree
Showing 5 changed files with 141 additions and 7 deletions.
1 change: 1 addition & 0 deletions RELEASE.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ Please follow the established format:

- Upgrade the gitpod workspace-full to a newer version which includes both Node 18 and Python 3.11.5. (#1862)
- Refactor backend integration with Kedro by replacing bootstrap_project with configure_project. (#1796)
- Fix Kedro-Viz waiting for valid Kedro project. (#1871)

# Release 9.0.0

Expand Down
18 changes: 15 additions & 3 deletions package/kedro_viz/launchers/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,9 @@
from kedro_viz.integrations.deployment.deployer_factory import DeployerFactory
from kedro_viz.integrations.pypi import get_latest_version, is_running_outdated_version
from kedro_viz.launchers.utils import (
_PYPROJECT,
_check_viz_up,
_find_kedro_project,
_start_browser,
_wait_for,
viz_deploy_progress_timer,
Expand Down Expand Up @@ -129,6 +131,17 @@ def run(
"""Launch local Kedro Viz instance"""
from kedro_viz.server import run_server

kedro_project_path = _find_kedro_project(Path.cwd())

if kedro_project_path is None:
display_cli_message(
"ERROR: Failed to start Kedro-Viz : "
"Could not find the project configuration "
f"file '{_PYPROJECT}' at '{Path.cwd()}'. ",
"red",
)
return

installed_version = parse(__version__)
latest_version = get_latest_version()
if is_running_outdated_version(installed_version, latest_version):
Expand All @@ -152,16 +165,15 @@ def run(
"save_file": save_file,
"pipeline_name": pipeline,
"env": env,
"project_path": kedro_project_path,
"autoreload": autoreload,
"include_hooks": include_hooks,
"package_name": PACKAGE_NAME,
"extra_params": params,
}
if autoreload:
project_path = Path.cwd()
run_server_kwargs["project_path"] = project_path
run_process_kwargs = {
"path": project_path,
"path": kedro_project_path,
"target": run_server,
"kwargs": run_server_kwargs,
"watcher_cls": RegExpWatcher,
Expand Down
24 changes: 23 additions & 1 deletion package/kedro_viz/launchers/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,14 @@

import logging
import webbrowser
from pathlib import Path
from time import sleep, time
from typing import Any, Callable
from typing import Any, Callable, Union

import requests

logger = logging.getLogger(__name__)
_PYPROJECT = "pyproject.toml"


class WaitForException(Exception):
Expand Down Expand Up @@ -105,3 +107,23 @@ def viz_deploy_progress_timer(process_completed, timeout):
)
sleep(1)
elapsed_time += 1


def _is_project(project_path: Union[str, Path]) -> bool:
metadata_file = Path(project_path).expanduser().resolve() / _PYPROJECT
if not metadata_file.is_file():
return False

try:
return "[tool.kedro]" in metadata_file.read_text(encoding="utf-8")
# pylint: disable=broad-exception-caught
except Exception:
return False


def _find_kedro_project(current_dir: Path) -> Any:
paths_to_check = [current_dir] + list(current_dir.parents)
for project_dir in paths_to_check:
if _is_project(project_dir):
return project_dir
return None
55 changes: 52 additions & 3 deletions package/tests/test_launchers/test_cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
from kedro_viz import __version__
from kedro_viz.constants import SHAREABLEVIZ_SUPPORTED_PLATFORMS, VIZ_DEPLOY_TIME_LIMIT
from kedro_viz.launchers import cli
from kedro_viz.launchers.utils import _PYPROJECT
from kedro_viz.server import run_server


Expand Down Expand Up @@ -85,6 +86,7 @@ def mock_project_path(mocker):
"save_file": None,
"pipeline_name": None,
"env": None,
"project_path": "testPath",
"autoreload": False,
"include_hooks": False,
"package_name": None,
Expand All @@ -100,6 +102,7 @@ def mock_project_path(mocker):
"save_file": None,
"pipeline_name": None,
"env": None,
"project_path": "testPath",
"autoreload": False,
"include_hooks": False,
"package_name": None,
Expand All @@ -120,6 +123,7 @@ def mock_project_path(mocker):
"save_file": None,
"pipeline_name": None,
"env": None,
"project_path": "testPath",
"autoreload": False,
"include_hooks": False,
"package_name": None,
Expand Down Expand Up @@ -151,6 +155,7 @@ def mock_project_path(mocker):
"save_file": "save_dir",
"pipeline_name": "data_science",
"env": "local",
"project_path": "testPath",
"autoreload": False,
"include_hooks": False,
"package_name": None,
Expand All @@ -166,6 +171,7 @@ def mock_project_path(mocker):
"save_file": None,
"pipeline_name": None,
"env": None,
"project_path": "testPath",
"autoreload": False,
"include_hooks": True,
"package_name": None,
Expand All @@ -185,6 +191,11 @@ def test_kedro_viz_command_run_server(
runner = CliRunner()
# Reduce the timeout argument from 600 to 1 to make test run faster.
mocker.patch("kedro_viz.launchers.cli._wait_for.__defaults__", (True, 1, True, 1))
# Mock finding kedro project
mocker.patch(
"kedro_viz.launchers.cli._find_kedro_project",
return_value=run_server_args["project_path"],
)

with runner.isolated_filesystem():
runner.invoke(cli.viz_cli, command_options)
Expand All @@ -195,8 +206,30 @@ def test_kedro_viz_command_run_server(
assert run_server_args["port"] in cli._VIZ_PROCESSES


def test_kedro_viz_command_should_log_project_not_found(
mocker, mock_project_path, mock_click_echo
):
# Reduce the timeout argument from 600 to 1 to make test run faster.
mocker.patch("kedro_viz.launchers.cli._wait_for.__defaults__", (True, 1, True, 1))
# Mock finding kedro project
mocker.patch("kedro_viz.launchers.cli._find_kedro_project", return_value=None)
runner = CliRunner()
with runner.isolated_filesystem():
runner.invoke(cli.viz_cli, ["viz", "run"])

mock_click_echo_calls = [
call(
"\x1b[31mERROR: Failed to start Kedro-Viz : "
"Could not find the project configuration "
f"file '{_PYPROJECT}' at '{mock_project_path}'. \x1b[0m"
)
]

mock_click_echo.assert_has_calls(mock_click_echo_calls)


def test_kedro_viz_command_should_log_outdated_version(
mocker, mock_http_response, mock_click_echo
mocker, mock_http_response, mock_click_echo, mock_project_path
):
installed_version = parse(__version__)
mock_version = f"{installed_version.major + 1}.0.0"
Expand All @@ -209,6 +242,10 @@ def test_kedro_viz_command_should_log_outdated_version(

# Reduce the timeout argument from 600 to 1 to make test run faster.
mocker.patch("kedro_viz.launchers.cli._wait_for.__defaults__", (True, 1, True, 1))
# Mock finding kedro project
mocker.patch(
"kedro_viz.launchers.cli._find_kedro_project", return_value=mock_project_path
)
runner = CliRunner()
with runner.isolated_filesystem():
runner.invoke(cli.viz_cli, ["viz", "run"])
Expand All @@ -228,7 +265,7 @@ def test_kedro_viz_command_should_log_outdated_version(


def test_kedro_viz_command_should_not_log_latest_version(
mocker, mock_http_response, mock_click_echo
mocker, mock_http_response, mock_click_echo, mock_project_path
):
requests_get = mocker.patch("requests.get")
requests_get.return_value = mock_http_response(
Expand All @@ -238,6 +275,10 @@ def test_kedro_viz_command_should_not_log_latest_version(
mocker.patch("kedro_viz.server.run_server")
# Reduce the timeout argument from 600 to 1 to make test run faster.
mocker.patch("kedro_viz.launchers.cli._wait_for.__defaults__", (True, 1, True, 1))
# Mock finding kedro project
mocker.patch(
"kedro_viz.launchers.cli._find_kedro_project", return_value=mock_project_path
)
runner = CliRunner()
with runner.isolated_filesystem():
runner.invoke(cli.viz_cli, ["viz", "run"])
Expand All @@ -248,14 +289,18 @@ def test_kedro_viz_command_should_not_log_latest_version(


def test_kedro_viz_command_should_not_log_if_pypi_is_down(
mocker, mock_http_response, mock_click_echo
mocker, mock_http_response, mock_click_echo, mock_project_path
):
requests_get = mocker.patch("requests.get")
requests_get.side_effect = requests.exceptions.RequestException("PyPI is down")

mocker.patch("kedro_viz.server.run_server")
# Reduce the timeout argument from 600 to 1 to make test run faster.
mocker.patch("kedro_viz.launchers.cli._wait_for.__defaults__", (True, 1, True, 1))
# Mock finding kedro project
mocker.patch(
"kedro_viz.launchers.cli._find_kedro_project", return_value=mock_project_path
)
runner = CliRunner()
with runner.isolated_filesystem():
runner.invoke(cli.viz_cli, ["viz", "run"])
Expand All @@ -272,6 +317,10 @@ def test_kedro_viz_command_with_autoreload(

# Reduce the timeout argument from 600 to 1 to make test run faster.
mocker.patch("kedro_viz.launchers.cli._wait_for.__defaults__", (True, 1, True, 1))
# Mock finding kedro project
mocker.patch(
"kedro_viz.launchers.cli._find_kedro_project", return_value=mock_project_path
)
runner = CliRunner()
with runner.isolated_filesystem():
runner.invoke(cli.viz_cli, ["viz", "run", "--autoreload"])
Expand Down
50 changes: 50 additions & 0 deletions package/tests/test_launchers/test_utils.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
from pathlib import Path
from unittest import mock
from unittest.mock import Mock, call, patch

Expand All @@ -7,6 +8,8 @@
from kedro_viz.constants import VIZ_DEPLOY_TIME_LIMIT
from kedro_viz.launchers.utils import (
_check_viz_up,
_find_kedro_project,
_is_project,
_start_browser,
viz_deploy_progress_timer,
)
Expand Down Expand Up @@ -69,3 +72,50 @@ def test_viz_deploy_progress_timer(capsys):
for second in range(1, VIZ_DEPLOY_TIME_LIMIT + 1):
expected_output = f"...Creating your build/deploy Kedro-Viz ({second}s)"
assert expected_output in captured.out


class TestIsProject:
project_path = Path.cwd()

def test_no_metadata_file(self, mocker):
mocker.patch.object(Path, "is_file", return_value=False)

assert not _is_project(self.project_path)

def test_toml_invalid_format(self, tmp_path):
"""Test for loading context from an invalid path."""
toml_path = tmp_path / "pyproject.toml"
toml_path.write_text("!!") # Invalid TOML

assert not _is_project(tmp_path)

def test_non_kedro_project(self, mocker):
mocker.patch.object(Path, "is_file", return_value=True)
mocker.patch.object(Path, "read_text", return_value="[tool]")

assert not _is_project(self.project_path)

def test_valid_toml_file(self, mocker):
mocker.patch.object(Path, "is_file", return_value=True)
pyproject_toml_payload = "[tool.kedro]" # \nproject_name = 'proj'"
mocker.patch.object(Path, "read_text", return_value=pyproject_toml_payload)

assert _is_project(self.project_path)

def test_toml_bad_encoding(self, mocker):
mocker.patch.object(Path, "is_file", return_value=True)
mocker.patch.object(Path, "read_text", side_effect=UnicodeDecodeError)

assert not _is_project(self.project_path)


@pytest.mark.parametrize(
"project_dir, is_project_found, expected",
[
("/path/to/valid/project", True, Path("/path/to/valid/project")),
("/path/to/nonexistent/project", False, None),
],
)
def test_find_kedro_project(project_dir, is_project_found, expected, mocker):
mocker.patch("kedro_viz.launchers.utils._is_project", return_value=is_project_found)
assert _find_kedro_project(Path(project_dir)) == expected

0 comments on commit 329f13b

Please sign in to comment.