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

Conversation

rashidakanchwala
Copy link
Contributor

@rashidakanchwala rashidakanchwala commented Nov 26, 2024

Description

This PR fixes #2045.

Previously, kedro viz --load-file checked if the command was run from within a Kedro project directory, which is not required. The fix ensures that kedro viz --load-file can run from anywhere as long as it has the correct path the API data directory .

The key behavior is now as follows:

  1. If the specified filepath does not exist, a ValueError is raised:
ValueError: The provided filepath 'hellod' does not exist.
  1. If the filepath exists but does not contain the expected directory structure (api/main, api/pipelines/, and api/nodes/), a FileNotFoundError is raised:

FileNotFoundError: [Errno 2] No such file or directory: '/path/to/xyz/api/main'

  1. If the filepath exists and has the required api directory structure and conent, kedro viz --load-file runs successfully using the API JSON data.

Development notes

QA notes

Checklist

  • Read the contributing guidelines
  • Opened this PR as a 'Draft Pull Request' if it is work-in-progress
  • Updated the documentation to reflect the code changes
  • Added new entries to the RELEASE.md file
  • Added tests to cover my changes

Signed-off-by: rashidakanchwala <[email protected]>
Signed-off-by: rashidakanchwala <[email protected]>
@rashidakanchwala rashidakanchwala changed the title done Fix kedro viz --load-file checking for kedro project configurations Nov 26, 2024
@rashidakanchwala rashidakanchwala changed the title Fix kedro viz --load-file checking for kedro project configurations Fix kedro viz --load-file to run from any directory without requiring a Kedro project Nov 26, 2024
@rashidakanchwala rashidakanchwala marked this pull request as draft November 26, 2024 13:54
Signed-off-by: rashidakanchwala <[email protected]>
@rashidakanchwala rashidakanchwala removed the request for review from ravi-kumar-pilla November 26, 2024 14:50
@rashidakanchwala rashidakanchwala marked this pull request as ready for review November 26, 2024 15:25
Signed-off-by: rashidakanchwala <[email protected]>
Signed-off-by: rashidakanchwala <[email protected]>
Copy link
Contributor

@SajidAlamQB SajidAlamQB left a comment

Choose a reason for hiding this comment

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

LGTM, thank you @rashidakanchwala!

"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

Copy link
Contributor

@Huongg Huongg left a comment

Choose a reason for hiding this comment

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

I tested it locally and its working as described in the ticket, thanks @rashidakanchwala

@ravi-kumar-pilla
Copy link
Contributor

Hi @rashidakanchwala , The PR looks good. Something not related to the ticket but if possible, can we update the docs here

image

and here with the directory structure that is needed to run viz when the user does kedro viz --load-file ?

Thank you

Copy link
Contributor

@ravi-kumar-pilla ravi-kumar-pilla left a comment

Choose a reason for hiding this comment

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

Left a doc comment but the PR works well... Thank you @rashidakanchwala

Signed-off-by: rashidakanchwala <[email protected]>
rashidakanchwala and others added 3 commits November 27, 2024 09:59
Signed-off-by: rashidakanchwala <[email protected]>
Signed-off-by: rashidakanchwala <[email protected]>
Signed-off-by: rashidakanchwala <[email protected]>
Signed-off-by: rashidakanchwala <[email protected]>
Signed-off-by: rashidakanchwala <[email protected]>
Signed-off-by: rashidakanchwala <[email protected]>
Signed-off-by: rashidakanchwala <[email protected]>
Signed-off-by: rashidakanchwala <[email protected]>
Signed-off-by: rashidakanchwala <[email protected]>
Signed-off-by: rashidakanchwala <[email protected]>
@rashidakanchwala rashidakanchwala enabled auto-merge (squash) November 28, 2024 12:13
@rashidakanchwala rashidakanchwala merged commit 4d26f2f into main Nov 28, 2024
37 checks passed
@rashidakanchwala rashidakanchwala deleted the fix-load-file-project-conf-issue branch November 28, 2024 12:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

kedro viz --load-file looks for project configurations.
6 participants