add support for easystack file that contains easyconfig filenames + implement parsing of configuration options#4021
Conversation
|
Let me see if I get this right: essentially, you want to support an additional yaml structure where we don't specify the packages to be installed using e.g. You would specify Correct? Some thoughts: It's a lot more compact, but one of the main reasons we opted for the first (hierarchical) structure, is that you probably want to add certain options at the software level (e.g. for all Another question that came to mind: how will we specify options here? I'm no YAML guru and don't know what's allowed/possible here, but can we do e.g. or something? Or do you then envision something more like: The second is easier to write, but harder to parse |
|
Just did a local test with Running That works fine. Mixing |
casparvl
left a comment
There was a problem hiding this comment.
Looks reasonable overall, but lets discuss the individual items I touched upon :)
Mainly because the current format is overly verbose for the common case where there's nothing special going on (no specific configuration options needed), etc.
Good question, I didn't think that through yet. I think something like this could work: easyconfigs:
example-1.2.3:
options: --from-pr 123
example-4.5.6I'll see if I can add dummy support for |
…without explicit '.eb' extension
…o top level item was specified in the YAML
|
I've implemented the dummy support for option passing, will push in a second. The support that I've added is that the parser can now understand this format: i.e. We can later quite easily have the parser store that in a data structure that satisfies the format proposed here for |
…in easystack files. As discussed here easybuilders#4021 (comment)
…ck files based on the 'easyconfig' top level keyword
…ybuild-framework into easystack_easyconfigs
…s neededso that the keys in the option dictionary can be expected to be the full EasyConfig names
…ndex the yaml dict _with_ the eb suffix, even though that dict contains the names _without_ suffix
| msg += "Instead found keys: %s" % dict_keys | ||
| raise EasyBuildError(msg) | ||
|
|
||
| return easystack |
There was a problem hiding this comment.
Shouldn't we verify that
- The easyconfigs can actually be found
- The options provided for each (if any) are valid
There was a problem hiding this comment.
We could consider resolving these with the robot and returning the full path to the ecs (and outputting these in debug mode).
We could also consider an option top prefer easyconfig files that are stored in the same path as the easystack file (which may not be in the robot path)...but I think that is a question of taste
There was a problem hiding this comment.
The job of the easystack file is to just provide a list of easyconfigs, nothing more. It basically replaces listing easyconfig filenames in the eb command.
Checking whether the easyconfigs can actually be found is done later through main (via det_easyconfig_paths).
Likewise for the options: passing those to the option parser, which will check if the options are valid, is done later (that's a part of @casparvl's PR #4057)
…nment' that one would get if the easystack file already included .eb extensions
…RL where it makes sense
1ccfb86 to
0550fc0
Compare
|
Documentation will be updated to mention support for top-level |
|
|
||
| return easyconfig_names, general_options | ||
| _log.debug("Using EasyConfig specific options based on the following dict:") | ||
| _log.debug(easystack.ec_opts) |
There was a problem hiding this comment.
We could use pprint.pformat here to make it more readable (let's do that in #4057)
| @@ -266,7 +265,7 @@ def main(args=None, logfile=None, do_build=None, testing=False, modtool=None): | |||
| # TODO add general_options (i.e. robot) to build options | |||
| orig_paths, general_options = parse_easystack(options.easystack) | |||
There was a problem hiding this comment.
general_options is not correct here, but this is being fixed in #4057 (renamed to opts_per_ec)
No description provided.