-
Notifications
You must be signed in to change notification settings - Fork 168
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 cli option to supply path to the construct yaml file (supersedes #728) #758
Conversation
Signed-off-by: Kevin Mills <[email protected]>
Co-authored-by: jaimergp <[email protected]>
Co-authored-by: jaimergp <[email protected]>
Co-authored-by: jaimergp <[email protected]>
Co-authored-by: jaimergp <[email protected]>
Co-authored-by: jaimergp <[email protected]>
Co-authored-by: jaimergp <[email protected]>
constructor/main.py
Outdated
@@ -357,6 +358,13 @@ def main(): | |||
action="store", | |||
metavar="CONDA_EXE") | |||
|
|||
p.add_argument('--construct-yaml-fn', |
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.
p.add_argument('--construct-yaml-fn', | |
p.add_argument('-c', '--config-file', |
That's easier to understand than jargon like fn
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.
Can we make it config-filename
then?
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.
Unless you're validating the data that it's just a name, I don't think that's useful IMO. It's easier to validate the value (e.g. if it's a relative or an absolute path, treat it as the default, and fallback to joining with the CWD)
news/758-yaml-filename
Outdated
@@ -0,0 +1,19 @@ | |||
### Enhancements | |||
|
|||
* Add a new `--construct-yaml-fn` argument to specify an input file not named `construct.yaml`. (#727 via #758) |
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.
* Add a new `--construct-yaml-fn` argument to specify an input file not named `construct.yaml`. (#727 via #758) | |
* Add a new `-c`/`--config-file` argument to specify an input file not named `construct.yaml`. (#727 via #758) |
tests/test_examples.py
Outdated
@@ -265,6 +266,8 @@ def create_installer( | |||
str(input_dir), | |||
"--output-dir", | |||
str(output_dir), | |||
"--construct-yaml-fn", |
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.
"--construct-yaml-fn", | |
"--config-file", |
tests/test_examples.py
Outdated
@@ -383,7 +386,9 @@ def test_example_miniforge(tmp_path, request): | |||
|
|||
def test_example_noconda(tmp_path, request): | |||
input_path = _example_path("noconda") | |||
for installer, install_dir in create_installer(input_path, tmp_path, with_spaces=True): | |||
for installer, install_dir in create_installer( | |||
input_path, tmp_path, construct_yaml_filename="constructor_input.yaml", with_spaces=True |
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.
input_path, tmp_path, construct_yaml_filename="constructor_input.yaml", with_spaces=True | |
input_path, tmp_path, config_file="constructor_input.yaml", with_spaces=True |
constructor/main.py
Outdated
@@ -404,7 +412,8 @@ def main(): | |||
out_dir = normalize_path(args.output_dir) | |||
main_build(dir_path, output_dir=out_dir, platform=args.platform, | |||
verbose=args.verbose, cache_dir=args.cache_dir, | |||
dry_run=args.dry_run, conda_exe=conda_exe) | |||
dry_run=args.dry_run, conda_exe=conda_exe, | |||
construct_yaml_filename=args.construct_yaml_filename) |
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.
construct_yaml_filename=args.construct_yaml_filename) | |
config_file=config_file) |
@jezdez PTAL. Once this goes in, I think we can release a new constructor version. |
@@ -381,6 +389,11 @@ def main(): | |||
dir_path = args.dir_path | |||
if not isdir(dir_path): | |||
p.error("no such directory: %s" % dir_path) | |||
if os.sep in args.config_filename: | |||
p.error("--config-filename can only be a filename, not a 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.
👍🏻
Description
Supersedes #728 and closes #727
Checklist - did you ...
news
directory (using the template) for the next release's release notes?