-
Notifications
You must be signed in to change notification settings - Fork 114
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
Implementation of Directory Selection for Server Startup [Santana's PR] #1792
Conversation
* Add link to open in new tab * add test and lint Signed-off-by: Sajid Alam <[email protected]> * Update RELEASE.md Signed-off-by: Sajid Alam <[email protected]> --------- Signed-off-by: Sajid Alam <[email protected]> Signed-off-by: Elias Santana <[email protected]>
Signed-off-by: Elias Santana <[email protected]>
…kedro-viz into feature/set-directory
@@ -130,7 +131,7 @@ def run_server( | |||
from kedro.framework.startup import bootstrap_project | |||
|
|||
parser = argparse.ArgumentParser(description="Launch a development viz server") | |||
parser.add_argument("project_path", help="Path to a Kedro project") | |||
parser.add_argument("--project-path", help="Path to a Kedro project") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For dev mode, earlier we had project_path as a required argument. This is good to make it consistent with cli, but can we add a default similar to cli ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, we need to adjust our Makefile which does python3 package/kedro_viz/server.py $(PROJECT_PATH)
to python3 package/kedro_viz/server.py --project-path=$(PROJECT_PATH)
"target": run_server, | ||
"kwargs": { | ||
"host": args.host, | ||
"port": args.port, | ||
"project_path": str(project_path), | ||
"project_path": str(args.project_path), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
since we are removing path = Path(project_path) if project_path else Path.cwd()
from run_server, we can just pass the resolved project_path
from line 143 here
@click.option( | ||
"--project-path", | ||
default=None, | ||
type=str, | ||
help="Select the indicated directory. Get the current directory if not indicated.", | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
kedro-org/kedro#3683 (comment) brought me here. Since kedro viz
is a global command, which mean it can run outside of a project. The logic should go as follow:
pseudocode:
if project_path:
use project path
elif ProjectMetadata:
within a project, Kedro already tries to locate pyproject.toml and plugin shouldn't do this again.
look for ProjectMetadata from cli hooks
else:
fail because the project is not found.
Description
Development notes
QA notes
Checklist
RELEASE.md
file