Change in vdsm[master]: json-rpc: Protocol detection

ybronhei at redhat.com ybronhei at redhat.com
Mon May 12 06:53:24 UTC 2014


Yaniv Bronhaim has posted comments on this change.

Change subject: json-rpc: Protocol detection
......................................................................


Patch Set 10: Code-Review-1

(12 comments)

http://gerrit.ovirt.org/#/c/26300/10/lib/vdsm/utils.py
File lib/vdsm/utils.py:

Line 768:     proc = execCmd(command, cwd=cwd, data=data, sync=False,
Line 769:                    nice=nice, ioclass=ioclass, execCmdLogger=execCmdLogger,
Line 770:                    deathSignal=deathSignal)
Line 771:     if recoveryCallback:
Line 772:         recoveryCallback(proc)
"recovery" sounds like it should run only on failure..
Line 773: 
Line 774:     if not proc.wait(cond=stop):
Line 775:         proc.kill()
Line 776:         raise ActionStopped()


http://gerrit.ovirt.org/#/c/26300/10/lib/yajsonrpc/stompReactor.py
File lib/yajsonrpc/stompReactor.py:

Line 171: 
Line 172: 
Line 173: # FIXME: We should go about making a listener wrapper like the client wrapper
Line 174: #        This is not as high priority as users don't interact with listeners
Line 175: #        as much
don't you do that here? ^
Line 176: class StompListenerImpl(object):
Line 177:     log = logging.getLogger("jsonrpc.StompListener")
Line 178: 
Line 179:     def __init__(self, reactor, address, acceptHandler, connected_socket):


http://gerrit.ovirt.org/#/c/26300/10/tests/jsonRpcUtils.py
File tests/jsonRpcUtils.py:

Line 46: 
Line 47: PERMUTATIONS = tuple(product((True, False), ("xml", "stomp")))
Line 48: 
Line 49: 
Line 50: class FakeCilentIf():
client ..
Line 51:     log = logging.getLogger("FakeCilentIf")
Line 52: 
Line 53:     def __init__(self):
Line 54:         self.threadLocal = threading.local()


Line 48: 
Line 49: 
Line 50: class FakeCilentIf():
Line 51:     log = logging.getLogger("FakeCilentIf")
Line 52: 
same typo
Line 53:     def __init__(self):
Line 54:         self.threadLocal = threading.local()
Line 55:         self.irs = True
Line 56:         self.gluster = None


Line 51:     log = logging.getLogger("FakeCilentIf")
Line 52: 
Line 53:     def __init__(self):
Line 54:         self.threadLocal = threading.local()
Line 55:         self.irs = True
won't you get AttributeError? hard to follow what you do with self.irs.. so im just raising a thought
Line 56:         self.gluster = None
Line 57: 
Line 58:         # API module is redefined for apiTests so we need to add BLANK_UUIDs
Line 59:         import API


Line 111:     if sslctx:
Line 112:         sock = SSL.Connection(sslctx.context)
Line 113:     else:
Line 114:         sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
Line 115:     sock.settimeout(3)
worth constant imo..
Line 116:     sock.connect((host, port))
Line 117:     return sock
Line 118: 
Line 119: 


http://gerrit.ovirt.org/#/c/26300/10/vdsm.spec.in
File vdsm.spec.in:

Line 44
Line 45
Line 46
Line 47
Line 48
why are you removing it ?? we need option to disable the json installation


Line 1377
Line 1378
Line 1379
Line 1380
Line 1381
not more vdsm-api.. worth to mention why in the commit message


Line 120: Requires: m2crypto
Line 121: Requires: libguestfs-tools-c
Line 122: Requires: libnl
Line 123: Requires: %{name}-xmlrpc = %{version}-%{release}
Line 124: Requires: %{name}-jsonrpc = %{version}-%{release}
why both?
Line 125: Requires: mom >= 0.3.2-3
Line 126: Requires: numactl
Line 127: 
Line 128: %ifarch x86_64


http://gerrit.ovirt.org/#/c/26300/10/vdsm/BindingXMLRPC.py
File vdsm/BindingXMLRPC.py:

Line 904:                 (self.setupNetworks, 'setupNetworks'),
Line 905:                 (self.ping, 'ping'),
Line 906:                 (self.setSafeNetworkConfig, 'setSafeNetworkConfig'),
Line 907:                 (self.fenceNode, 'fenceNode'),
Line 908:                 (self.stop, 'prepareForShutdown'),
why to change api in this patch? add stop verb instead, some might using perpareForShutdown
Line 909:                 (self.setLogLevel, 'setLogLevel'),
Line 910:                 (self.setMOMPolicy, 'setMOMPolicy'),
Line 911:                 (self.setMOMPolicyParameters, 'setMOMPolicyParameters'),
Line 912:                 (self.setHaMaintenanceMode, 'setHaMaintenanceMode'),


http://gerrit.ovirt.org/#/c/26300/10/vdsm/protoDetector.py
File vdsm/protoDetector.py:

Line 1: import errno
add copyright
Line 2: import logging
Line 3: import os
Line 4: import select
Line 5: import socket


http://gerrit.ovirt.org/#/c/26300/10/vdsm/sslutils.py
File vdsm/sslutils.py:

Line 1: import socket
add copyright
Line 2: from M2Crypto import SSL, X509
Line 3: 
Line 4: 
Line 5: class SSLSocket(object):


-- 
To view, visit http://gerrit.ovirt.org/26300
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Id739a40e2b37dcc175137ec91cd5ec166ad24a75
Gerrit-PatchSet: 10
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Piotr Kliczewski <piotr.kliczewski at gmail.com>
Gerrit-Reviewer: Barak Azulay <bazulay at redhat.com>
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski at gmail.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi at redhat.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