Change in vdsm[master]: utils: Add unhandled exceptions logging decorator

danken at redhat.com danken at redhat.com
Wed Nov 27 13:36:47 UTC 2013


Dan Kenigsberg has posted comments on this change.

Change subject: utils: Add unhandled exceptions logging decorator
......................................................................


Patch Set 1:

(1 comment)

....................................................
File lib/vdsm/utils.py
Line 592:             try:
Line 593:                 return f(*a, **kw)
Line 594:             except Exception:
Line 595:                 log = logging.getLogger(on)
Line 596:                 log.exception(msg)
Functionally speaking, logged() swallows the exception. It does not do it silently, but program execution continues as if the function never failed, returning None. This is too C-style imo.

To prove my point, If main() in your example is an entry point to a script, decorating it with @logged means that the script would always return 0, misleading a even the rare prudent scripter that used `sh -e`.

I do not have a short name for "log unhandled exception and return None". That's another reason to re-raise the exception.
Line 597:         return wrapper
Line 598:     return decorator
Line 599: 
Line 600: 


-- 
To view, visit http://gerrit.ovirt.org/21778
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I39265bac4f26fefa365bad080dafb6106145f846
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: Allon Mureinik <amureini at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Daniel Erez <derez at redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce at redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi at redhat.com>
Gerrit-Reviewer: Sergey Gotliv <sgotliv at redhat.com>
Gerrit-Reviewer: Vered Volansky <vvolansk at redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list