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

fromani at redhat.com fromani at redhat.com
Tue Jun 3 10:51:56 UTC 2014


Francesco Romani has posted comments on this change.

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


Patch Set 31:

(5 comments)

Initial and partial review, just few minor notes. So far seems really nice code.

http://gerrit.ovirt.org/#/c/26300/31/lib/vdsm/sslUtils.py
File lib/vdsm/sslUtils.py:

Line 77: #             result = self.connection.peek(size)
Line 78: #         else:
Line 79: #             result = self.connection.read(size)
Line 80: #         return result
Line 81: #     recv = read
rebase/debug relic?
Line 82: 
Line 83:     def __getattr__(self, name):
Line 84:         return getattr(self.connection, name)
Line 85: 


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

Line 232: 
Line 233: class ConnectedTCPServer(SocketServer.TCPServer):
Line 234: 
Line 235:     def __init__(self, RequestHandlerClass):
Line 236:         SocketServer.TCPServer.__init__(self, None, RequestHandlerClass, False)
what about super() here?
Line 237:         self.queue = Queue()
Line 238: 
Line 239:     def add(self, connected_socket, socket_address):
Line 240:         self.queue.put((connected_socket, socket_address))


http://gerrit.ovirt.org/#/c/26300/31/tests/bridgeTests.py
File tests/bridgeTests.py:

Line 31: 
Line 32: def createFakeAPI():
Line 33:     _newAPI = imp.new_module('API')
Line 34:     _API = __import__('API', globals(), locals(), {}, -1)
Line 35: #     setattr(_newAPI, 'Host', Host)
rebase/debug relic?
Line 36:     setattr(_newAPI, 'Global', Host)
Line 37: 
Line 38:     # Apply the whitelist to our version of API
Line 39:     for name in apiWhitelist:


http://gerrit.ovirt.org/#/c/26300/31/tests/jsonRpcHelper.py
File tests/jsonRpcHelper.py:

Line 37: 
Line 38: protonReactor = None
Line 39: try:
Line 40:     from yajsonrpc import protonReactor
Line 41:     protonReactor  # unused import
I'd add in the comment the reason why it is unused (AFAIK/IIRC to make pep8/pylint happy)
Line 42: except ImportError:
Line 43:     pass
Line 44: 
Line 45: 


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

Line 971:                 (self.setupNetworks, 'setupNetworks'),
Line 972:                 (self.ping, 'ping'),
Line 973:                 (self.setSafeNetworkConfig, 'setSafeNetworkConfig'),
Line 974:                 (self.fenceNode, 'fenceNode'),
Line 975:                 (self.stop, 'prepareForShutdown'),
Just for my better understanding, what's the reason for this name change?
Line 976:                 (self.setLogLevel, 'setLogLevel'),
Line 977:                 (self.setMOMPolicy, 'setMOMPolicy'),
Line 978:                 (self.setMOMPolicyParameters, 'setMOMPolicyParameters'),
Line 979:                 (self.setHaMaintenanceMode, 'setHaMaintenanceMode'),


-- 
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: 31
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: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: Liron Ar <laravot at redhat.com>
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek at redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski at gmail.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi at redhat.com>
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr 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