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

avoid running expensive 'module use' command when using Lmod as modules tool, update $MODULEPATH directly instead #3557

Merged
merged 4 commits into from
Apr 5, 2021

Conversation

Flamefire
Copy link
Contributor

I discovered an issue: The priority.
There are 2 problems:

  • module use foo won't put foo to the front if any other path has a (big) priority
  • We have a function prepend_module_path with an optional priority. Hence if we don't pass a priority then this function will not "prepend" the path

@boegel

First bullet is not a problem, that's just how module use works?
Second one: yeah, if we really want to be sure, we should give it a (very) high priority, or at least detect that we need to do so.

It is a problem if I replace module use by setting MODULEPATH because that would yield a different behavior. I currently check if any priority is in use and fallback to module use instead
We use the priority in load_fake_module, but I guess we should not. The problem is: It doesn't tell what is intended. In prepend_module_path we could (hard, but possible) check current priorities and choose a higher one. Who knows that the 10000 we currently use isn't in use already or a user used 10001?
hence I'd deprecate the priority argument

@boegel boegel added this to the release after 4.3.3 milestone Feb 3, 2021
@boegel boegel changed the title Avoid module use in LMod if possible to allow faster execution Avoid module use in Lmod if possible to allow faster execution Apr 5, 2021
@boegel boegel changed the title Avoid module use in Lmod if possible to allow faster execution avoid running expensive 'module use' command when using Lmod as modules tool, update $MODULEPATH directly instead Apr 5, 2021
Copy link
Member

@boegel boegel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@boegel boegel merged commit 89a1808 into easybuilders:develop Apr 5, 2021
@Flamefire
Copy link
Contributor Author

Follow up issue for the unadressed points: #3631
BTW: As you added tests for module unuse I think we should add the same handling for the unuse function: If no priority in effect, simply remove it.

@Flamefire Flamefire deleted the avoid_module_use branch April 6, 2021 06:47
Flamefire added a commit to Flamefire/easybuild-framework that referenced this pull request Apr 6, 2021
Followup to easybuilders#3557
No check for priorities is required as LMod correctly handles that
Also adds a check for an empty path which is technically possible and
handled by LMod like any other path
bartoldeman pushed a commit to ComputeCanada/easybuild-framework that referenced this pull request Apr 26, 2021
Followup to easybuilders#3557
No check for priorities is required as LMod correctly handles that
Also adds a check for an empty path which is technically possible and
handled by LMod like any other path
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants