Skip to content
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

NotImplementedError on --fetch with external_module used in easyconfig #4298

Closed
Flamefire opened this issue Jul 19, 2023 · 4 comments · Fixed by #4308
Closed

NotImplementedError on --fetch with external_module used in easyconfig #4298

Flamefire opened this issue Jul 19, 2023 · 4 comments · Fixed by #4308
Milestone

Comments

@Flamefire
Copy link
Contributor

After a lengthy discussion in Slack and a longer search for the issue I think this is a bug:

eb EP-Stability-WF-1.0.4-intel-2020b-DD-3.37.0.eb --fetch -f
== Temporary log file in case of crash /tmp/eb-064p5o3j/easybuild-m72q5q32.log
== found valid index for /home/.local/easybuild/easyconfigs, so using it...
Traceback (most recent call last):
  File "/opt/compilers/python/3.9.4/none/lib/python3.9/runpy.py", line 197, in _run_module_as_main
    return _run_code(code, main_globals, None,
  File "/opt/compilers/python/3.9.4/none/lib/python3.9/runpy.py", line 87, in _run_code
    exec(code, run_globals)
  File "/home/.local/lib/python3.9/site-packages/easybuild/main.py", line 597, in <module>
    main()
  File "/home/.local/lib/python3.9/site-packages/easybuild/main.py", line 424, in main
    easyconfigs, generated_ecs = parse_easyconfigs(paths, validate=not options.inject_checksums)
  File "/home/.local/lib/python3.9/site-packages/easybuild/framework/easyconfig/tools.py", line 405, in parse_easyconfigs
    easyconfigs.extend(process_easyconfig(ec_file, **kwargs))
  File "/home/.local/lib/python3.9/site-packages/easybuild/framework/easyconfig/easyconfig.py", line 2069, in process_easyconfig
    ec = EasyConfig(spec, build_specs=build_specs, validate=validate, hidden=hidden)
  File "/home/.local/lib/python3.9/site-packages/easybuild/framework/easyconfig/easyconfig.py", line 514, in __init__
    self.parse()
  File "/home/.local/lib/python3.9/site-packages/easybuild/framework/easyconfig/easyconfig.py", line 750, in parse
    self['dependencies'] = remove_false_versions(self._parse_dependency(dep) for dep in self['dependencies'])
  File "/home/.local/lib/python3.9/site-packages/easybuild/framework/easyconfig/easyconfig.py", line 748, in remove_false_versions
    return [dep for dep in deps if not (isinstance(dep, dict) and dep['version'] is False)]
  File "/home/.local/lib/python3.9/site-packages/easybuild/framework/easyconfig/easyconfig.py", line 748, in <listcomp>
    return [dep for dep in deps if not (isinstance(dep, dict) and dep['version'] is False)]
  File "/home/.local/lib/python3.9/site-packages/easybuild/framework/easyconfig/easyconfig.py", line 750, in <genexpr>
    self['dependencies'] = remove_false_versions(self._parse_dependency(dep) for dep in self['dependencies'])
  File "/home/.local/lib/python3.9/site-packages/easybuild/framework/easyconfig/easyconfig.py", line 1558, in _parse_dependency
    dependency.update(self.handle_external_module_metadata(dep[0]))
  File "/home/.local/lib/python3.9/site-packages/easybuild/framework/easyconfig/easyconfig.py", line 1408, in handle_external_module_metadata
    probed_metadata = self.probe_external_module_metadata(mod_name, existing_metadata=metadata)
  File "/home/.local/lib/python3.9/site-packages/easybuild/framework/easyconfig/easyconfig.py", line 1350, in probe_external_module_metadata
    prefix = self.modules_tool.get_setenv_value_from_modulefile(mod_name, prefix_var_name)
  File "/home/.local/lib/python3.9/site-packages/easybuild/tools/modules.py", line 1180, in get_setenv_value_from_modulefile
    raise NotImplementedError
NotImplementedError

What I found:

So in summary: --fetch must not ignore the modules tool as resolving EC dependencies may not be possible without it.

Flamefire added a commit to Flamefire/easybuild-framework that referenced this issue Jul 19, 2023
The modules tool is required when resolving (at least) external dependencies.

Fixes easybuilders#4298
@boegel boegel added this to the 4.x milestone Aug 2, 2023
@boegel
Copy link
Member

boegel commented Aug 2, 2023

--fetch was deliberately implemented in #2457 such that no modules tool is required, so eb --fetch can also be used on your laptop or another system where no modules tool may be available.
With that in mind, I don't think we should start requiring a modules tool to use --fetch.

Question is then how this problem can be fixed in another way.

Maybe get_setenv_value_from_modulefile should be implemented generically in ModulesTool, without requiring an actual modules tool (so without the call to get_value_from_modulefile which runs module show)?

@Flamefire
Copy link
Contributor Author

Question is then how this problem can be fixed in another way.

Maybe get_setenv_value_from_modulefile should be implemented generically in ModulesTool, without requiring an actual modules tool (so without the call to get_value_from_modulefile which runs module show)?

I have no idea how this would be possible: How could you get_*_from_modulefile without a "modulefile"?

Maybe we can rather not call probe_external_module_metadata if there is no modules tool? Not sure what external_module_metadata is used for and if it is really required.

But it all boils down to: --fetch needs to resolve dependencies which isn't possible without a modules-tool, is it?
Maybe we can add a flag to ignore the modules tool and only consider dependencies defined in ECs and download everything for that use case?

@boegel
Copy link
Member

boegel commented Aug 2, 2023

Question is then how this problem can be fixed in another way.
Maybe get_setenv_value_from_modulefile should be implemented generically in ModulesTool, without requiring an actual modules tool (so without the call to get_value_from_modulefile which runs module show)?

I have no idea how this would be possible: How could you get_*_from_modulefile without a "modulefile"?

We can get the contents of a module file without running module show, just by reading the module file directly instead?

Maybe we can rather not call probe_external_module_metadata if there is no modules tool? Not sure what external_module_metadata is used for and if it is really required.

See https://docs.easybuild.io/using-external-modules/#using_external_modules_metadata w.r.t. metadata for external modules.

It definitely not required for just fetching sources.
In fact, external modules are totally irrelevant when doing --fetch.

But it all boils down to: --fetch needs to resolve dependencies which isn't possible without a modules-tool, is it? Maybe we can add a flag to ignore the modules tool and only consider dependencies defined in ECs and download everything for that use case?

Resolving dependencies doesn't require a modules tool, that's purely done based on *dependencies in easyconfigs.
Checking which easyconfigs are installed already is what requires a modules tool (since that implies using module avail).

@Flamefire
Copy link
Contributor Author

We can get the contents of a module file without running module show, just by reading the module file directly instead?

But how do you find the module file? Especially finding the correct module file would required duplicating logic module show does (inside e.g. lmod).

Resolving dependencies doesn't require a modules tool, that's purely done based on *dependencies in easyconfigs. Checking which easyconfigs are installed already is what requires a modules tool (since that implies using module avail).

Ah ok, so installed modules are ignored and sources for all ECs in the dependency chain are collected even if they are installed, aren't they?

If so I'd say that probe_external_module_metadata can simply return an empty dict when "NoModulesTool" is used as-if it wasn't found.

Flamefire added a commit to Flamefire/easybuild-framework that referenced this issue Aug 3, 2023
… active

E.g. for `--fetch` the modules tool is explicitly set to
`None`/`NoModulesTool` to allow running on e.g. laptops to fetch sources.
This however fails for external module dependencies in which it tries to
resolve them using the modules tool which fails with `NotImplementedError`.

Simply ignore the request in that case as-if the module metadata wasn't found.

Fixes easybuilders#4298
@boegel boegel changed the title NotImplementedError on --fetch with external_module used in ECs NotImplementedError on --fetch with external_module used in easyconfig Aug 11, 2023
@boegel boegel modified the milestones: 4.x, next release (4.8.1?) Aug 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants