Change in vdsm[master]: jsonrpcvdscli: create a client for vdsm with jsonrpc

piotr.kliczewski at gmail.com piotr.kliczewski at gmail.com
Thu Apr 16 08:16:03 UTC 2015


Piotr Kliczewski has posted comments on this change.

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


Patch Set 3:

(2 comments)

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

Line 74: 
Line 75: def connect(cif, client_socket):
Line 76:     stompClient = cif.createStompClient(client_socket)
Line 77:     if stompClient is None:
Line 78:         raise Exception("Failed to create a stomp client")
> I don't see in the code how createStompClient can return None without explo
It is possible that we get None instead of the client so at this stage we need to take care of it.
Line 79: 
Line 80:     client = JsonRpcClient(stompClient)
Line 81:     client.connect()
Line 82:     server = _Server(client)


Line 77:     if stompClient is None:
Line 78:         raise Exception("Failed to create a stomp client")
Line 79: 
Line 80:     client = JsonRpcClient(stompClient)
Line 81:     client.connect()
> In the other hand, the connect part seems like something that should be cov
There is lazy initialization on the connection on first sent message. I think that we should get rid of connect method which does noting in current implementation.
Line 82:     server = _Server(client)
Line 83: 


-- 
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: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yeela Kaplan <ykaplan at redhat.com>
Gerrit-Reviewer: Dima Kuznetsov <dkuznets at redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski at gmail.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei at redhat.com>
Gerrit-Reviewer: Yeela Kaplan <ykaplan at redhat.com>
Gerrit-Reviewer: automation at ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list