Change in vdsm[master]: bridge: usage of yaml schema

alitke at redhat.com alitke at redhat.com
Thu Mar 17 18:06:21 UTC 2016


Adam Litke has posted comments on this change.

Change subject: bridge: usage of yaml schema
......................................................................


Patch Set 5:

(2 comments)

https://gerrit.ovirt.org/#/c/54528/5/lib/yajsonrpc/__init__.py
File lib/yajsonrpc/__init__.py:

Line 70: class JsonRpcInvalidParamsError(JsonRpcError):
Line 71:     def __init__(self, msg=None):
Line 72:         if not msg:
Line 73:             msg = "Invalid method parameter(s)."
Line 74:         JsonRpcError.__init__(self, -32602, msg)
This is very unhelpful.  We are not told which parameter is invalid or what the offending value is.  Why not just fix the code that raises this exception to always provide a meaningful message?
Line 75: 
Line 76: 
Line 77: class JsonRpcInternalError(JsonRpcError):
Line 78:     def __init__(self, msg=None):


https://gerrit.ovirt.org/#/c/54528/5/tests/bridgeTests.py
File tests/bridgeTests.py:

Line 93
Line 94
Line 95
Line 96
Line 97
This is style fixing only.  I support doing this cleanup but it should be separated from changes that affect logic.  Applies to this whole patch, not just this file.


-- 
To view, visit https://gerrit.ovirt.org/54528
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia76d8245568514d20e446237bd667d87fb4ad3e8
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Piotr Kliczewski <piotr.kliczewski at gmail.com>
Gerrit-Reviewer: Adam Litke <alitke at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: Jenkins CI
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