Piotr Kliczewski has posted comments on this change. Change subject: jsonrpcvdscli: create a client for vdsm with jsonrpc ......................................................................
Patch Set 10: (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) > What do you think is the optimal default timeout? It is not about the timeout. I am thinking that it would be better to update client call method. There is wait without timeout so we can add it there with default value as you have here and remove it from here. 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] > How is it different than timeout not checked on call of jsonRpcClient? What I am thinking about is that responses will be not populated when the timeout occurs. 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
