probe external modules for missing metadata that is not provided via extermal module metadata file #3174
Conversation
We got a request from one of our customers to enable the module names always as lowercases. This is the implementation of this feature. We would like to contribute this feature back.
Adding the lowercase module name scheme
This commit handles metadata for external module dependencies when there is not entry in the metadata file It should look for the pair of variables definitions in the modules in the given order 1. CRAY_XXXX_PREFIX and CRAY_XXXX_VERSION 2. CRAY_XXXX_DIR and CRAY_XXXX_VERSION 2. CRAY_XXXX_ROOT and CRAY_XXXX_VERSION 5. XXXX_PREFIX and XXXX_VERSION 4. XXXX_DIR and XXXX_VERSION 5. XXXX_ROOT and XXXX_VERSION 3. XXXX_HOME and XXXX_VERSION If no pair is found, an empty dependency dictionary is returned, as is the default behaviour This avoids the need for a very large metadata file.
| prefix = self.modules_tool.get_variable_from_modulefile(dep_name, p) | ||
| version = self.modules_tool.get_variable_from_modulefile(dep_name, v) | ||
|
|
||
| if prefix and version: |
There was a problem hiding this comment.
if prefix is found but not version, and if the version is in dep_name, can't we simply use that one?
There was a problem hiding this comment.
The point is to only trust the metadata obtained by probing the module if it provides both the installation prefix and software version.
If it only provides one of both, we could have a "lucky hit", and the obtained metadata may be less trustworthy...
Let's go forward as is with this strict requirement, which can easily be loosened up later (the other way around is more difficult).
The other scenario, where the prefix is already known from the metadata file, but (only) the version is available through the module, may actually be more likely (since the prefix can be specified as the name of the environment variable specifying the installation prefix, not an actual hardcoded prefix, which can be done generically in the metadata file).
This does not fix the style checks
|
|
||
| short_ext_modname = dep_name.split('/')[0] | ||
| if not 'name' in dependency: | ||
| dependency['name'] = [short_ext_modname] |
There was a problem hiding this comment.
the test is failing because this is added regardless of finding the rest of the metadata, if this is moved inside the if dep_prefix and dep_version: block then it doesn't create a problem
There was a problem hiding this comment.
fixed, thanks for pointing this out @migueldiascosta!
sync with develop
….get_setenv_value_from_modulefile, and add dedicated test for it
…_module_metadata methods in EasyConfig class
…es for missing metadata
…tation in MockModule class used in robot tests
There was a problem hiding this comment.
@victorusu I've reworked this a bit with the changes in victorusu#5.
Most significant changes:
- renaming of
_handle_ext_module_metadata_by_probing_modulestoprobe_external_module_metadata - changed logic in
handle_external_module_metadata - fixed broken test (see @migueldiascosta's suggestion, thanks!)
- renamed
get_variable_from_modulefiletoget_setenv_value_from_modulefile - added tests fo both
get_setenv_value_from_modulefileand probing of external modules for missing metadata
…ns prefix value in probe_external_module_metadata
fixes, cleanup, enhancements and tests for probing external modules for missing metadata
boegel
changed the title
This commit handles metadata for external module dependencies when there is no entry in the metadata file
It should look for the pair of variables definitions in the modules in the given order
If no pair is found, an empty dependency dictionary is returned, as is the default behavior.
This avoids the need for a very large metadata file.