Saggi Mizrahi has uploaded a new change for review.
Change subject: Refactor communication infra ......................................................................
Refactor communication infra
This is one phase in many, there is still a lot to do but it's a good step in the right direction.
As I said, this is far from complete but it's enough code for a commit IMO.
- Properly layer code to allow multiple transports - Isolate json-rpc logic to the actual json-rpc layer - Someone more conferment but still incomplete json-rpc 2.0 implementation - Make sure tcp handling uses one thread - Write some more tests
Change-Id: I7a65364824cc8433263ccf6ed44f1f97141eff2c Signed-off-by: Saggi Mizrahi smizrahi@redhat.com --- M tests/Makefile.am M tests/apiTests.py A tests/jsonRpcTests.py M vdsm/clientIF.py M vdsm/utils.py M vdsm_api/BindingJsonRpc.py M vdsm_api/Bridge.py A vdsm_api/jsonrpc/__init__.py A vdsm_api/jsonrpc/tcpReactor.py 9 files changed, 617 insertions(+), 99 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/86/9986/1
diff --git a/tests/Makefile.am b/tests/Makefile.am index 8ad34e2..c26adc3 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -32,6 +32,7 @@ gluster_cli_tests.py \ guestIFTests.py \ hooksTests.py \ + jsonRpcTests.py \ libvirtvmTests.py \ lsblkTests.py \ main.py \ diff --git a/tests/apiTests.py b/tests/apiTests.py index bf40f9e..d770cf8 100644 --- a/tests/apiTests.py +++ b/tests/apiTests.py @@ -30,7 +30,9 @@ from testrunner import VdsmTestCase as TestCaseBase from vdsm import constants import BindingJsonRpc +import jsonrpc import apiData +
ip = '0.0.0.0' port = 9824 @@ -144,7 +146,9 @@ # Support parallel testing. Try hard to find an open port to use while True: try: - server = BindingJsonRpc.BindingJsonRpc(bridge, ip, port) + server = BindingJsonRpc.BindingJsonRpc(bridge, + [('tcp', {"ip": ip, + "port": port})]) break except socket.error as ex: if ex.errno == errno.EADDRINUSE: @@ -195,16 +199,21 @@ resp = msize + msg return resp
+ def _createRequest(self, method, reqId=None, params=()): + return {'jsonrpc': '2.0', "id": reqId, "method": method, + "params": params} + def sendMessage(self, msg): with closing(socket.socket(socket.AF_INET, socket.SOCK_STREAM)) as sock: + sock.settimeout(3) try: sock.connect((ip, port)) except socket.error, e: raise ConnectionError("Unable to connect to server: %s", e) try: sock.sendall(msg) - except socket.error, e: + except (socket.error, socket.timeout), e: raise ProtocolError("Unable to send request: %s", e) try: data = sock.recv(JsonRawTest._Size.size) @@ -224,7 +233,9 @@ def testPing(self): self.clearAPI() self.programAPI("testPing") - msg = self.buildMessage({'id': 1, 'method': 'Host.ping', + msg = self.buildMessage({'jsonrpc': '2.0', + 'id': 1, + 'method': 'Host.ping', 'params': {}}) reply = self.sendMessage(msg) self.assertFalse('error' in reply) @@ -233,21 +244,26 @@ def testPingError(self): self.clearAPI() self.programAPI("testPingError") - msg = self.buildMessage({'id': 1, 'method': 'Host.ping', + msg = self.buildMessage({'jsonrpc': '2.0', + 'id': 1, 'method': 'Host.ping', 'params': {}}) reply = self.sendMessage(msg) self.assertEquals(1, reply['error']['code']) self.assertFalse('result' in reply)
def testNoMethod(self): - msg = self.buildMessage({'id': 1, 'method': 'Host.fake'}) + msg = self.buildMessage({'jsonrpc': '2.0', + 'id': 1, + 'method': 'Host.fake'}) reply = self.sendMessage(msg) - self.assertEquals(4, reply['error']['code']) + self.assertEquals(jsonrpc.JsonRpcMethodNotFoundError().code, + reply['error']['code'])
def testBadMethod(self): - msg = self.buildMessage({'id': 1, 'method': 'malformed''}) + msg = self.buildMessage(self._createRequest('malformed'', 1)) reply = self.sendMessage(msg) - self.assertEquals(4, reply['error']['code']) + self.assertEquals(jsonrpc.JsonRpcMethodNotFoundError().code, + reply['error']['code'])
def testMissingSize(self): self.assertRaises(ProtocolError, self.sendMessage, @@ -257,7 +273,9 @@ msg = "malformed message" msize = JsonRawTest._Size.pack(len(msg)) msg = msize + msg - self.assertRaises(ProtocolError, self.sendMessage, msg) + reply = self.sendMessage(msg) + self.assertEquals(jsonrpc.JsonRpcParseError().code, + reply['error']['code'])
def testSynchronization(self): def doPing(msg): @@ -266,7 +284,9 @@ ret = self.sendMessage(msg) self.assertFalse('error' in ret)
- msg = self.buildMessage({'id': 1, 'method': 'Host.ping'}) + msg = self.buildMessage({'jsonrpc': '2.0', + 'id': 1, + 'method': 'Host.ping'}) # Send Truncated message self.assertRaises(ProtocolError, doPing, msg[:-1])
@@ -280,6 +300,8 @@ doPing(msg)
def testInternalError(self): - msg = self.buildMessage({'id': 1, 'method': 'Host.ping'}) + msg = self.buildMessage({'jsonrpc': '2.0', + 'id': 1, 'method': 'Host.ping'}) reply = self.sendMessage(msg) - self.assertEquals(5, reply['error']['code']) + self.assertEquals(jsonrpc.JsonRpcInternalError().code, + reply['error']['code']) diff --git a/tests/jsonRpcTests.py b/tests/jsonRpcTests.py new file mode 100644 index 0000000..85b98a6 --- /dev/null +++ b/tests/jsonRpcTests.py @@ -0,0 +1,158 @@ +# +# Copyright 2012 Red Hat, Inc. +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 2 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA +# +# Refer to the README and COPYING files for full details of the license +# +import threading +import socket +from contextlib import contextmanager +from functools import partial +from contextlib import closing + +from testrunner import VdsmTestCase as TestCaseBase, \ + expandPermutations, \ + permutations + +from jsonrpc import tcpReactor + +PORT_RANGE = xrange(49152, 65535) + + +_distributedPorts = [] + + +def _getFreePort(): + sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM) + with closing(sock): + for port in PORT_RANGE: + if port in _distributedPorts: + continue + + try: + sock.bind(("0.0.0.0", port)) + except: + continue + + _distributedPorts.append(port) + return port + else: + raise Exception("Could not find a free port") + + +@contextmanager +def _tcpServerConstructor(messageHandler): + port = _getFreePort() + address = ("localhost", port) + reactor = tcpReactor.TCPReactor(address, messageHandler) + + try: + yield reactor, partial(TCPRactorClient, address) + finally: + reactor.stop() + + +REACTOR_CONSTRUCTORS = {"tcp": _tcpServerConstructor} +REACTOR_TYPE_PERMUTATIONS = [[r] for r in REACTOR_CONSTRUCTORS.iterkeys()] + + +@contextmanager +def constructReactor(tp, messageHandler): + with REACTOR_CONSTRUCTORS[tp](messageHandler) as res: + yield res + + +class _EchoMessageHandler(object): + def handleMessage(self, msgCtx): + msgCtx.sendReply(msgCtx.data) + + +class TCPRactorClient(object): + def __init__(self, address): + self.sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM) + self.address = address + + def connect(self): + self.sock.connect(self.address) + + def sendMessage(self, msg, timeout=None): + msg = tcpReactor._Size.pack(len(msg)) + msg + self.sock.settimeout(timeout) + self.sock.send(msg) + + def recvMessage(self, timeout=None): + self.sock.settimeout(timeout) + rawSize = self.sock.recv(tcpReactor._Size.size) + size = tcpReactor._Size.unpack(rawSize)[0] + return self.sock.recv(size) + + def close(self): + self.sock.close() + + +@expandPermutations +class ReactorTests(TestCaseBase): + @permutations(REACTOR_TYPE_PERMUTATIONS) + def test(self, reactorType): + data = """I've seen things you people wouldn't believe. Attack ships on + fire off the shoulder of Orion. I've watched c-beams glitter + in the dark near the Tannhauser Gate. All those ... moments + will be lost in time, like tears...in rain. Time to die.""" + # (C) Blade Runner + + msgHandler = _EchoMessageHandler() + + def serve(reactor): + try: + reactor.serve_forever() + except socket.error as e: + pass + except Exception as e: + self.log.error("Server died unexpectedly", exc_info=True) + self.fail("Server died: (%s) %s" % (type(e), e)) + + with constructReactor(reactorType, msgHandler) as (reactor, + clientFactory): + t = threading.Thread(target=serve, args=(reactor,)) + t.setDaemon(True) + t.start() + + clientNum = 1 + repeats = 1 + subRepeats = 1 + + clients = [] + try: + for i in range(clientNum): + client = clientFactory() + client.connect() + clients.append(client) + + for i in range(repeats): + for client in clients: + for i in range(subRepeats): + self.log.info("Sending message...") + client.sendMessage(data, 1) + + for i in range(repeats * subRepeats): + for client in clients: + self.log.info("Waiting for reply...") + retData = client.recvMessage(1) + self.log.info("Asserting reply...") + self.assertEquals(retData, data) + finally: + for client in clients: + client.close() diff --git a/vdsm/clientIF.py b/vdsm/clientIF.py index 65b4e72..48a9cf8 100644 --- a/vdsm/clientIF.py +++ b/vdsm/clientIF.py @@ -156,8 +156,8 @@ schema = os.path.join(constants.P_VDSM, 'vdsmapi-schema.json') ip = self._getServerIP(config.get('addresses', 'management_ip')) port = config.getint('addresses', 'json_port') - self.bindings['json'] = BindingJsonRpc(DynamicBridge(schema), - ip, port) + conf = [('tcp', {"ip": ip, "port": port})] + self.bindings['json'] = BindingJsonRpc(DynamicBridge(schema), conf)
def _prepareBindings(self): self.bindings = {} diff --git a/vdsm/utils.py b/vdsm/utils.py index b4614e7..bb012d0 100644 --- a/vdsm/utils.py +++ b/vdsm/utils.py @@ -905,6 +905,38 @@ return unicode(self.cmd)
+class PollEvent(object): + def __init__(self): + self._r, self._w = os.pipe() + self._lock = threading.Lock() + self._isSet = False + + def fileno(self): + return self._r + + def set(self): + with self._lock: + if self._isSet: + return + + os.write(self._w, "a") + self._isSet = True + + def isSet(self): + return self._isSet + + def clear(self): + with self._lock: + if not self._isSet: + return + + os.read(self._r, 1) + + def __del__(self): + os.close(self._r) + os.close(self._w) + + def retry(func, expectedException=Exception, tries=None, timeout=None, sleep=1): """ diff --git a/vdsm_api/BindingJsonRpc.py b/vdsm_api/BindingJsonRpc.py index 1ddc863..b8c8658 100644 --- a/vdsm_api/BindingJsonRpc.py +++ b/vdsm_api/BindingJsonRpc.py @@ -14,86 +14,42 @@ # License along with this program; if not, write to the Free Software # Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA import threading -import SocketServer -import json import logging - import struct -
_Size = struct.Struct("!Q")
+from jsonrpc import JsonRpcServer +from jsonrpc.tcpReactor import TCPReactor
-class BindingJsonRpc: + +class BindingJsonRpc(object): log = logging.getLogger('BindingJsonRpc')
- def __init__(self, bridge, ip, port): + def __init__(self, bridge, backendConfig): + reactors = [] self.bridge = bridge - self.serverPort = port - self.serverIP = ip - self._createServer() + self.server = JsonRpcServer(bridge) + for backendType, cfg in backendConfig: + if backendType == "tcp": + reactors.append(self._createTcpReactor(cfg))
- def _createServer(self): - ip = self.serverIP or '0.0.0.0' - self.server = JsonRpcServer((ip, self.serverPort), JsonRpcTCPHandler, - self.bridge) + self._reactors = reactors + + def _createTcpReactor(self, cfg): + address = cfg.get("ip", "0.0.0.0") + port = cfg.get("port", 4044) + return TCPReactor((address, port), self.server)
def start(self): - t = threading.Thread(target=self.server.serve_forever, - name='JsonRpc') - t.setDaemon(True) - t.start() + for reactor in self._reactors: + reactorName = reactor.__class__.__name__ + t = threading.Thread(target=reactor.serve_forever, + name='JsonRpc (%s)' % reactorName) + t.setDaemon(True) + t.start()
def prepareForShutdown(self): self.server.shutdown() - - -class JsonRpcServer(SocketServer.TCPServer): - def __init__(self, addrInfo, handler, bridge): - self.bridge = bridge - self.allow_reuse_address = True - SocketServer.TCPServer.__init__(self, addrInfo, handler) - - -class JsonRpcTCPHandler(SocketServer.StreamRequestHandler): - log = logging.getLogger('JsonRpcTCPHandler') - - def handle(self): - while True: - # self.request is the TCP socket connected to the client - try: - data = self.request.recv(_Size.size) - if len(data) != _Size.size: - self.log.debug("Connection closed") - return - msgLen = _Size.unpack(data)[0] - msg = json.loads(self.request.recv(msgLen)) - except: - self.log.warn("Unexpected exception", exc_info=True) - return - self.log.debug('Received request: %s', msg) - - try: - ret = self.server.bridge.dispatch(msg['method'], - msg.get('params', {})) - resp = self.buildResponse(msg['id'], ret) - except Exception, e: - self.log.warn("Dispatch error", exc_info=True) - err = {'error': {'code': 5, - 'message': 'Dispatch error: %s' % e}} - resp = self.buildResponse(msg['id'], err) - - self.wfile.write(resp) - self.wfile.flush() - - def buildResponse(self, msgId, result): - msgData = {'id': msgId} - if result['error']['code'] != 0: - msgData['error'] = result['error'] - else: - msgData['result'] = result['result'] - msg = json.dumps(msgData) - msg = msg.encode('utf-8') - self.log.debug('Sending reply: %s', msg) - msize = _Size.pack(len(msg)) - return msize + msg + for reactor in self._reactors: + reactor.stop() diff --git a/vdsm_api/Bridge.py b/vdsm_api/Bridge.py index d93dc22..08970ba 100644 --- a/vdsm_api/Bridge.py +++ b/vdsm_api/Bridge.py @@ -14,10 +14,12 @@ # Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
from functools import partial + import vdsmapi import logging import types import API +import jsonrpc
class VdsmError(Exception): @@ -26,22 +28,26 @@ self.message = message
-class MethodBridge(object): +class DynamicBridge(object): + def __init__(self, schema): + self._parseSchema(schema) + def dispatch(self, name, argobj): methodName = name.replace('.', '_') result = None - error = {'code': 0, 'message': 'Success'} try: fn = getattr(self, methodName) except AttributeError: - error = {'code': 4, - 'message': "Operation '%s' not supported" % name} - return {'result': result, 'error': error} + raise jsonrpc.JsonRpcMethodNotFoundError() + try: result = fn(argobj) except VdsmError, e: - error = {'code': e.code, 'message': e.message} - return {'result': result, 'error': error} + # TBD: Do we really want to always log here + self.log.debug("Operation failed, returning error", exc_info=True) + raise jsonrpc.JsonRpcError(e.code, e.message) + + return result
def _getArgs(self, argobj, arglist): return tuple(argobj[arg] for arg in arglist if arg in argobj) @@ -53,11 +59,6 @@ return response[member] except KeyError: raise VdsmError(5, "Response is missing '%s' member" % member) - - -class DynamicBridge(MethodBridge): - def __init__(self, schema): - self._parseSchema(schema)
def _parseSchema(self, schema): self.commands = {} @@ -82,7 +83,7 @@ className, methodName = attr.split('_') return partial(self._dynamicMethod, className, methodName) else: - raise AttributeError() + raise AttributeError(attr)
def _getArgList(self, cmd): return self.commands[cmd].get('data', {}).keys() @@ -148,15 +149,30 @@ self._typeFixup('return', retType, result) return result
- def _dynamicMethod(self, className, methodName, argobj): + def _nameArgs(self, args, kwargs, arglist): + kwargs = kwargs.copy() + for i, arg in enumerate(args): + argName = arglist[i] + kwargs[argName] = arg + + return kwargs + + def _dynamicMethod(self, className, methodName, *args, **kwargs): cmd = '%s_%s' % (className, methodName) - ctorArgObj = argobj.get('__obj__', {}) + + # __obj__ is deprecated, support here is only for BC until the patches + # that remove it get pushed in. + ctorArgObj = kwargs.pop('__obj__', {}) ctorargs = self._getArgs(ctorArgObj, self._getObjargList(className)) + # This might look a bit wasteful because we unpack the argobj in the + # next line. I put this just so I need to do minimal changes to the + # rest of the code but still support an arglist + argobj = self._nameArgs(args, kwargs, self._getArgList(cmd)) args = self._getArgs(argobj, self._getArgList(cmd)) apiobj = self._getApiClass(className) api = apiobj(*ctorargs)
- self._fixupArgs(cmd, args) + self._fixupArgs(cmd, argobj)
# Call the override function (if given). Otherwise, just call directly fn = command_info.get(cmd, {}).get('call') @@ -169,7 +185,7 @@ if result['status']['code']: code = result['status']['code'] msg = result['status']['message'] - raise VdsmError(code, msg) + raise jsonrpc.JsonRpcError(code, msg)
retfield = command_info.get(cmd, {}).get('ret') if isinstance(retfield, types.FunctionType): diff --git a/vdsm_api/jsonrpc/__init__.py b/vdsm_api/jsonrpc/__init__.py new file mode 100644 index 0000000..43e4199 --- /dev/null +++ b/vdsm_api/jsonrpc/__init__.py @@ -0,0 +1,176 @@ +# Copyright (C) 2012 Saggi Mizrahi, Red Hat Inc. +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License version 2 as +# published by the Free Software Foundation. +# +# This program is distributed in the hope that it will be useful, but +# WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +# General Public License for more details. +# +# You should have received a copy of the GNU General Public +# License along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA +import json +import logging + +__all__ = ["tcpReactor"] + + +class JsonRpcError(RuntimeError): + def __init__(self, code, msg): + self.code = code + self._msg = msg + + +class JsonRpcParseError(JsonRpcError): + def __init__(self): + JsonRpcError.__init__(self, -32700, + "Invalid JSON was received by the server. " + "An error occurred on the server while parsing " + "the JSON text.") + + +class JsonRpcInvalidRequestError(JsonRpcError): + def __init__(self): + JsonRpcError.__init__(self, -32600, + "The JSON sent is not a valid Request object.") + + +class JsonRpcMethodNotFoundError(JsonRpcError): + def __init__(self): + JsonRpcError.__init__(self, -32601, + "The method does not exist / is not available.") + + +class JsonRpcInvalidParamsError(JsonRpcError): + def __init__(self): + JsonRpcError.__init__(self, -32602, + "Invalid method parameter(s).") + + +class JsonRpcInternalError(JsonRpcError): + def __init__(self): + JsonRpcError.__init__(self, -32603, + "Internal JSON-RPC error.") + + +class JsonRpcRequest(object): + def __init__(self, method, params, reqId): + self.method = method + self.params = params + self.id = reqId + + def invokeFunction(self, func): + if isinstance(self.params, list): + return func(*self.params) + else: + return func(**self.params) + + def isNotification(self): + return self.id is None + + +class JsonRpcServer(object): + log = logging.getLogger("JsonRpcServer") + + def __init__(self, bridge): + self._bridge = bridge + + def _parseMessage(self, msg): + try: + return json.loads(msg, 'utf-8') + except: + raise JsonRpcParseError() + + def _parseRequest(self, obj): + if obj.get("jsonrpc") != "2.0": + raise JsonRpcInvalidRequestError() + + method = obj.get("method") + if method is None: + raise JsonRpcInvalidRequestError() + + reqId = obj.get("id") + if not isinstance(reqId, int): + raise JsonRpcInvalidRequestError() + + params = obj.get('params', []) + if not isinstance(params, (list, dict)): + raise JsonRpcInvalidRequestError() + + return JsonRpcRequest(method, params, reqId) + + def _jsonError2Response(self, err, req): + respId = None + if req is not None: + respId = req.id + + return json.dumps({"jsonrpc": "2.0", + "error": {"code": err.code, "message": err.message}, + "id": respId}) + + def _generateResponse(self, req, result): + return json.dumps({"jsonrpc": "2.0", + "result": result, + "id": req.id}, 'utf-8') + + def _processRequest(self, srv, conn, req, method): + # TODO: Multithreading + resp = None + try: + res = req.invokeFunction(method) + resp = self._generateResponse(req, res) + + except JsonRpcError as e: + if not req.isNotification(): + resp = self._jsonError2Response(e, req) + + self.log.error("Error in request", exc_info=True) + except: + if not req.isNotification(): + resp = self._jsonError2Response(JsonRpcInternalError(), req) + + self.log.error("Unexpected error in request", exc_info=True) + + if req.isNotification(): + return + + srv.sendMessage(conn, self._jsonError2Response(e, resp)) + + def handleMessage(self, msgCtx): + #TODO: support batch requests + req = None + resp = None + try: + obj = self._parseMessage(msgCtx.data) + req = self._parseRequest(obj) + + mangledMethod = req.method.replace(".", "_") + self.log.debug("Looking for method '%s' in bridge", + mangledMethod) + try: + method = getattr(self._bridge, mangledMethod) + except AttributeError: + raise JsonRpcMethodNotFoundError() + else: + res = req.invokeFunction(method) + resp = self._generateResponse(req, res) + + except JsonRpcError as e: + resp = self._jsonError2Response(e, req) + self.log.error("Error processing request", exc_info=True) + except: + resp = self._jsonError2Response(JsonRpcInternalError(), req) + self.log.error("Unexpected error while processing request", + exc_info=True) + + if resp is None: + return + + # Notification don't respond even on errors + if req is not None and req.isNotification(): + return + + msgCtx.sendReply(resp) diff --git a/vdsm_api/jsonrpc/tcpReactor.py b/vdsm_api/jsonrpc/tcpReactor.py new file mode 100644 index 0000000..a815033 --- /dev/null +++ b/vdsm_api/jsonrpc/tcpReactor.py @@ -0,0 +1,157 @@ +# Copyright (C) 2012 Saggi Mizrahi, Red Hat Inc. +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License version 2 as +# published by the Free Software Foundation. +# +# This program is distributed in the hope that it will be useful, but +# WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +# General Public License for more details. +# +# You should have received a copy of the GNU General Public +# License along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA +import errno +import struct +import logging +import socket +from select import poll, POLLIN, POLLPRI, POLLERR, POLLHUP, POLLOUT + +from vdsm import utils + +_Size = struct.Struct("!Q") + + +class TCPConnection(object): + def __init__(self, conn, buffsize=(2 ** 20)): + self._conn = conn + conn.setblocking(False) + self.buffsize = buffsize + self._buffer = "" + self._sendData = [] + + def fileno(self): + return self._conn.fileno() + + def addSendData(self, data): + self._sendData.append(data) + + def hasSendData(self): + return len(self._sendData) > 0 + + def processOutput(self): + try: + if (len(self._sendData) > 0): + data = self._sendData[0] + bsent = self._conn.send(data) + if bsent < len(data): + self._sendData[0] = data[:bsent] + else: + self._sendData.pop(0) + except (OSError, IOError) as e: + if e.errno not in (errno.EINTR, errno.EAGAIN): + raise + + def processInput(self): + try: + self._buffer += self._conn.recv(self.buffsize - len(self._buffer)) + except (OSError, IOError) as e: + if e.errno not in (errno.EINTR, errno.EAGAIN): + raise + + buffLen = len(self._buffer) + if buffLen < _Size.size: + return + + if buffLen > self.buffsize: + self._conn.close() + self.sendData = [] + + msgLen = _Size.unpack(self._buffer[:_Size.size])[0] + if (buffLen - _Size.size) < msgLen: + return None + + msgStart = _Size.size + msgEnd = msgLen + _Size.size + res = self._buffer[msgStart:msgEnd] + self._buffer = self._buffer[msgEnd:] + return res + + +class TCPMessageContext(object): + + def __init__(self, server, conn, data): + self._server = server + self._conn = conn + self._data = data + + @property + def data(self): + return self._data + + def sendReply(self, data): + self._server.sendReply(self, data) + + +class TCPReactor(object): + log = logging.getLogger("jsonrpc.TCPReactor") + + def __init__(self, address, messageHandler): + self._sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM) + self._addr = address + self._messageHandler = messageHandler + self._inputEvent = utils.PollEvent() + # TODO: Close on exec + self._sock.bind(self._addr) + + def serve_forever(self): + sock = self._sock + sock.listen(1) + poller = poll() + poller.register(sock, POLLIN | POLLPRI) + poller.register(self._inputEvent, POLLIN | POLLPRI) + connections = {} + self.log.debug("Starting to accept clients") + while True: + for fd, conn in connections.iteritems(): + if conn.hasSendData(): + poller.modify(fd, (POLLIN | POLLPRI | POLLOUT)) + else: + poller.modify(fd, (POLLIN | POLLPRI)) + + for fd, ev in poller.poll(): + if ev & (POLLERR | POLLHUP): + if fd == sock.fileno(): + self.log.info("Listening socket closed, shutting down") + return + + self.log.debug("Connection closed") + poller.unregister(fd) + del connections[fd] + + elif fd == self._inputEvent.fileno(): + self._inputEvent.clear() + + elif fd == sock.fileno(): + conn, addr = sock.accept() + self.log.debug("Processing new connection") + connections[conn.fileno()] = TCPConnection(conn) + poller.register(conn, (POLLIN | POLLPRI)) + else: + conn = connections[fd] + if ev & (POLLIN | POLLPRI): + msg = conn.processInput() + if msg is not None: + ctx = TCPMessageContext(self, conn, msg) + self._messageHandler.handleMessage(ctx) + if ev & POLLOUT: + conn.processOutput() + + def sendReply(self, ctx, message): + conn = ctx._conn + conn.addSendData(_Size.pack(len(message)) + message) + self._inputEvent.set() + + def stop(self): + self._sock.close()
-- To view, visit http://gerrit.ovirt.org/9986 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange Gerrit-Change-Id: I7a65364824cc8433263ccf6ed44f1f97141eff2c Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Saggi Mizrahi smizrahi@redhat.com
oVirt Jenkins CI Server has posted comments on this change.
Change subject: Refactor communication infra ......................................................................
Patch Set 1:
Build Started http://jenkins.ovirt.org/job/vdsm_unit_tests_manual_gerrit/328/ (2/2)
-- To view, visit http://gerrit.ovirt.org/9986 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I7a65364824cc8433263ccf6ed44f1f97141eff2c Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: Refactor communication infra ......................................................................
Patch Set 1:
Build Started http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/294/ (1/2)
-- To view, visit http://gerrit.ovirt.org/9986 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I7a65364824cc8433263ccf6ed44f1f97141eff2c Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: Refactor communication infra ......................................................................
Patch Set 1: Fails
Build Failed
http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/294/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_unit_tests_manual_gerrit/328/ : FAILURE
-- To view, visit http://gerrit.ovirt.org/9986 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I7a65364824cc8433263ccf6ed44f1f97141eff2c Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: Refactor communication infra ......................................................................
Patch Set 2:
Build Started http://jenkins.ovirt.org/job/vdsm_unit_tests_manual_gerrit/330/ (2/2)
-- To view, visit http://gerrit.ovirt.org/9986 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I7a65364824cc8433263ccf6ed44f1f97141eff2c Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: Refactor communication infra ......................................................................
Patch Set 2:
Build Started http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/296/ (1/2)
-- To view, visit http://gerrit.ovirt.org/9986 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I7a65364824cc8433263ccf6ed44f1f97141eff2c Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: Refactor communication infra ......................................................................
Patch Set 2: Fails
Build Failed
http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/296/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_unit_tests_manual_gerrit/330/ : FAILURE
-- To view, visit http://gerrit.ovirt.org/9986 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I7a65364824cc8433263ccf6ed44f1f97141eff2c Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: Refactor communication infra ......................................................................
Patch Set 3:
Build Started http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/298/ (1/2)
-- To view, visit http://gerrit.ovirt.org/9986 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I7a65364824cc8433263ccf6ed44f1f97141eff2c Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Barak Azulay bazulay@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybronhei@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: Refactor communication infra ......................................................................
Patch Set 3:
Build Started http://jenkins.ovirt.org/job/vdsm_unit_tests_manual_gerrit/332/ (2/2)
-- To view, visit http://gerrit.ovirt.org/9986 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I7a65364824cc8433263ccf6ed44f1f97141eff2c Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Barak Azulay bazulay@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybronhei@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: Refactor communication infra ......................................................................
Patch Set 3: Fails
Build Failed
http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/298/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_unit_tests_manual_gerrit/332/ : FAILURE
-- To view, visit http://gerrit.ovirt.org/9986 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I7a65364824cc8433263ccf6ed44f1f97141eff2c Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Barak Azulay bazulay@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybronhei@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: Refactor communication infra ......................................................................
Patch Set 4:
Build Started http://jenkins.ovirt.org/job/vdsm_unit_tests_manual_gerrit/352/ (1/2)
-- To view, visit http://gerrit.ovirt.org/9986 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I7a65364824cc8433263ccf6ed44f1f97141eff2c Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Barak Azulay bazulay@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybronhei@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: Refactor communication infra ......................................................................
Patch Set 4:
Build Started http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/318/ (2/2)
-- To view, visit http://gerrit.ovirt.org/9986 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I7a65364824cc8433263ccf6ed44f1f97141eff2c Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Barak Azulay bazulay@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybronhei@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: Refactor communication infra ......................................................................
Patch Set 4: Fails
Build Failed
http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/318/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_unit_tests_manual_gerrit/352/ : FAILURE
-- To view, visit http://gerrit.ovirt.org/9986 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I7a65364824cc8433263ccf6ed44f1f97141eff2c Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Barak Azulay bazulay@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybronhei@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: Refactor communication infra ......................................................................
Patch Set 5:
Build Started http://jenkins.ovirt.org/job/vdsm_unit_tests_manual_gerrit/354/ (1/2)
-- To view, visit http://gerrit.ovirt.org/9986 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I7a65364824cc8433263ccf6ed44f1f97141eff2c Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Barak Azulay bazulay@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybronhei@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: Refactor communication infra ......................................................................
Patch Set 5:
Build Started http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/320/ (2/2)
-- To view, visit http://gerrit.ovirt.org/9986 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I7a65364824cc8433263ccf6ed44f1f97141eff2c Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Barak Azulay bazulay@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybronhei@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: Refactor communication infra ......................................................................
Patch Set 5: Fails
Build Failed
http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/320/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_unit_tests_manual_gerrit/354/ : FAILURE
-- To view, visit http://gerrit.ovirt.org/9986 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I7a65364824cc8433263ccf6ed44f1f97141eff2c Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Barak Azulay bazulay@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybronhei@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: Refactor communication infra ......................................................................
Patch Set 6:
Build Started http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/322/ (1/2)
-- To view, visit http://gerrit.ovirt.org/9986 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I7a65364824cc8433263ccf6ed44f1f97141eff2c Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Barak Azulay bazulay@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybronhei@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: Refactor communication infra ......................................................................
Patch Set 6:
Build Started http://jenkins.ovirt.org/job/vdsm_unit_tests_manual_gerrit/356/ (2/2)
-- To view, visit http://gerrit.ovirt.org/9986 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I7a65364824cc8433263ccf6ed44f1f97141eff2c Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Barak Azulay bazulay@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybronhei@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: Refactor communication infra ......................................................................
Patch Set 6: Fails
Build Failed
http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/322/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_unit_tests_manual_gerrit/356/ : FAILURE
-- To view, visit http://gerrit.ovirt.org/9986 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I7a65364824cc8433263ccf6ed44f1f97141eff2c Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Barak Azulay bazulay@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybronhei@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: Refactor communication infra ......................................................................
Patch Set 7:
Build Started http://jenkins.ovirt.org/job/vdsm_unit_tests_manual_gerrit/363/ (2/2)
-- To view, visit http://gerrit.ovirt.org/9986 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I7a65364824cc8433263ccf6ed44f1f97141eff2c Gerrit-PatchSet: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Barak Azulay bazulay@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybronhei@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: Refactor communication infra ......................................................................
Patch Set 7:
Build Started http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/329/ (1/2)
-- To view, visit http://gerrit.ovirt.org/9986 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I7a65364824cc8433263ccf6ed44f1f97141eff2c Gerrit-PatchSet: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Barak Azulay bazulay@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybronhei@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: Refactor communication infra ......................................................................
Patch Set 7: Fails
Build Failed
http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/329/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_unit_tests_manual_gerrit/363/ : FAILURE
-- To view, visit http://gerrit.ovirt.org/9986 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I7a65364824cc8433263ccf6ed44f1f97141eff2c Gerrit-PatchSet: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Barak Azulay bazulay@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybronhei@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: Refactor communication infra ......................................................................
Patch Set 8:
Build Started http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/331/ (2/2)
-- To view, visit http://gerrit.ovirt.org/9986 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I7a65364824cc8433263ccf6ed44f1f97141eff2c Gerrit-PatchSet: 8 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Barak Azulay bazulay@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybronhei@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: Refactor communication infra ......................................................................
Patch Set 8:
Build Started http://jenkins.ovirt.org/job/vdsm_unit_tests_manual_gerrit/365/ (1/2)
-- To view, visit http://gerrit.ovirt.org/9986 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I7a65364824cc8433263ccf6ed44f1f97141eff2c Gerrit-PatchSet: 8 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Barak Azulay bazulay@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybronhei@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: Refactor communication infra ......................................................................
Patch Set 8: Fails
Build Failed
http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/331/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_unit_tests_manual_gerrit/365/ : FAILURE
-- To view, visit http://gerrit.ovirt.org/9986 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I7a65364824cc8433263ccf6ed44f1f97141eff2c Gerrit-PatchSet: 8 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Barak Azulay bazulay@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybronhei@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: Refactor communication infra ......................................................................
Patch Set 9:
Build Started http://jenkins.ovirt.org/job/vdsm_unit_tests_manual_gerrit/373/ (1/2)
-- To view, visit http://gerrit.ovirt.org/9986 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I7a65364824cc8433263ccf6ed44f1f97141eff2c Gerrit-PatchSet: 9 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Barak Azulay bazulay@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybronhei@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: Refactor communication infra ......................................................................
Patch Set 9:
Build Started http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/339/ (2/2)
-- To view, visit http://gerrit.ovirt.org/9986 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I7a65364824cc8433263ccf6ed44f1f97141eff2c Gerrit-PatchSet: 9 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Barak Azulay bazulay@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybronhei@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: Refactor communication infra ......................................................................
Patch Set 9:
Build Successful
http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/339/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_unit_tests_manual_gerrit/373/ : SUCCESS
-- To view, visit http://gerrit.ovirt.org/9986 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I7a65364824cc8433263ccf6ed44f1f97141eff2c Gerrit-PatchSet: 9 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Barak Azulay bazulay@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybronhei@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: Refactor communication infra ......................................................................
Patch Set 10:
Build Started http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/341/ (1/2)
-- To view, visit http://gerrit.ovirt.org/9986 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I7a65364824cc8433263ccf6ed44f1f97141eff2c Gerrit-PatchSet: 10 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Barak Azulay bazulay@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybronhei@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: Refactor communication infra ......................................................................
Patch Set 10:
Build Started http://jenkins.ovirt.org/job/vdsm_unit_tests_manual_gerrit/375/ (2/2)
-- To view, visit http://gerrit.ovirt.org/9986 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I7a65364824cc8433263ccf6ed44f1f97141eff2c Gerrit-PatchSet: 10 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Barak Azulay bazulay@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybronhei@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: Refactor communication infra ......................................................................
Patch Set 10:
Build Successful
http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/341/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_unit_tests_manual_gerrit/375/ : SUCCESS
-- To view, visit http://gerrit.ovirt.org/9986 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I7a65364824cc8433263ccf6ed44f1f97141eff2c Gerrit-PatchSet: 10 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Barak Azulay bazulay@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybronhei@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server
Adam Litke has posted comments on this change.
Change subject: Refactor communication infra ......................................................................
Patch Set 10: (4 inline comments)
A few minor (but important comments). I am okay with merging this after they are resolved.
.................................................... File tests/jsonRpcTests.py Line 100: def handleMessage(self, msgCtx): Line 101: msgCtx.sendReply(msgCtx.data) Line 102: Line 103: Line 104: class TCPRactorClient(object): TCPReactorClient Line 105: def __init__(self, address): Line 106: self.sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM) Line 107: self.address = address Line 108:
Line 162: fire off the shoulder of Orion. I've watched c-beams glitter Line 163: in the dark near the Tannhauser Gate. All those ... moments Line 164: will be lost in time, like tears...in rain. Time to die.""" Line 165: # (C) Blade Runner Line 166: Please do not put copyrighted text in the source code. Use lorem ipsum instead. Line 167: msgHandler = _EchoMessageHandler() Line 168: Line 169: def serve(reactor): Line 170: try:
Line 223: Peasant 3: Well, she turned me into a newt! Line 224: Sir Bedevere: A newt? Line 225: Peasant 3: [meekly after a long pause] ... I got better. Line 226: Crowd: [shouts] Burn her anyway!""" Line 227: # (C) Monty Python and the Holdy Grail Again... Line 228: Line 229: bridge = _DummyBridge() Line 230: with constructServer(reactorType, bridge) as (server, clientFactory): Line 231: client = clientFactory()
Line 254: Dennis: Oh, king eh? Very nice. And how'd you get that, eh? Line 255: By exploiting the workers. By hanging on to outdated Line 256: imperialist dogma which perpetuates the economic and Line 257: social differences in our society.""" Line 258: # (C) Monty Python and the Holdy Grail ... Line 259: Line 260: bridge = _DummyBridge() Line 261: with constructServer(reactorType, bridge) as (server, clientFactory): Line 262: client = clientFactory()
-- To view, visit http://gerrit.ovirt.org/9986 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I7a65364824cc8433263ccf6ed44f1f97141eff2c Gerrit-PatchSet: 10 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Barak Azulay bazulay@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybronhei@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: Refactor communication infra ......................................................................
Patch Set 11:
Build Started http://jenkins.ovirt.org/job/vdsm_unit_tests_manual_gerrit/389/ (2/2)
-- To view, visit http://gerrit.ovirt.org/9986 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I7a65364824cc8433263ccf6ed44f1f97141eff2c Gerrit-PatchSet: 11 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Barak Azulay bazulay@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybronhei@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: Refactor communication infra ......................................................................
Patch Set 11:
Build Started http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/355/ (1/2)
-- To view, visit http://gerrit.ovirt.org/9986 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I7a65364824cc8433263ccf6ed44f1f97141eff2c Gerrit-PatchSet: 11 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Barak Azulay bazulay@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybronhei@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: Refactor communication infra ......................................................................
Patch Set 11:
Build Successful
http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/355/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_unit_tests_manual_gerrit/389/ : SUCCESS
-- To view, visit http://gerrit.ovirt.org/9986 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I7a65364824cc8433263ccf6ed44f1f97141eff2c Gerrit-PatchSet: 11 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Barak Azulay bazulay@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybronhei@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server
Adam Litke has posted comments on this change.
Change subject: Refactor communication infra ......................................................................
Patch Set 11: I would prefer that you didn't submit this
Lorem Ipsum.
-- To view, visit http://gerrit.ovirt.org/9986 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I7a65364824cc8433263ccf6ed44f1f97141eff2c Gerrit-PatchSet: 11 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Barak Azulay bazulay@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybronhei@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: Refactor communication infra ......................................................................
Patch Set 12:
Build Started http://jenkins.ovirt.org/job/vdsm_unit_tests_manual_gerrit/452/ (1/2)
-- To view, visit http://gerrit.ovirt.org/9986 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I7a65364824cc8433263ccf6ed44f1f97141eff2c Gerrit-PatchSet: 12 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Barak Azulay bazulay@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybronhei@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: Refactor communication infra ......................................................................
Patch Set 12:
Build Started http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/417/ (2/2)
-- To view, visit http://gerrit.ovirt.org/9986 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I7a65364824cc8433263ccf6ed44f1f97141eff2c Gerrit-PatchSet: 12 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Barak Azulay bazulay@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybronhei@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: Refactor communication infra ......................................................................
Patch Set 12:
Build Successful
http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/417/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_unit_tests_manual_gerrit/452/ : SUCCESS
-- To view, visit http://gerrit.ovirt.org/9986 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I7a65364824cc8433263ccf6ed44f1f97141eff2c Gerrit-PatchSet: 12 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Barak Azulay bazulay@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybronhei@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server
Adam Litke has posted comments on this change.
Change subject: Refactor communication infra ......................................................................
Patch Set 12: Verified; Looks good to me, but someone else must approve
It's not perfect but it's good enough to merge so that we can continue to make progress. Verified that the tests run successfully here.
-- To view, visit http://gerrit.ovirt.org/9986 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I7a65364824cc8433263ccf6ed44f1f97141eff2c Gerrit-PatchSet: 12 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Barak Azulay bazulay@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybronhei@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server
Vinzenz Feenstra has posted comments on this change.
Change subject: Refactor communication infra ......................................................................
Patch Set 12: Looks good to me, but someone else must approve
-- To view, visit http://gerrit.ovirt.org/9986 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I7a65364824cc8433263ccf6ed44f1f97141eff2c Gerrit-PatchSet: 12 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Barak Azulay bazulay@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Vinzenz Feenstra vfeenstr@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybronhei@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: Refactor communication infra ......................................................................
Patch Set 13:
Build Started http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/460/ (1/2)
-- To view, visit http://gerrit.ovirt.org/9986 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I7a65364824cc8433263ccf6ed44f1f97141eff2c Gerrit-PatchSet: 13 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Barak Azulay bazulay@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Vinzenz Feenstra vfeenstr@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybronhei@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: Refactor communication infra ......................................................................
Patch Set 13:
Build Started http://jenkins.ovirt.org/job/vdsm_unit_tests_manual_gerrit/495/ (2/2)
-- To view, visit http://gerrit.ovirt.org/9986 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I7a65364824cc8433263ccf6ed44f1f97141eff2c Gerrit-PatchSet: 13 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Barak Azulay bazulay@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Vinzenz Feenstra vfeenstr@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybronhei@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: Refactor communication infra ......................................................................
Patch Set 13:
Build Successful
http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/460/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_unit_tests_manual_gerrit/495/ : SUCCESS
-- To view, visit http://gerrit.ovirt.org/9986 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I7a65364824cc8433263ccf6ed44f1f97141eff2c Gerrit-PatchSet: 13 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Barak Azulay bazulay@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Vinzenz Feenstra vfeenstr@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybronhei@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server
Vinzenz Feenstra has posted comments on this change.
Change subject: Refactor communication infra ......................................................................
Patch Set 13: Looks good to me, but someone else must approve
-- To view, visit http://gerrit.ovirt.org/9986 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I7a65364824cc8433263ccf6ed44f1f97141eff2c Gerrit-PatchSet: 13 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Barak Azulay bazulay@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Vinzenz Feenstra vfeenstr@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybronhei@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server
Dan Kenigsberg has posted comments on this change.
Change subject: Refactor communication infra ......................................................................
Patch Set 13: I would prefer that you didn't submit this
(6 inline comments)
.................................................... File vdsm_api/BindingJsonRpc.py Line 37: self._reactors = reactors Line 38: Line 39: def _createTcpReactor(self, cfg): Line 40: address = cfg.get("ip", "0.0.0.0") Line 41: port = cfg.get("port", 4044) do we really want this 4044 default in this level? Line 42: return TCPReactor((address, port), self.server) Line 43: Line 44: def start(self): Line 45: for reactor in self._reactors:
.................................................... File vdsm_api/jsonrpc/__init__.py Line 136: res = req.invokeFunction(method) Line 137: resp = self._generateResponse(req, res) Line 138: Line 139: except JsonRpcError as e: Line 140: resp = self._jsonError2Response(e, req) pep8 does not mind this over-indentation? Line 141: self.log.error("Error processing request", exc_info=True) Line 142: except: Line 143: resp = self._jsonError2Response(JsonRpcInternalError(), req) Line 144: self.log.error("Unexpected error while processing request",
.................................................... File vdsm_api/jsonrpc/tcpReactor.py Line 61: raise Line 62: Line 63: buffLen = len(self._buffer) Line 64: if buffLen < _Size.size: Line 65: return it would be nicer to use the same form as you do below:
return None Line 66: Line 67: if buffLen > self.buffsize: Line 68: self._conn.close() Line 69: self.sendData = []
Line 63: buffLen = len(self._buffer) Line 64: if buffLen < _Size.size: Line 65: return Line 66: Line 67: if buffLen > self.buffsize: could this happen unless there's a bug in self._conn.recv()?
what is this non-private self.sendData ? Line 68: self._conn.close() Line 69: self.sendData = [] Line 70: Line 71: msgLen = _Size.unpack(self._buffer[:_Size.size])[0]
.................................................... File vdsm/utils.py Line 922: with self._lock: Line 923: if self._isSet: Line 924: return Line 925: Line 926: os.write(self._w, "a") should not you handle EINTR just to fend off future improbable bugs? Line 927: self._isSet = True Line 928: Line 929: def isSet(self): Line 930: return self._isSet
Line 934: if not self._isSet: Line 935: return Line 936: Line 937: os.read(self._r, 1) Line 938: don't you need to reset self._isSet sometime? Line 939: def __del__(self): Line 940: os.close(self._r) Line 941: os.close(self._w) Line 942:
-- To view, visit http://gerrit.ovirt.org/9986 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I7a65364824cc8433263ccf6ed44f1f97141eff2c Gerrit-PatchSet: 13 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Barak Azulay bazulay@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Vinzenz Feenstra vfeenstr@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybronhei@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: Refactor communication infra ......................................................................
Patch Set 14:
Build Started http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/539/ (1/2)
-- To view, visit http://gerrit.ovirt.org/9986 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I7a65364824cc8433263ccf6ed44f1f97141eff2c Gerrit-PatchSet: 14 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Barak Azulay bazulay@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Vinzenz Feenstra vfeenstr@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybronhei@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: Refactor communication infra ......................................................................
Patch Set 14:
Build Started http://jenkins.ovirt.org/job/vdsm_unit_tests_manual_gerrit/574/ (2/2)
-- To view, visit http://gerrit.ovirt.org/9986 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I7a65364824cc8433263ccf6ed44f1f97141eff2c Gerrit-PatchSet: 14 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Barak Azulay bazulay@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Vinzenz Feenstra vfeenstr@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybronhei@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: Refactor communication infra ......................................................................
Patch Set 14:
Build Successful
http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/539/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_unit_tests_manual_gerrit/574/ : SUCCESS
-- To view, visit http://gerrit.ovirt.org/9986 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I7a65364824cc8433263ccf6ed44f1f97141eff2c Gerrit-PatchSet: 14 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Barak Azulay bazulay@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Vinzenz Feenstra vfeenstr@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybronhei@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: Refactor communication infra ......................................................................
Patch Set 15:
Build Started http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/554/ (1/2)
-- To view, visit http://gerrit.ovirt.org/9986 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I7a65364824cc8433263ccf6ed44f1f97141eff2c Gerrit-PatchSet: 15 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Barak Azulay bazulay@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Vinzenz Feenstra vfeenstr@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybronhei@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: Refactor communication infra ......................................................................
Patch Set 15:
Build Started http://jenkins.ovirt.org/job/vdsm_unit_tests_manual_gerrit/589/ (2/2)
-- To view, visit http://gerrit.ovirt.org/9986 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I7a65364824cc8433263ccf6ed44f1f97141eff2c Gerrit-PatchSet: 15 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Barak Azulay bazulay@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Vinzenz Feenstra vfeenstr@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybronhei@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: Refactor communication infra ......................................................................
Patch Set 15:
Build Successful
http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/554/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_unit_tests_manual_gerrit/589/ : SUCCESS
-- To view, visit http://gerrit.ovirt.org/9986 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I7a65364824cc8433263ccf6ed44f1f97141eff2c Gerrit-PatchSet: 15 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Barak Azulay bazulay@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Vinzenz Feenstra vfeenstr@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybronhei@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server
Vinzenz Feenstra has posted comments on this change.
Change subject: Refactor communication infra ......................................................................
Patch Set 15: Looks good to me, but someone else must approve
-- To view, visit http://gerrit.ovirt.org/9986 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I7a65364824cc8433263ccf6ed44f1f97141eff2c Gerrit-PatchSet: 15 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Barak Azulay bazulay@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Vinzenz Feenstra vfeenstr@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybronhei@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server
Dan Kenigsberg has posted comments on this change.
Change subject: Refactor communication infra ......................................................................
Patch Set 15: I would prefer that you didn't submit this
(2 inline comments)
marking -1 for two unanswered questions. I know they are not the most urgent issues ever, but I'd like to have them addressed (even with "my code is great, the reviewer is an idiot").
.................................................... File vdsm_api/BindingJsonRpc.py Line 37: self._reactors = reactors Line 38: Line 39: def _createTcpReactor(self, cfg): Line 40: address = cfg.get("ip", "0.0.0.0") Line 41: port = cfg.get("port", 4044) do we really want this 4044 default in this level? Line 42: return TCPReactor((address, port), self.server) Line 43: Line 44: def start(self): Line 45: for reactor in self._reactors:
.................................................... File vdsm_api/jsonrpc/tcpReactor.py Line 63: buffLen = len(self._buffer) Line 64: if buffLen < _Size.size: Line 65: return None Line 66: Line 67: if buffLen > self.buffsize: could this happen unless there's a bug in self._conn.recv()? I feel as if this condition could be dropped. Line 68: self._conn.close() Line 69: self._sendData = [] Line 70: Line 71: msgLen = _Size.unpack(self._buffer[:_Size.size])[0]
-- To view, visit http://gerrit.ovirt.org/9986 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I7a65364824cc8433263ccf6ed44f1f97141eff2c Gerrit-PatchSet: 15 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Barak Azulay bazulay@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Vinzenz Feenstra vfeenstr@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybronhei@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server
Saggi Mizrahi has posted comments on this change.
Change subject: Refactor communication infra ......................................................................
Patch Set 15: (2 inline comments)
.................................................... File vdsm_api/BindingJsonRpc.py Line 37: self._reactors = reactors Line 38: Line 39: def _createTcpReactor(self, cfg): Line 40: address = cfg.get("ip", "0.0.0.0") Line 41: port = cfg.get("port", 4044) I remember already replying on this comment. Anyway, it doesn't matter as VDSM always sends the port number from it's own defaults. If I didn't put a default here I would have had to put error handling code here and in every other place in the stack and that would have just been annoying. Line 42: return TCPReactor((address, port), self.server) Line 43: Line 44: def start(self): Line 45: for reactor in self._reactors:
.................................................... File vdsm_api/jsonrpc/tcpReactor.py Line 63: buffLen = len(self._buffer) Line 64: if buffLen < _Size.size: Line 65: return None Line 66: Line 67: if buffLen > self.buffsize: It is a remnant of days gone by. I'll remove it in another patch, please don't make me rebase everything and loos all the +1s Line 68: self._conn.close() Line 69: self._sendData = [] Line 70: Line 71: msgLen = _Size.unpack(self._buffer[:_Size.size])[0]
-- To view, visit http://gerrit.ovirt.org/9986 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I7a65364824cc8433263ccf6ed44f1f97141eff2c Gerrit-PatchSet: 15 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Barak Azulay bazulay@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Vinzenz Feenstra vfeenstr@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybronhei@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server
Dan Kenigsberg has posted comments on this change.
Change subject: Refactor communication infra ......................................................................
Patch Set 15: (2 inline comments)
.................................................... File vdsm_api/BindingJsonRpc.py Line 37: self._reactors = reactors Line 38: Line 39: def _createTcpReactor(self, cfg): Line 40: address = cfg.get("ip", "0.0.0.0") Line 41: port = cfg.get("port", 4044) sorry, I do not follow. which error handling code would have been required? to handle the case of not having cfg['ip'] ? having a default here only hide such an error instead of crying out loud about it. Line 42: return TCPReactor((address, port), self.server) Line 43: Line 44: def start(self): Line 45: for reactor in self._reactors:
.................................................... File vdsm_api/jsonrpc/tcpReactor.py Line 63: buffLen = len(self._buffer) Line 64: if buffLen < _Size.size: Line 65: return None Line 66: Line 67: if buffLen > self.buffsize: I would not forget Vinzenz's +1s. do you have other people acks? I know that I haven't looked beyond this first patch. Line 68: self._conn.close() Line 69: self._sendData = [] Line 70: Line 71: msgLen = _Size.unpack(self._buffer[:_Size.size])[0]
-- To view, visit http://gerrit.ovirt.org/9986 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I7a65364824cc8433263ccf6ed44f1f97141eff2c Gerrit-PatchSet: 15 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Barak Azulay bazulay@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Vinzenz Feenstra vfeenstr@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybronhei@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server
Adam Litke has posted comments on this change.
Change subject: Refactor communication infra ......................................................................
Patch Set 15: I would prefer that you didn't submit this
(1 inline comment)
I may have found a bug. Please check out the comment.
.................................................... File vdsm_api/jsonrpc/tcpReactor.py Line 45: if (len(self._sendData) > 0): Line 46: data = self._sendData[0] Line 47: bsent = self._conn.send(data) Line 48: if bsent < len(data): Line 49: self._sendData[0] = data[:bsent] I might very well be wrong, but shouldn't this be:
self._sendData[0] = data[bsent:]
The way you have it written, it looks like you will just keep the part of the data you already sent. Line 50: else: Line 51: self._sendData.pop(0) Line 52: except (OSError, IOError) as e: Line 53: if e.errno not in (errno.EINTR, errno.EAGAIN):
-- To view, visit http://gerrit.ovirt.org/9986 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I7a65364824cc8433263ccf6ed44f1f97141eff2c Gerrit-PatchSet: 15 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Barak Azulay bazulay@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Vinzenz Feenstra vfeenstr@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybronhei@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: Refactor communication infra ......................................................................
Patch Set 17:
Build Started http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/702/ (1/2)
-- To view, visit http://gerrit.ovirt.org/9986 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I7a65364824cc8433263ccf6ed44f1f97141eff2c Gerrit-PatchSet: 17 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Barak Azulay bazulay@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Vinzenz Feenstra vfeenstr@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybronhei@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: Refactor communication infra ......................................................................
Patch Set 17:
Build Started http://jenkins.ovirt.org/job/vdsm_unit_tests_manual_gerrit/737/ (2/2)
-- To view, visit http://gerrit.ovirt.org/9986 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I7a65364824cc8433263ccf6ed44f1f97141eff2c Gerrit-PatchSet: 17 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Barak Azulay bazulay@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Vinzenz Feenstra vfeenstr@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybronhei@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: Refactor communication infra ......................................................................
Patch Set 17:
Build Successful
http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/702/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_unit_tests_manual_gerrit/737/ : SUCCESS
-- To view, visit http://gerrit.ovirt.org/9986 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I7a65364824cc8433263ccf6ed44f1f97141eff2c Gerrit-PatchSet: 17 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Barak Azulay bazulay@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Vinzenz Feenstra vfeenstr@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybronhei@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server
Adam Litke has posted comments on this change.
Change subject: Refactor communication infra ......................................................................
Patch Set 17: Looks good to me, but someone else must approve
I am sure this patch is not yet perfect. That being said, I think we should take it because this is low risk to existing users and will make it easier for us to collaborate on the new api from this point forward.
-- To view, visit http://gerrit.ovirt.org/9986 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I7a65364824cc8433263ccf6ed44f1f97141eff2c Gerrit-PatchSet: 17 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Barak Azulay bazulay@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Vinzenz Feenstra vfeenstr@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybronhei@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server
On Mon, Jan 07, 2013 at 03:10:38PM -0500, danken@redhat.com wrote:
Dan Kenigsberg has posted comments on this change.
Change subject: Refactor communication infra ......................................................................
Patch Set 15: (2 inline comments)
.................................................... File vdsm_api/BindingJsonRpc.py Line 37: self._reactors = reactors Line 38: Line 39: def _createTcpReactor(self, cfg): Line 40: address = cfg.get("ip", "0.0.0.0") Line 41: port = cfg.get("port", 4044) sorry, I do not follow. which error handling code would have been required? to handle the case of not having cfg['ip'] ? having a default here only hide such an error instead of crying out loud about it.
Saggi, Adam, could you help me here?
I do not understand the need for the default port here. Could you elaborate on the justification?
On Tue, Jan 15, 2013 at 03:40:17PM +0200, Dan Kenigsberg wrote:
On Mon, Jan 07, 2013 at 03:10:38PM -0500, danken@redhat.com wrote:
Dan Kenigsberg has posted comments on this change.
Change subject: Refactor communication infra ......................................................................
Patch Set 15: (2 inline comments)
.................................................... File vdsm_api/BindingJsonRpc.py Line 37: self._reactors = reactors Line 38: Line 39: def _createTcpReactor(self, cfg): Line 40: address = cfg.get("ip", "0.0.0.0") Line 41: port = cfg.get("port", 4044) sorry, I do not follow. which error handling code would have been required? to handle the case of not having cfg['ip'] ? having a default here only hide such an error instead of crying out loud about it.
Saggi, Adam, could you help me here?
I do not understand the need for the default port here. Could you elaborate on the justification?
I'll give Saggi a chance to defend it. On the face of it I'd agree with you Dan that the initialization code in clientIF should be structured similarly to the BindingXMLRPC code. Configuration for all ports (regardless of how the underlying code is structured) should probably be in a flat namespace and together in the vdsm.conf file.
As I said in my comment on the patch.
This is just so I don't have to change it to.
try: port = cfg["port"] except KeyEerror: raise ValueError("cfg")
And then, have code handling the error upwards in the stack. It is just there to get things passed with minimal code.
Further more, if it was an http server instead of a json-rpc server no one would have minded if I had put 80 as the default port even lower down the stack meaning that having the default set at any level is not such a taboo.
In any case, since for Dan it is more important to change it now and have everyone blocked the patch for another iteration is more important then pushing it in and arguing about it later I'll send another version today.
----- Original Message -----
From: "Dan Kenigsberg" danken@redhat.com To: "Saggi Mizrahi" smizrahi@redhat.com, "Adam Litke" agl@us.ibm.com Cc: "Vinzenz Feenstra" vfeenstr@redhat.com, vdsm-patches@fedorahosted.org Sent: Tuesday, January 15, 2013 8:40:17 AM Subject: Re: Refactor communication infra
On Mon, Jan 07, 2013 at 03:10:38PM -0500, danken@redhat.com wrote:
Dan Kenigsberg has posted comments on this change.
Change subject: Refactor communication infra ......................................................................
Patch Set 15: (2 inline comments)
.................................................... File vdsm_api/BindingJsonRpc.py Line 37: self._reactors = reactors Line 38: Line 39: def _createTcpReactor(self, cfg): Line 40: address = cfg.get("ip", "0.0.0.0") Line 41: port = cfg.get("port", 4044) sorry, I do not follow. which error handling code would have been required? to handle the case of not having cfg['ip'] ? having a default here only hide such an error instead of crying out loud about it.
Saggi, Adam, could you help me here?
I do not understand the need for the default port here. Could you elaborate on the justification?
oVirt Jenkins CI Server has posted comments on this change.
Change subject: Refactor communication infra ......................................................................
Patch Set 18:
Build Started http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/735/ (1/2)
-- To view, visit http://gerrit.ovirt.org/9986 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I7a65364824cc8433263ccf6ed44f1f97141eff2c Gerrit-PatchSet: 18 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Barak Azulay bazulay@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Vinzenz Feenstra vfeenstr@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybronhei@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server
Dan Kenigsberg has posted comments on this change.
Change subject: Refactor communication infra ......................................................................
Patch Set 18: Looks good to me, approved
-- To view, visit http://gerrit.ovirt.org/9986 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I7a65364824cc8433263ccf6ed44f1f97141eff2c Gerrit-PatchSet: 18 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Barak Azulay bazulay@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Vinzenz Feenstra vfeenstr@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybronhei@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: Refactor communication infra ......................................................................
Patch Set 18:
Build Started http://jenkins.ovirt.org/job/vdsm_unit_tests_manual_gerrit/770/ (2/2)
-- To view, visit http://gerrit.ovirt.org/9986 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I7a65364824cc8433263ccf6ed44f1f97141eff2c Gerrit-PatchSet: 18 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Barak Azulay bazulay@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Vinzenz Feenstra vfeenstr@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybronhei@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: Refactor communication infra ......................................................................
Patch Set 18:
Build Successful
http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/735/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_unit_tests_manual_gerrit/770/ : SUCCESS
-- To view, visit http://gerrit.ovirt.org/9986 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I7a65364824cc8433263ccf6ed44f1f97141eff2c Gerrit-PatchSet: 18 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Barak Azulay bazulay@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Vinzenz Feenstra vfeenstr@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybronhei@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server
Dan Kenigsberg has posted comments on this change.
Change subject: Refactor communication infra ......................................................................
Patch Set 18: Verified
-- To view, visit http://gerrit.ovirt.org/9986 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I7a65364824cc8433263ccf6ed44f1f97141eff2c Gerrit-PatchSet: 18 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Barak Azulay bazulay@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Vinzenz Feenstra vfeenstr@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybronhei@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server
Dan Kenigsberg has submitted this change and it was merged.
Change subject: Refactor communication infra ......................................................................
Refactor communication infra
This is one phase in many, there is still a lot to do but it's a good step in the right direction.
As I said, this is far from complete but it's enough code for a commit IMO.
- Properly layer code to allow multiple transports - Isolate json-rpc logic to the actual json-rpc layer - Someone more conferment but still incomplete json-rpc 2.0 implementation - Make sure tcp handling uses one thread - Write some more tests
Change-Id: I7a65364824cc8433263ccf6ed44f1f97141eff2c Signed-off-by: Saggi Mizrahi smizrahi@redhat.com --- M configure.ac M tests/Makefile.am M tests/apiTests.py A tests/jsonRpcTests.py M tests/testrunner.py M vdsm.spec.in M vdsm/clientIF.py M vdsm/utils.py M vdsm_api/BindingJsonRpc.py M vdsm_api/Bridge.py M vdsm_api/Makefile.am A vdsm_api/jsonrpc/Makefile.am A vdsm_api/jsonrpc/__init__.py A vdsm_api/jsonrpc/tcpReactor.py 14 files changed, 786 insertions(+), 106 deletions(-)
Approvals: Dan Kenigsberg: Verified; Looks good to me, approved
-- To view, visit http://gerrit.ovirt.org/9986 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: merged Gerrit-Change-Id: I7a65364824cc8433263ccf6ed44f1f97141eff2c Gerrit-PatchSet: 18 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Barak Azulay bazulay@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Vinzenz Feenstra vfeenstr@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybronhei@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server
vdsm-patches@lists.fedorahosted.org