Nir Soffer has posted comments on this change. Change subject: vdsm: lastclient info in jsonrpc ......................................................................
Patch Set 21: (1 comment) http://gerrit.ovirt.org/#/c/28817/21/vdsm/rpc/Bridge.py File vdsm/rpc/Bridge.py: Line 270: Line 271: retfield = command_info.get(cmd, {}).get('ret') Line 272: if isinstance(retfield, types.FunctionType): Line 273: if cmd == 'Host_getCapabilities': Line 274: ret = retfield(self._threadLocal.server, result) > I disagree, having things contained is better IMO. This is a double hack - this module provides infrastructure for hacking the return value, but it is not good enough, so we have now this ugly hack checking the name of the function, hacking the hack :-) This is not contained - hacking of getCapabilities is spread now in two places, instead of encapsulated in Host_getCapabilities. Line 275: else: Line 276: ret = retfield(result) Line 277: elif _glusterEnabled and className.startswith('Gluster'): Line 278: ret = dict([(key, value) for key, value in result.items() -- To view, visit http://gerrit.ovirt.org/28817 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I402e15cb05f89a98dab14491d9da5985335e095e Gerrit-PatchSet: 21 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Piotr Kliczewski <[email protected]> Gerrit-Reviewer: Antoni Segura Puimedon <[email protected]> Gerrit-Reviewer: Barak Azulay <[email protected]> Gerrit-Reviewer: Dan Kenigsberg <[email protected]> Gerrit-Reviewer: Nir Soffer <[email protected]> Gerrit-Reviewer: Oved Ourfali <[email protected]> Gerrit-Reviewer: Piotr Kliczewski <[email protected]> Gerrit-Reviewer: Saggi Mizrahi <[email protected]> Gerrit-Reviewer: Yaniv Bronhaim <[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
