Irit Goihman has posted comments on this change.
Change subject: jsonrpc: introduce new client
......................................................................
Patch Set 13: Verified+1
(3 comments)
https://gerrit.ovirt.org/#/c/64502/10/lib/vdsm/client.py
File lib/vdsm/client.py:
Line 108: ServerError will be raised if the command encountered execution error:
Line 109:
Line 110: vdsm.client.ServerError: Vdsm request failed
Line 111: (code=4, message=Virtual machine already exists)
Line 112:
you repeat the errors description twice (here and in the call
method). I gu
It is intentional. This description is the more informative one, with
guidelines for developers who are willing to use this module, and the description in the
call method is part of the method description. The next update should be responsible and
update both places.
Line 113:
Line 114: Please note that if the client isn't a context manager, it should be
Line 115: closed at the end of the run:
Line 116:
Line 192: if args is None:
Line 193: args = {}
Line 194: req = yajsonrpc.JsonRpcRequest(method, args, reqId=str(uuid.uuid4()))
Line 195: try:
Line 196: responses = self._client.call(req, timeout=timeout)
why not just have it as default values? like "def call(self,
method, args={
Done
Line 197: except EnvironmentError as e:
Line 198: raise ClientError(e)
Line 199:
Line 200: if not responses:
Line 224: if t is None:
Line 225: raise
Line 226
Line 227
Line 228
why is this type check ?
__exit__ should only raise exception
if it was raised during exit. So if t (type) is None it means an exception wasn't
raised during the execution of the context manager and we have to handle the exception
raised in __exit__. t,v, tb represent the exception that was raised before exiting the
context manager and if they're not None, the raised exception from __exit__ will
override it.
--
To view, visit
https://gerrit.ovirt.org/64502
To unsubscribe, visit
https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Idd45d7e88bf2246beaf30550b12201917f32c354
Gerrit-PatchSet: 13
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Irit Goihman <igoihman(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Irit Goihman <igoihman(a)redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Sivák <msivak(a)redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Simone Tiraboschi <stirabos(a)redhat.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org>
Gerrit-HasComments: Yes