Change in vdsm[master]: events: introduction of schema
ybronhei at redhat.com
ybronhei at redhat.com
Sun Apr 3 06:21:00 UTC 2016
Yaniv Bronhaim has posted comments on this change.
Change subject: events: introduction of schema
......................................................................
Patch Set 7: Code-Review-1
(2 comments)
https://gerrit.ovirt.org/#/c/54754/4/lib/api/vdsmapi.py
File lib/api/vdsmapi.py:
Line 81: self._methods = {}
Line 82: self._types = {}
Line 83: try:
Line 84: for path in paths:
Line 85: with open(path) as f:
this get be done separately
Line 86: loaded_schema = yaml.load(f)
Line 87: types = loaded_schema.pop('types')
Line 88: self._types.update(types)
Line 89: self._methods.update(loaded_schema)
https://gerrit.ovirt.org/#/c/54754/7/lib/vdsm/rpc/Bridge.py
File lib/vdsm/rpc/Bridge.py:
Line 101: self._schema.get_method(method)
Line 102: except (KeyError, ValueError):
Line 103: raise yajsonrpc.JsonRpcMethodNotFoundError(method)
Line 104:
Line 105: className, methodName = method.split('.', 1)
it can raise "ValueError: need more than 1 value to unpack" if something wrong in "method". why do you take out of the try block? please do it in separate patch with explanation
Line 106: return partial(self._dynamicMethod, className, methodName)
Line 107:
Line 108: def _convert_class_name(self, name):
Line 109: """
--
To view, visit https://gerrit.ovirt.org/54754
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I43a9f1852b1459a2e905244049507704f0187201
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Piotr Kliczewski <piotr.kliczewski at gmail.com>
Gerrit-Reviewer: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Mucha <mmucha at redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski at gmail.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei at redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation at ovirt.org>
Gerrit-HasComments: Yes
More information about the vdsm-patches
mailing list