Yaniv Bronhaim has posted comments on this change. Change subject: [wip] jsonrpc: Vdsm changes ......................................................................
Patch Set 13: Code-Review-1 (10 comments) .................................................... Commit Message Line 3: AuthorDate: 2013-11-29 12:33:29 +0100 Line 4: Commit: pkliczewski <[email protected]> Line 5: CommitDate: 2014-01-02 16:43:20 +0100 Line 6: Line 7: [wip] jsonrpc: Vdsm changes when will you plan to remove the wip? Line 8: Line 9: Here are engine changes: http://gerrit.ovirt.org/#/c/20926/ Line 10: Line 11: Changes are done on top of Saggi's reimplementation of async tcp server Line 8: Line 9: Here are engine changes: http://gerrit.ovirt.org/#/c/20926/ Line 10: Line 11: Changes are done on top of Saggi's reimplementation of async tcp server Line 12: using json. The changes include: on top of? is it part of that patch or you depend on something ? or there is a link for those changes? Line 13: - Fixing ssl Line 14: - Gluster api support Line 15: - Fixing number of issues around Bridge.py and json binding Line 16: - Update vdsm schema and code to be consistent Line 9: Here are engine changes: http://gerrit.ovirt.org/#/c/20926/ Line 10: Line 11: Changes are done on top of Saggi's reimplementation of async tcp server Line 12: using json. The changes include: Line 13: - Fixing ssl what was wrong in ssl? Line 14: - Gluster api support Line 15: - Fixing number of issues around Bridge.py and json binding Line 16: - Update vdsm schema and code to be consistent Line 17: - getAllVmStats was only available for xml so it was moved to API Line 11: Changes are done on top of Saggi's reimplementation of async tcp server Line 12: using json. The changes include: Line 13: - Fixing ssl Line 14: - Gluster api support Line 15: - Fixing number of issues around Bridge.py and json binding elaborate Line 16: - Update vdsm schema and code to be consistent Line 17: - getAllVmStats was only available for xml so it was moved to API Line 18: - vm snapshot method had not optional argument Line 19: - added implementation for full vm list Line 12: using json. The changes include: Line 13: - Fixing ssl Line 14: - Gluster api support Line 15: - Fixing number of issues around Bridge.py and json binding Line 16: - Update vdsm schema and code to be consistent I prefer the update of the schema in separate patch ? is it possible? Line 17: - getAllVmStats was only available for xml so it was moved to API Line 18: - vm snapshot method had not optional argument Line 19: - added implementation for full vm list Line 20: - added gerHardwareInfo method Line 13: - Fixing ssl Line 14: - Gluster api support Line 15: - Fixing number of issues around Bridge.py and json binding Line 16: - Update vdsm schema and code to be consistent Line 17: - getAllVmStats was only available for xml so it was moved to API remove tabs Line 18: - vm snapshot method had not optional argument Line 19: - added implementation for full vm list Line 20: - added gerHardwareInfo method Line 21: .................................................... File lib/yajsonrpc/__init__.py Line 98: method = obj.get("method") Line 99: if method is None: Line 100: raise JsonRpcInvalidRequestError() Line 101: Line 102: reqId = obj.get("id") id is not int anymore ? why do you remove the check ? Line 103: Line 104: params = obj.get('params', []) Line 105: if not isinstance(params, (list, dict)): Line 106: raise JsonRpcInvalidRequestError() .................................................... File tests/jsonRpcTests.py Line 198: {'text': data}, Line 199: 10, CALL_TIMEOUT), Line 200: data) Line 201: Line 202: @brokentest('fail with "error: [Errno 9] Bad file descriptor"') what about the rest that defined as broken ? Line 203: @permutations(CONNECTION_PERMUTATIONS) Line 204: def testMethodMissingMethod(self, rt, ssl): Line 205: bridge = _DummyBridge() Line 206: with constructServer(rt, bridge, ssl) as (server, clientFactory): .................................................... File vdsm/API.py Line 651: ip = '0' Line 652: self.log.info('network %s: using %s', network, ip) Line 653: return ip Line 654: Line 655: def snapshot(self, snapDrives, snapMemVolHandle=None): do it separately in external patch. it fixes a bug Line 656: v = self._cif.vmContainer.get(self._UUID) Line 657: if not v: Line 658: return errCode['noVM'] Line 659: memoryParams = {} .................................................... File vdsm/gluster/vdsmapi-gluster-schema.json Line 1: # think all those scheme changes can be separately , and not related to jsonrpc Line 2: # Copyright 2013 Red Hat, Inc. Line 3: # Line 4: # This program is free software; you can redistribute it and/or modify Line 5: # it under the terms of the GNU General Public License as published by -- To view, visit http://gerrit.ovirt.org/19497 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: If828355b7efe28fe6a2e784069425fefd2f3f25c Gerrit-PatchSet: 13 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Saggi Mizrahi <[email protected]> Gerrit-Reviewer: Adam Litke <[email protected]> Gerrit-Reviewer: Barak Azulay <[email protected]> Gerrit-Reviewer: Eduardo <[email protected]> Gerrit-Reviewer: Piotr Kliczewski <[email protected]> Gerrit-Reviewer: Saggi Mizrahi <[email protected]> Gerrit-Reviewer: Yaniv Bronhaim <[email protected]> Gerrit-Reviewer: mooli tayer <[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
