Change in vdsm[master]: janitorial: add utility to overwrite error message
nsoffer at redhat.com
nsoffer at redhat.com
Thu Feb 19 16:01:49 UTC 2015
Nir Soffer has posted comments on this change.
Change subject: janitorial: add utility to overwrite error message
......................................................................
Patch Set 1:
(3 comments)
Nice! but I think we need bigger change.
http://gerrit.ovirt.org/#/c/37912/1/lib/vdsm/utils.py
File lib/vdsm/utils.py:
Line 1242: for _ in range(suffix_len))
Line 1243: return prefix + suffix
Line 1244:
Line 1245:
Line 1246: def error_with_message(code, message):
error_response(name, message) ?
But lets not add more to the utils junkyard. How about moving the relevant code to response module:
# response.py
def error(name, message=None):
...
def ok():
return {"status": {"code": ..., "message": ...}}
Line 1247: return {'status':
Line 1248: {'code': errCode[code]['status']['code'],
http://gerrit.ovirt.org/#/c/37912/1/vdsm/virt/vm.py
File vdsm/virt/vm.py:
Line 3715: diskelem.toxml(), libvirt.VIR_DOMAIN_DEVICE_MODIFY_FORCE)
Line 3716: except Exception:
Line 3717: self.log.debug("updateDeviceFlags failed", exc_info=True)
Line 3718: self.cif.teardownVolumePath(drivespec)
Line 3719: return errCode['changeDisk']
I think we should change all these use the error response function:
return response.error("changeDisk")
The code should be consistent, use same way to return a response/error. Then we can change the response format by modifying one place.
Line 3720: if vmDev in self.conf:
Line 3721: self.cif.teardownVolumePath(self.conf[vmDev])
Line 3722:
Line 3723: self.conf[vmDev] = path
Line 3746: except TimeoutError as tmo:
Line 3747: res = utils.error_with_message('ticketErr', unicode(tmo))
Line 3748: else:
Line 3749: hooks.after_vm_set_ticket(self._domain.xml, self.conf, params)
Line 3750: res = {'status': doneCode}
Same for this - should be a function to generate an ok response.
return response.OK
Line 3751: return res
Line 3752:
Line 3753: def _reviveTicket(self, newlife):
Line 3754: """
--
To view, visit http://gerrit.ovirt.org/37912
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ife293080c2c6654315a609478f3276c9008ea6e7
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei 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