This is an automatically generated e-mail. To reply, visit: http://reviewboard-fedoraserver.rhcloud.com/r/189/ |
On August 18th, 2015, 5:35 p.m. UTC, Stephen Gallagher wrote:
src/rolekit/async.py (Diff revision 1) def error_handler_with_conversion(e):170 # We can’t use log.exception() because the traceback is no longer available.
170 # We can’t use log.exception() because the traceback is no longer available.
171 # So the three cases in dbus_handle_exceptions amount to just this.
171 # So the three cases in dbus_handle_exceptions amount to just this.
172 if not isinstance(e, DBusException):172 if not isinstance(e, DBusException):173 log.error("{0}: {1}".format(type(e), str(e)))173 log.error("{0}: {1}".format(type(e), str(e)))174 if not isinstance(e, RolekitError):175 log.exception()This change really needs a comment to explain it. Why is it okay to call log.exception() if we have something other than a RolekitError here? That disagrees with the comment on lines 170-171.
On August 25th, 2015, 10:07 a.m. UTC, Nils Philippsen wrote:
I can't say much about the previous comment because I didn't write it :). All I know is that I had some issue throwing exceptions on the daemon side and no way to find out where exactly it bombed. Adding the call to
log.exception()
logged the complete traceback just as I would expect it. Do you have some background for me? Then I could maybe change the existing comment to reflect what's really going on.
I think the comment is not correct. A traceback in this place is properly logged in my tests.
- Thomas
On August 12th, 2015, 3:43 p.m. UTC, Nils Philippsen wrote:
Review request for RoleKit Mailing List, Miloslav Trmac, Nils Philippsen, Stephen Gallagher, and Thomas Woerner.
By Nils Philippsen.
Updated Aug. 12, 2015, 3:43 p.m.
Repository:
rolekit
Description
Diffs
|