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