Piotr Kliczewski has posted comments on this change.

Change subject: jsonrpcvdscli: create a client for vdsm with jsonrpc
......................................................................


Patch Set 10: Code-Review-1

(2 comments)

https://gerrit.ovirt.org/#/c/39203/10/lib/vdsm/jsonrpcvdscli.py
File lib/vdsm/jsonrpcvdscli.py:

Line 52:                             (methodName, args, e))
Line 53: 
Line 54:         req = JsonRpcRequest(method, args, reqId=str(uuid4()))
Line 55:         call = self._client.call_async(req)
Line 56:         call.wait(CALL_TIMEOUT)
I would update method call on client to accept time out with default value set 
to CALL_TIMEOUT.
Line 57: 
Line 58:         resp = call.responses[0]
Line 59: 
Line 60:         if resp.error is not None:


Line 54:         req = JsonRpcRequest(method, args, reqId=str(uuid4()))
Line 55:         call = self._client.call_async(req)
Line 56:         call.wait(CALL_TIMEOUT)
Line 57: 
Line 58:         resp = call.responses[0]
What if the response have not arrived before timeout?

We do not check whether timeout occurred or not.
Line 59: 
Line 60:         if resp.error is not None:
Line 61:             raise JsonRpcError(resp.error['code'], 
resp.error['message'])
Line 62: 


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib9dbd70d28968db1305628281015f7b2379c8058
Gerrit-PatchSet: 10
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yeela Kaplan <[email protected]>
Gerrit-Reviewer: Dan Kenigsberg <[email protected]>
Gerrit-Reviewer: Dima Kuznetsov <[email protected]>
Gerrit-Reviewer: Francesco Romani <[email protected]>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Piotr Kliczewski <[email protected]>
Gerrit-Reviewer: Yaniv Bronhaim <[email protected]>
Gerrit-Reviewer: Yeela Kaplan <[email protected]>
Gerrit-Reviewer: [email protected]
Gerrit-HasComments: Yes
_______________________________________________
vdsm-patches mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Reply via email to