add 'parse' hook to add support for tweaking 'raw' easyconfig (REVIEW) #2562
Conversation
easybuild/tools/hooks.py
Outdated
| except ImportError as err: | ||
| raise EasyBuildError("Failed to import hooks implementation from %s: %s", hooks_path, err) | ||
| else: | ||
| raise EasyBuildError("Provided path for hooks implementation should be location of a Python file (*.py)") |
There was a problem hiding this comment.
line too long (121 > 120 characters)
easybuild/tools/hooks.py
Outdated
| except ImportError as err: | ||
| raise EasyBuildError("Failed to import hooks implementation from %s: %s", hooks_path, err) | ||
| else: | ||
| raise EasyBuildError("Provided path for hooks implementation should be location of a Python file (*.py)") |
There was a problem hiding this comment.
line too long (121 > 120 characters)
…o also trigger parse_hook
… function, to avoid calling parse_hook before start_hook
boegel
changed the title
|
@akesandgren Good to go? |
|
Yeah i think so, can't find any problems with it. |
|
@vanzod Any thoughts on this? |
|
@ocaisa To answer the issue you raised during today's conf call: any changes made to the So, unless you make sure you have the hooks active every time, picking up a dumped easyconfig file later may result in surprises... We can look into fixing that, but maybe some people wouldn't expect things to trickle down, I'm not quite sure (I can see problems with both approaches), and I'm not sure how easy it would be either to make things trickle down (the classic "there be dragons here" probably applies...). |
|
I for one would expect it NOT to end up in the dumped one, but it might be good to have a comment added that hooks where changing stuff and whether the dumped result is with or without the changes done by hooks. But if someone wants the changes to end up in the dump, this should probably be configurable. |
|
@boegel I wouldn't have a problem with the archived easyconfig not including the changes but I think I would expect to see them in the |
|
Yes, that would be good, didn't think of that one. |
see #2557
cc @akesandgren