-
Notifications
You must be signed in to change notification settings - Fork 217
Honor sanity_check_commands & sanity_check_paths from extensions
#5012
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
base: develop
Are you sure you want to change the base?
Honor sanity_check_commands & sanity_check_paths from extensions
#5012
Conversation
8743828 to
5943e1d
Compare
e1fd987 to
4b556af
Compare
|
I would really like to see #5015 merged first so we can more easily review this... |
This now calls the inherited `EasyBlock.sanity_check_step` that handles the `sanity_check_commands` & `sanity_check_paths` for extensions too.
It is redundant as `self.is_extension` provides that information already.
f4b578f to
18417ad
Compare
|
Rebased after merge of that PR |
| '6.0', | ||
| ) | ||
| if extension != self.is_extension: | ||
| raise EasyBuildError('Unexpected value for `extension` argument. ' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This isn't exactly ignoring it, as is stated above...
What's the point of raising an error for an incorrect value being passed that's going to be ignored?!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because if we have different values it is an error somewhere.
So a) it doesn't need to be passed but b) if it is it should match the expected value
We can remove that check in EB 6 but I'd better play it safe until then
Currently those are silently ignored leading to success where it should fail.
The only official easyconfig affected by this so far is
MDAnalysis-2.4.2-foss-2021a.ebbut I ran into this while creating an easyconfigIncludes
saved_envcontext manager for restoring environment in tests #5015