Kobi Ianko has posted comments on this change.

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


Patch Set 1:

(2 comments)

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

Line 4490:             # persist the target value to make it consistent after 
recovery
Line 4491:             self.saveState()
Line 4492:             return {'status': doneCode}
Line 4493: 
Line 4494:     def _reportError(self, key='Err', msg=None, entity=None):
> My thoughts are that this should be done on a different patch, and does not
done, part from the 'Err', I have no idea what was the poet intentions, prefer 
not to change this at this point.
Line 4495:         if entity is not None:
Line 4496:             self.log.error("Set new " + entity + " failed", 
exc_info=True)
Line 4497:         if msg is None:
Line 4498:             error = errCode[key]


Line 4492:             return {'status': doneCode}
Line 4493: 
Line 4494:     def _reportError(self, key='Err', msg=None, entity=None):
Line 4495:         if entity is not None:
Line 4496:             self.log.error("Set new " + entity + " failed", 
exc_info=True)
> same here.
Done
Line 4497:         if msg is None:
Line 4498:             error = errCode[key]
Line 4499:         else:
Line 4500:             error = {'status': {'code': errCode[key]


-- 
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: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Kobi Ianko <k...@redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <dan...@redhat.com>
Gerrit-Reviewer: Doron Fediuck <dfedi...@redhat.com>
Gerrit-Reviewer: Kobi Ianko <k...@redhat.com>
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Reply via email to