Kobi Ianko has posted comments on this change. Change subject: Refactoring reportError func ......................................................................
Patch Set 1: (3 comments) http://gerrit.ovirt.org/#/c/28482/1/vdsm/virt/vm.py File vdsm/virt/vm.py: Line 4476: try: Line 4477: target = int(target) Line 4478: self._dom.setMemory(target) Line 4479: except ValueError: Line 4480: return self._reportError(msg='an integer is required for target', entity='balloon target') > please adhere to PEP8. sure thing :) Line 4481: except libvirt.libvirtError as e: Line 4482: if e.get_error_code() == libvirt.VIR_ERR_NO_DOMAIN: Line 4483: return self._reportError(key='noVM', entity='balloon target') Line 4484: return self._reportError(msg=e.message,entity='balloon target') 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): > Please add a docstring explaining what this function does and where it can My thoughts are that this should be done on a different patch, and does not relate directly to the feature 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) > please use self.log.exception() that does not need an explicity exc_info=Tr same here. I'll be happy to do it once my duties are 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 <[email protected]> Gerrit-Reviewer: Dan Kenigsberg <[email protected]> Gerrit-Reviewer: Doron Fediuck <[email protected]> Gerrit-Reviewer: Kobi Ianko <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
