Skip to content

Conversation

@boegel
Copy link
Member

@boegel boegel commented May 22, 2018

fix for #2499 reported by @mboisson based on suggestion by @ocaisa

:param priority: priority for this path in $MODULEPATH (Lmod-specific)
"""
if priority:
self.log.info("Ignoring specified priority '%s' when prepending %s to $MODULEPATH", priority, path)
Copy link
Member

Choose a reason for hiding this comment

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

I would add something here that it's called in use, not a prepend_path?

Copy link
Member Author

Choose a reason for hiding this comment

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

fixed

mkdir(path, parents=True)

if priority:
self.run_module(['use', '--priority', str(priority), path])
Copy link
Member

Choose a reason for hiding this comment

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

I thought it was Lmod specific?

Copy link
Member Author

Choose a reason for hiding this comment

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

It is, this use implementation is custom to the Lmod class.

:param priority: priority for this path in $MODULEPATH (Lmod-specific)
"""
if priority:
self.log.info("Ignoring specified priority '%s' when running 'module use %s'", priority, path)
Copy link
Member

Choose a reason for hiding this comment

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

while in the code it's clear, I wonder if the log messages should also mention why the priority is ignored, for those not using Lmod

@boegel boegel changed the title prepend location for temporary module file to $MODULEPATH with high priority in load_fake_module method prepend location for temporary module file to $MODULEPATH with high priority + mark it as default in load_fake_module method May 23, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants