Saggi Mizrahi has posted comments on this change.
Change subject: jsonrpc: Create the java bindings and fix bugs
......................................................................
Patch Set 5:
(15 comments)
....................................................
File lib/yajsonrpc/__init__.py
Line 88
Line 89
Line 90
Line 91
Line 92
The spec says that ID can be anything
Line 315: try:
Line 316: mobj = json.loads(message)
Line 317: isResponse = self._isResponse(mobj)
Line 318: except:
Line 319: self.log.warning("Problem parsing message from
client")
It might be huge and it might contain sensitive data (passwords)
Since we never manually construct json objects it only happens if the libraries are
broken.
Line 320: transport.close()
Line 321: del self._clients[transport]
Line 322: continue
Line 323:
Line 333: if v is None:
Line 334: v = res
Line 335:
Line 336: if v != res:
Line 337: raise TypeError("batch is mixed")
Might contain sensitive data
Line 338:
Line 339: return v
Line 340: else:
Line 341: return ("result" in obj or "error" in obj)
Line 340: else:
Line 341: return ("result" in obj or "error" in obj)
Line 342:
Line 343: def close(self):
Line 344: self._inbox.put(None)
We need to properly close channels.
None is a good a flag as any IMO.
Line 345:
Line 346:
Line 347: class JsonRpcCall(object):
Line 348: def __init__(self):
Line 449: self._workQueue = Queue()
Line 450: self._threadFactory = threadFactory
Line 451:
Line 452: def queueRequest(self, req):
Line 453: print "DSAD"
oops :)
Line 454: self._workQueue.put_nowait(req)
Line 455: print "DSAD"
Line 456:
Line 457: def _serveRequest(self, ctx, req):
....................................................
File lib/yajsonrpc/betterAsyncore.py
Line 105: class AsyncChat(object):
Line 106: # these are overridable defaults
Line 107:
Line 108: ac_in_buffer_size = 4096
Line 109: ac_out_buffer_size = 4096
They are. They are just class related constants and not global constants.
Line 110:
Line 111: def __init__(self, impl):
Line 112: self._fifoLock = Lock()
Line 113: self._impl = impl
Line 154:
Line 155: try:
Line 156: data = dispatcher.recv(self.ac_in_buffer_size)
Line 157: except socket.error:
Line 158: dispatcher.handle_error()
No, the API mandates that the I am not responsible for error reporting.
Line 159: return
Line 160:
Line 161: self.ac_in_buffer = self.ac_in_buffer + data
Line 162:
Line 318:
Line 319: try:
Line 320: impl.init(self)
Line 321: except AttributeError:
Line 322: pass
No, it just means that it's optional
Line 323:
Line 324: def __invoke(self, name, *args, **kwargs):
Line 325: if hasattr(self.__impl, name):
Line 326: return getattr(self.__impl, name)(self, *args, **kwargs)
Line 373:
Line 374: def connect(self, addr):
Line 375: self.connected = False
Line 376: self.connecting = True
Line 377: socket = self.socket
It's faster and pyflakes can catch errors this way.
Line 378: socket.setblocking(1)
Line 379: socket.connect(addr)
Line 380: socket.setblocking(0)
Line 381: self.addr = addr
....................................................
File lib/yajsonrpc/protonReactor.py
Line 62: self._reactor = reactor
Line 63: self._connected = False
Line 64:
Line 65: def setTimeout(self, timeout):
Line 66: # TODO
I don't think it's needed. It will just make the log dirty.
Line 67: pass
Line 68:
Line 69: def closed(self):
Line 70: return (self.connector is None or
....................................................
File tests/jsonRpcTests.py
Line 18: # Refer to the README and COPYING files for full details of the license
Line 19: #
Line 20: import threading
Line 21: import socket
Line 22: import logging
53
Line 23: from Queue import Queue
Line 24: from contextlib import contextmanager
Line 25: from testValidation import brokentest
Line 26:
....................................................
File tests/jsonRpcUtils.py
Line 23: pass
Line 24:
Line 25:
Line 26: def hasProton():
Line 27: return protonReactor is not None
I don't understand the question
Line 28:
Line 29:
Line 30: def getFreePort():
Line 31: sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
Line 57: ("127.0.0.1", port))
Line 58:
Line 59:
Line 60: REACTOR_CONSTRUCTORS = {"tcp": _tcpServerConstructor,
Line 61: "amqp": _protonServerConstructor}
Because proton is the implementation not the protocol
Line 62: REACTOR_TYPE_PERMUTATIONS = [[r] for r in REACTOR_CONSTRUCTORS.iterkeys()]
Line 63: SSL_OPTIONS = (True, False)
Line 64: CONNECTION_PERMUTATIONS = tuple(product(REACTOR_CONSTRUCTORS.iterkeys(),
Line 65: SSL_OPTIONS))
Line 59:
Line 60: REACTOR_CONSTRUCTORS = {"tcp": _tcpServerConstructor,
Line 61: "amqp": _protonServerConstructor}
Line 62: REACTOR_TYPE_PERMUTATIONS = [[r] for r in REACTOR_CONSTRUCTORS.iterkeys()]
Line 63: SSL_OPTIONS = (True, False)
True means ssl=True
False means ssl=False
Line 64: CONNECTION_PERMUTATIONS = tuple(product(REACTOR_CONSTRUCTORS.iterkeys(),
Line 65: SSL_OPTIONS))
Line 66:
Line 67: CERT_DIR = os.path.abspath(os.path.dirname(__file__))
....................................................
File vdsm_api/vdsmapi.py
Line 116: Find the API schema file whether we are running from within the source dir
Line 117: or from an installed location
Line 118: """
Line 119: # Don't depend on module VDSM if not looking for schema
Line 120: from vdsm import constants
I don't think it matters
Line 121:
Line 122: localpath = os.path.dirname(__file__)
Line 123: installedpath = constants.P_VDSM
Line 124: for directory in localpath, installedpath:
--
To view, visit
http://gerrit.ovirt.org/19497
To unsubscribe, visit
http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: If828355b7efe28fe6a2e784069425fefd2f3f25c
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: Barak Azulay <bazulay(a)redhat.com>
Gerrit-Reviewer: Eduardo <ewarszaw(a)redhat.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com>
Gerrit-Reviewer: mooli tayer <mtayer(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes