derive EasyBuildError from LoggedException, deprecate log.error in favor of raise EasyBuildError#1218
Conversation
…ldError in log.error/log.exception
|
This is just a proof-of-concept, all other uses of @stdweird, @JensTimmerman, @riccardomurri: thoughts? |
There was a problem hiding this comment.
why not support EasyBuildError(msg, *args) ?
|
Refer to this link for build results (access rights to CI server needed): |
|
remarks fixed, pls rereview @stdweird? |
|
Refer to this link for build results (access rights to CI server needed): |
There was a problem hiding this comment.
thsi method is deprected and add _ to indicate it's private
|
Refer to this link for build results (access rights to CI server needed): |
|
remarks fixed, please revisit before I tackle all remaining occurences of |
There was a problem hiding this comment.
I may have to revisit this bit, since the tests treat all log.deprecated statements being hit as errors (by defining EASYBUILD_DEPRECATED to same ridiculously high version)
I don't think that'll work out well (since we'll be hitting this one all the time). Not sure where I can move this though for proper deprecation of _error_no_raise...
There was a problem hiding this comment.
After giving this some more thought, I don't think we should be using log.deprecated at all here...
The idea behind the log.deprecated is to indicate that deprecated functionality, i.e. stuff that will be removed in the future, is being triggered.
In this case _error_no_raise actually implement the behaviour that we want to be using, so it shouldn't trigger log.deprecated at all... Leaving it like this, we'll be triggering log.deprecated whenever an EasyBuildError is created.
I'll see if I can refurbish things to log.error being the behaviour we want (no raise), and move the bit currently in error that does the raise in a separate method that does contain log.deprecated.
I'm not sure that will fly though, since by default we actually want log.error to still raise, for now (to retain backward-compatibility).
There was a problem hiding this comment.
then simply add comment that indicates that method should never be used and that we are going to remove this in 3.0. no need to hack more around. it's already bad enough 😄
|
Refer to this link for build results (access rights to CI server needed): |
|
Refer to this link for build results (access rights to CI server needed): |
|
and again... Jenkins: test this please |
|
Jenkins: test this please |
|
Refer to this link for build results (access rights to CI server needed): |
|
Jenkins: test this please |
|
documentation has been updated accordingly, see http://easybuild.readthedocs.org/en/latest/Deprecated-functionality.html#report-errors-by-raising-easybuilderror-rather-than-using-log-methods |
|
Refer to this link for build results (access rights to CI server needed): |
|
Jenkins: test this please |
|
Refer to this link for build results (access rights to CI server needed): |
|
Refer to this link for build results (access rights to CI server needed): |
|
Jenkins: test this please |
2 similar comments
|
Jenkins: test this please |
|
Jenkins: test this please |
|
Refer to this link for build results (access rights to CI server needed): |
|
Refer to this link for build results (access rights to CI server needed): |
|
Refer to this link for build results (access rights to CI server needed): |
|
Refer to this link for build results (access rights to CI server needed): |
There was a problem hiding this comment.
I need to change this using Fancylogger.error, otherwise errors are reporting in the logs like:
== 2015-03-31 13:34:58,605 main ERROR EasyBuild crashed with an error
(at easybuild/tools/build_log.py:136 in _error_no_raise):
Can't find path /Users/kehoste/work/easybuild-framework/foo.eb
There was a problem hiding this comment.
while it used to be like:
ERROR: EasyBuild crashed with an error (at easybuild/framework/easyconfig/tools.py:317 in parse_easyconfigs):
Can't find path /Users/kehoste/work/easybuild-framework/foo.eb
There was a problem hiding this comment.
I'll handle this in a follow-up PR.
|
That's it, I'm merging in this beast. Progress! Thanks for the review @stdweird! |
derive EasyBuildError from LoggedException, deprecate log.error in favor of raise EasyBuildError
Only used in one place and all error logging should happen in the constructor of `EasyBuildError` since easybuilders#1218
replaces #1183
depends on
hpcugent/vsc-base#160hpcugent/vsc-base#163update: see http://easybuild.readthedocs.org/en/latest/Deprecated-functionality.html#report-errors-by-raising-easybuilderror-rather-than-using-log-methods