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'),
vdsm-patches@lists.fedorahosted.org