Change in vdsm[master]: Refactoring reportError func

danken at redhat.com danken at redhat.com
Mon Jun 9 13:31:02 UTC 2014


Dan Kenigsberg has posted comments on this change.

Change subject: Refactoring reportError func
......................................................................


Patch Set 5: Code-Review-1

(2 comments)

http://gerrit.ovirt.org/#/c/28482/5/vdsm/virt/vm.py
File vdsm/virt/vm.py:

Line 4492:             return {'status': doneCode}
Line 4493: 
Line 4494:     def _reportError(self, key='Err', msg=None):
Line 4495:         """
Line 4496:         Print an error and return an error status.
a more precise statement would be

 """Convert an exception to an error status.

This method should be called only within exception-handling context.
"""
Line 4497:         This method should be called in cases of exception.
Line 4498:         """
Line 4499:         self.log.exception("Operation failed")
Line 4500:         if msg is None:


Line 4497:         This method should be called in cases of exception.
Line 4498:         """
Line 4499:         self.log.exception("Operation failed")
Line 4500:         if msg is None:
Line 4501:             error = errCode[key]
the "poet" asked about the default value of "key", which makes no sense to him.
Please drop the default value (or explain why it's useful).

As it stands, your code would explode on
  
  self._reportError(msg='an integer is required for target')
Line 4502:         else:
Line 4503:             error = {'status': {'code': errCode[key]
Line 4504:                                 ['status']['code'], 'message': msg}}
Line 4505:         return error


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I041a662f4fe02f671a7335d7eca84704443f5bee
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Kobi Ianko <kobi at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Doron Fediuck <dfediuck at redhat.com>
Gerrit-Reviewer: Kobi Ianko <kobi at redhat.com>
Gerrit-Reviewer: automation at ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list