Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix kedro viz --load-file to run from any directory without requiring a Kedro project #2206

Merged
merged 20 commits into from
Nov 28, 2024
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 15 additions & 10 deletions package/kedro_viz/launchers/cli/run.py
Original file line number Diff line number Diff line change
Expand Up @@ -123,16 +123,21 @@ def run(
)
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
kedro_project_path = None
Copy link
Contributor

@Huongg Huongg Nov 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hey I'm thinking maybe this kedro_project_path does not need to be defined outside the if-else statement if it is only used within the else block?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it is needed later, it is passed to run_server


if load_file:
if not Path(load_file).exists():
raise ValueError(f"The provided filepath '{load_file}' does not exist.")
else:
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()
Expand Down
3 changes: 0 additions & 3 deletions package/kedro_viz/server.py
Original file line number Diff line number Diff line change
Expand Up @@ -132,9 +132,6 @@ def run_server(

app = apps.create_api_app_from_project(path, autoreload)
else:
if not Path(load_file).exists():
raise ValueError(f"The provided filepath '{load_file}' does not exist.")

app = apps.create_api_app_from_file(f"{path}/{load_file}/api")

uvicorn.run(app, host=host, port=port, log_config=None)
Expand Down
21 changes: 21 additions & 0 deletions package/tests/test_launchers/test_cli/test_run.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
from pathlib import Path
from unittest.mock import call

import pytest
Expand Down Expand Up @@ -411,3 +412,23 @@ def test_find_available_port_with_occupied_ports(self, mocker):
assert (
available_port == 4143
), "Expected port 4143 to be returned as the available port"


def test_invalid_load_file_directory(mocker):
"""
Test that Kedro-Viz raises a ValueError when an invalid filepath
is provided to the `--load-file` argument.
"""
runner = CliRunner()

# Mock the existence of the file path to always return False (invalid path)
mocker.patch.object(Path, "exists", return_value=False)

# Invoke the CLI with an invalid `--load-file` path
result = runner.invoke(
main.viz_cli, ["viz", "run", "--load-file", "nonexistent_path.json"]
)

assert "The provided filepath 'nonexistent_path.json' does not exist." == str(
result.exception
)
35 changes: 9 additions & 26 deletions package/tests/test_server.py
Original file line number Diff line number Diff line change
Expand Up @@ -121,32 +121,15 @@ def test_specific_pipeline(
{"data_science": example_pipelines["data_science"]}
)

@pytest.mark.parametrize(
"file_path, expected_exception",
[
("test.json", ValueError), # File does not exist, expect ValueError
("test.json", None), # File exists, expect no ValueError
],
)
def test_load_file(
self, file_path, expected_exception, patched_create_api_app_from_file, tmp_path
):
if expected_exception is not None:
with pytest.raises(expected_exception) as exc_info:
run_server(load_file=file_path)

# Check if the error message contains the expected message
assert "The provided filepath" in str(exc_info.value)
assert "does not exist." in str(exc_info.value)
else:
json_file_path = tmp_path / file_path

# File exists, no exception expected
with json_file_path.open("w") as file:
json.dump({"name": "John", "age": 30}, file)

run_server(load_file=json_file_path)
patched_create_api_app_from_file.assert_called_once()
def test_load_file(self, patched_create_api_app_from_file, tmp_path):
file_path = "test.json"
json_file_path = tmp_path / file_path

with json_file_path.open("w") as file:
json.dump({"name": "John", "age": 30}, file)

run_server(load_file=json_file_path)
patched_create_api_app_from_file.assert_called_once()

def test_save_file(self, tmp_path, mocker):
mock_filesystem = mocker.patch("fsspec.filesystem")
Expand Down
Loading