Update spack stack extension for containers: make specs configurable, clean up arguments for env/ctr #333
Conversation
…stack create [env|ctr]
climbfuji
force-pushed
the
feature/container_env_updates
branch
from
5ef344f to
6c02740
Compare
climbfuji
force-pushed
the
feature/container_env_updates
branch
from
2aa856c to
f4d7b6c
Compare
…stack/tests/test_stack_create.py
climbfuji
force-pushed
the
feature/container_env_updates
branch
from
f4d7b6c to
bea1111
Compare
climbfuji
requested review from
AlexanderRichert-NOAA,
eap,
srherbener and
ulmononian
| @@ -51,44 +51,80 @@ def container_config_help(): | |||
| help_string = "Pre-configured container." + os.linesep | |||
| help_string += "Available options are: " + os.linesep | |||
| for config in container_configs: | |||
| if config == "README.md": | |||
There was a problem hiding this comment.
Alternatively, we could use the logic from lib/jcsda-emc/spack-stack/tests/test_stack_create.py:
_, _, containers = next(os.walk(container_path))
# Exclude files like "README.md"
containers = [x for x in containers if x.endswith(".yaml")]
There was a problem hiding this comment.
I think this alternate logic would be better as long as we know that config is always going to be in files with a .yaml extension. That way we don't pick up an undesirable file if something besides README.md shows up.
Would there be cases of config files using a .yml extension, ie is it worthwhile to add the check x.endswith(".yml") to the test in the alternate logic.
There was a problem hiding this comment.
I agree, the alternate logic is better. The spack convention is to always use .yaml, I don't think we need .yml. (Also, the logic in the create container code assumes .yaml, because that's what it appends to the arguments.
| help_string += "\t" + config.rstrip(".yaml") + os.linesep | ||
| return help_string | ||
|
|
||
|
|
||
| def container_specs_help(): |
There was a problem hiding this comment.
The diffs in this file are confusing. It may be better checking out the current code and this PR and verify side by side in meld (which sometimes does a better job with diffs). Most of the common arguments on the left moved down into the env section, and ctr got its own arguments.
…ck/cmd/stack_cmds/create.py
Description
This PR makes specs for containers configurable, allowing us to build containers with different specs stored in separate files in a subdirectory. It also cleans up the arguments for
spack stack create [env|ctr]so that only arguments applicable for containers are available forspack stack create ctr, and similar forspack stack create env. We could do more cleanup on theenvside by stripping out options we don't use.I tested these changes manually (building container on the Ubuntu CI instance) and they are also tested in CI as part of JCSDA/spack-stack#805.
Issue(s) addressed
Resolves JCSDA/spack-stack#774
Dependencies
n/a
Impact
n/a
Checklist
spack stack create ctr: make specs configurable, remove warnings, add CI builds spack-stack#805