Bugfix: allow missing modules if they are blacklisted by scheibelp · Pull Request #13540 · spack/spack

scheibelp · 2019-11-01T18:22:03Z

For spack module loads and spack module find, allow missing upstream modules (print a debug message vs. raising an error); also skip local modules if they are blacklisted.

Old behavior:

spack module find will report an error if the root spec's module or any dependency module does not exist (regardless of whether it is blacklisted)
spack module loads works the same

New behavior: the short version is that the module finding is now more lenient (especially for blacklisted modules):

spack module find will report an error if the root spec's module does not exist, unless
- the module for the spec is blacklisted
  - For the root package, a debug message is output so that the user could check this with spack -d...
  - For any dependency package (when requesting dependency modules with spack module find) a message will not be reported (it's an error if the module is not blacklisted, but if it is blacklisted, the module is silently skipped)
- the spec is associated with an upstream
  - instead of throwing exceptions when the module isn't find, the logic now prints out a debug-level message so the user has a hint of what is missing if they were expecting to see a module file for an upstream package
spack module loads works exactly the same way as the new spack module find, but informational messages are not generated for blacklisted packages

…error); also skip local modules if they are blacklisted

scheibelp · 2019-11-08T01:46:20Z

@tgamblin this should add tests (e.g. for spack module find when there is a blacklist) but do you agree with the intended behavior in the PR description?

tgamblin

@scheibelp: LGTM, though I think it would be good if @alalazo also took a look.

My only request is more tests before we merge this.

…form the test with lmod for the module command test; for now I'm doing the latter because it's easier

tgamblin · 2019-12-03T05:53:28Z

@glennpj: does this fix #13346 for you?

tgamblin

@scheibelp: One minor change request: use return None instead of return for functions that are meant to return a value.

And one larger question that @becker33 made me think of: I am not sure these should just be debug messages. I suspect that if a user is using spack env loads, they will want to know that the behavior will be different from spack env activate for the same environment -- specifically that some packages will be omitted. My personal opinion on this is that the user should just activate the environment and not bother with modules in the first place, but I think with blacklisting it's not entirely intuitive.

How much work would it be to:

Add commented lines to the loads file for each blacklisted module that would have otherwise been loaded.
Emit one warning after spack env loads -- something like "modules for some packages do not exist due to blacklisting -- see <file> for details, and consider using spack env activate".

I think this is better behavior than (mostly) silently logging the missing modules.

lib/spack/spack/modules/common.py

glennpj · 2019-12-03T16:44:28Z

@glennpj: does this fix #13346 for you?

Yes, it does.

…dules are omitted (blacklisted or missing from upstream); update 'module loads' output to explicitly mention missing modules in a comment

lib/spack/spack/modules/common.py

scheibelp · 2019-12-04T21:03:49Z

@tgamblin changes are now addressed:

explicit return of None
updated docstring (per Bugfix: allow missing modules if they are blacklisted #13540 (comment))
loads output includes comments for missing (blacklisted/upstream) modules (note that the comment mentions that it could be blacklisted or that it could be missing from an upstream but doesn't distinguish) as well as an overall warning message

tgamblin

@scheibelp: Looks great to me! I think this will help people understand when/which modules are not present.

`spack module loads` and `spack module find` previously failed if any upstream modules were missing. This prevented it from being used with upstreams (or, really, any spack instance) that blacklisted modules. This PR makes module finding is now more lenient (especially for blacklisted modules). - `spack module find` now does not report an error if the spec is blacklisted - instead, it prints a single warning if any modules will be omitted from the loads file - It comments the missing modules out of the loads file so the user can see what's missing - Debug messages are also printed so users can check this with `spack -d...` - also added tests for new functionality

v0.13.2 This release contains major performance improvements for Spack environments, as well as some bugfixes and minor changes. * allow missing modules if they are blacklisted (#13540) * speed up environment activation (#13557) * mirror path works for unknown versions (#13626) * environments: don't try to modify run-env if a spec is not installed (#13589) * use semicolons instead of newlines in module/python command (#13904) * verify.py: os.path.exists exception handling (#13656) * Document use of the maintainers field (#13479) * bugfix with config caching (#13755) * hwloc: added 'master' version pointing at the HEAD of the master branch (#13734) * config option to allow gpg warning suppression (#13744) * fix for relative symlinks when relocating binary packages (#13727) * allow binary relocation of strings in relative binaries (#13724)

`spack module loads` and `spack module find` previously failed if any upstream modules were missing. This prevented it from being used with upstreams (or, really, any spack instance) that blacklisted modules. This PR makes module finding is now more lenient (especially for blacklisted modules). - `spack module find` now does not report an error if the spec is blacklisted - instead, it prints a single warning if any modules will be omitted from the loads file - It comments the missing modules out of the loads file so the user can see what's missing - Debug messages are also printed so users can check this with `spack -d...` - also added tests for new functionality

scheibelp added 4 commits November 1, 2019 11:18

allow missing upstream modules (print a debug message vs. raising an …

4aad273

…error); also skip local modules if they are blacklisted

flake edits

8769eaf

Merge branch 'develop' into bugfix/module-find-relaxed

69ec323

restore expected module load order

8aa8787

adamjstewart added the modules label Nov 3, 2019

tgamblin requested a review from becker33 November 11, 2019 21:06

tgamblin requested a review from alalazo December 2, 2019 05:51

tgamblin requested changes Dec 2, 2019

View reviewed changes

scheibelp added 2 commits December 2, 2019 16:26

add test for recursive module find when there is a blacklisted module

9b05095

testing framwork requires either a refactor of fixtures or that I per…

a178b45

…form the test with lmod for the module command test; for now I'm doing the latter because it's easier

tgamblin requested changes Dec 3, 2019

View reviewed changes

lib/spack/spack/modules/common.py Outdated Show resolved Hide resolved

lib/spack/spack/modules/common.py Outdated Show resolved Hide resolved

lib/spack/spack/modules/common.py Outdated Show resolved Hide resolved

lib/spack/spack/modules/common.py Outdated Show resolved Hide resolved

scheibelp added 2 commits December 3, 2019 18:42

explicitly return None

e42f05d

add warning message for 'module find' and 'module loads' when some mo…

0754f4b

…dules are omitted (blacklisted or missing from upstream); update 'module loads' output to explicitly mention missing modules in a comment

alalazo reviewed Dec 4, 2019

View reviewed changes

lib/spack/spack/modules/common.py Outdated Show resolved Hide resolved

scheibelp added 2 commits December 4, 2019 10:37

test spack loads when blacklisting modules

ce425c9

format docstring for get_module

1915a87

scheibelp changed the title ~~[WIP] Bugfix: allow missing modules if they are blacklisted~~ Bugfix: allow missing modules if they are blacklisted Dec 4, 2019

tgamblin approved these changes Dec 5, 2019

View reviewed changes

tgamblin merged commit e5f04f9 into spack:develop Dec 5, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bugfix: allow missing modules if they are blacklisted#13540

Bugfix: allow missing modules if they are blacklisted#13540
tgamblin merged 10 commits intospack:developfrom
scheibelp:bugfix/module-find-relaxed

scheibelp commented Nov 1, 2019 •

edited by tgamblin

Loading

Uh oh!

scheibelp commented Nov 8, 2019

Uh oh!

tgamblin left a comment

Uh oh!

tgamblin commented Dec 3, 2019

Uh oh!

tgamblin left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

glennpj commented Dec 3, 2019

Uh oh!

Uh oh!

scheibelp commented Dec 4, 2019

Uh oh!

tgamblin left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Conversation

scheibelp commented Nov 1, 2019 • edited by tgamblin Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

scheibelp commented Nov 8, 2019

Uh oh!

tgamblin left a comment

Choose a reason for hiding this comment

Uh oh!

tgamblin commented Dec 3, 2019

Uh oh!

tgamblin left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

glennpj commented Dec 3, 2019

Uh oh!

Uh oh!

scheibelp commented Dec 4, 2019

Uh oh!

tgamblin left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

scheibelp commented Nov 1, 2019 •

edited by tgamblin

Loading