-
Notifications
You must be signed in to change notification settings - Fork 26
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
Add --build-arg argument to compile and run commands #306
Conversation
b047562
to
c3dfddb
Compare
@@ -228,6 +234,7 @@ def compile(args): | |||
pipeline=args.pipeline, | |||
extra_volumes=args.extra_volumes, | |||
output_path=args.output_path, | |||
build_args=args.build_arg, |
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.
Any reason we have build_args (plural) here?
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.
There can be multiple build arguments, but the command line is --build-arg
since you provide them one at a time.
Eg. --build-arg FONDANT_VERSION=a8b9fd --build-arg CUSTOM_ARG=value
becomes build_args = ["FONANT_VERSION=a8b9fd", "CUSTOM_ARG=value"]
@@ -35,7 +35,7 @@ def __init__( | |||
""" | |||
self.client = ClipClient( | |||
url=url, | |||
indice_name="laion5B", #TODO:revert back to laion5b-L-14 after backend correction | |||
indice_name="laion5B-L-14", |
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.
fyi: this will only work for the public service atm. @ChristiaensBert did you do the backed modifications?
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.
Yes, I reverted the arguments to the public service as well. Seems most logical to include in our example so users can easily test it on a small scale.
) | ||
|
||
class NoAliasDumper(yaml.SafeDumper): | ||
def ignore_aliases(self, data): |
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.
Could you briefly explain why we need this ?
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.
Otherwise it creates the build arguments as an anchor and refers to it everywhere with an alias. It also uses random numbers as name for this, which reduces readability of the docker-compose.yaml
.
Fixes ml6team#305 Can be used to test a pipeline with fondant installed from a specific git hash available on github: ```bash fondant run pipeline:pipeline --local --build-arg FONDANT_VERSION=551f7b1 ```
Fixes #305 Can be used to test a pipeline with fondant installed from a specific git hash available on github: ```bash fondant run pipeline:pipeline --local --build-arg FONDANT_VERSION=551f7b1 ```
Fixes #305
Can be used to test a pipeline with fondant installed from a specific git hash available on github: