Yeela Kaplan has posted comments on this change. Change subject: jsonrpcvdscli: create a client for vdsm with jsonrpc ......................................................................
Patch Set 3: (5 comments) https://gerrit.ovirt.org/#/c/39203/3/lib/vdsm/jsonrpcvdscli.py File lib/vdsm/jsonrpcvdscli.py: Line 1: # > Any reason not to merge this into vdscli.py? They are very different, and jsonrpcvdscli will eventually replace vdscli when xmlrpc won't be supported. the main difference is the need to translate each command to jsonrpc representation... so I think it's better to keep this semantics separate. It also makes the code much more readable... Line 2: # Copyright 2015 Red Hat, Inc. Line 3: # Line 4: # This program is free software; you can redistribute it and/or modify Line 5: # it under the terms of the GNU General Public License as published by Line 36: 'migrationCreate': 'VM.migrationCreate', Line 37: } Line 38: Line 39: Line 40: class _Server: > until we are python 3 only, please inherit from object Done Line 41: Line 42: def __init__(self, client): Line 43: self._client = client Line 44: 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() > There is lazy initialization on the connection on first sent message. I thi Do you mean we should change it here? I think that if connect does nothing (as can be seen in stompconnection) then we should consider removing it altogether, cause this is just a sideaffect... What do you guys think? Line 82: server = _Server(client) Line 83: Line 80: client = JsonRpcClient(stompClient) Line 81: client.connect() Line 82: server = _Server(client) Line 83: Line 84: return server > just return _Server(client) Done https://gerrit.ovirt.org/#/c/39203/3/lib/yajsonrpc/__init__.py File lib/yajsonrpc/__init__.py: Line 23: from vdsm.utils import traceback Line 24: Line 25: __all__ = ["betterAsyncore", "stompReactor", "stomp"] Line 26: Line 27: CALL_TIMEOUT = 15 > vaguely related, but I don't care that much (has to be introduced somewhere You are right.. I just need it here because of the jsonrpcvdscli use... Line 28: Line 29: _STATE_INCOMING = 1 Line 30: _STATE_OUTGOING = 2 Line 31: _STATE_ONESHOT = 4 -- 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 <ykap...@redhat.com> Gerrit-Reviewer: Dima Kuznetsov <dkuzn...@redhat.com> Gerrit-Reviewer: Francesco Romani <from...@redhat.com> Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczew...@gmail.com> Gerrit-Reviewer: Yaniv Bronhaim <ybron...@redhat.com> Gerrit-Reviewer: Yeela Kaplan <ykap...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
