-
Notifications
You must be signed in to change notification settings - Fork 25
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
Restructure-cli #488
Restructure-cli #488
Conversation
Ok, this looks good to me @PhilippeMoussalli. |
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.
Lgtm, just have one smaller suggestion.
src/fondant/cli.py
Outdated
help="Output directory", | ||
default=None, | ||
help="Output path of compiled pipeline", | ||
default="docker_compose.yml", |
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.
Is there a reason for going away from the default docker-compose naming?
default="docker_compose.yml", | |
default="docker-compose.yml", |
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.
Good catch, forgot that it was referenced like this
src/fondant/cli.py
Outdated
help="Output directory", | ||
default=None, | ||
help="Output path of compiled pipeline", | ||
default="docker_compose.yml", |
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.
See my comment above.
default="docker_compose.yml", | |
default="docker-compose.yml", |
The `run` command was broken in #488. This fixes it by adding the `ref` argument to the subparsers separately.
With the introduction of a new compiler and runner for vertex, I feel the need to restructure the CLI a bit. This is a draft PR (docs and tests still need to be updated).
Running
Mode is not longer a positional argument, Pipeline/reference is moved to the end (similar to docker).
fondant run local pipeline.py
Help
Arguments are not isolated per mode which makes it which argument belongs to which mode (e.g. extra volumes not needed for KFP)
To further deep dive