-
Notifications
You must be signed in to change notification settings - Fork 214
Make specifying exts_defaultclass
optional
#4800
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?
Make specifying exts_defaultclass
optional
#4800
Conversation
a106f61
to
c7ec26d
Compare
@Flamefire I'll pick up on this again after the release of EasyBuild v5.1.0, since this change is way more involved than the last-minute regression bug fix being done in #4888 ... |
f4f4579
to
fc9cd40
Compare
Ok, I rebased this after #4686 caused this to conflict. Although I think the change is not so complex: We only use the property in 2 places in a single file which are both modified here. So to me the change is very local. Or is 5.1.0 so immanent there is no time for that? |
- Some places could use `dict.items`. - Pylint supressions added - Ignore G002 (Logging statements should not use % formatting for their first argument) - Early return for skipped tests to avoid indent
It is NOT required when all extensions have either custom easyblocks or an `easyblock` key in their option dict. In those cases `exts_defaultclass` is redundant making it harder to write the EasyConfig. Check for a missing easyblock when actually using it so the error will be just a bit later but still (almost) the same.
fc9cd40
to
90b3789
Compare
@boegel Ran into this again, can we merge this? |
It is NOT required when all extensions have either custom easyblocks or an
easyblock
key in their option dict.In those cases
exts_defaultclass
is redundant making it harder to write the EasyConfig.Check for a missing easyblock when actually using it so the error will be just a bit later but still (almost) the same.
One noteworthy semi-related change is the try-finally to the
clean_up_fake_module
call. We did that only when the exts_defaultclass was missing but not when anything else fails which looks like an omission. To keep at least the behavior for this the try-finally is required as the exception comes frominit_ext_instances
now. As a (IMO good) side effect it will also be called for any other error happening along the way, e.g. the similar error that the default easyblock is not foundAlso some small drive-by fixes without any functional change to reduce the amount of linter warnings in the touched files.
Recent example usage for a CMakePythonPackage Easyconfig:
But also applies if only software-specific easyblocks are used and we really don't want a default