Saggi Mizrahi has posted comments on this change. Change subject: jsonrpc: Stomp support ......................................................................
Patch Set 21: (6 comments) http://gerrit.ovirt.org/#/c/26750/21/lib/yajsonrpc/__init__.py File lib/yajsonrpc/__init__.py: Line 509: if obj is None: Line 510: break Line 511: Line 512: client, msg = obj Line 513: self.log.debug("Parsing message") > we don't want those print in release (and don't tell me its only debug lvel I want them at least for when we start using it. It will be very helpful at the start. Line 514: self._parseMessage(client, msg) Line 515: Line 516: def _parseMessage(self, client, msg): Line 517: ctx = _JsonRpcServeRequestContext(client) http://gerrit.ovirt.org/#/c/26750/21/lib/yajsonrpc/stomp.py File lib/yajsonrpc/stomp.py: Line 1: import logging > add copyrights Oops Line 2: import socket Line 3: import cStringIO Line 4: from threading import Timer, Event Line 5: from uuid import uuid4 Line 244: """ Line 245: if sock is None: Line 246: sock = self._sock = socket.socket() Line 247: else: Line 248: sock = sock > shouldn't be - self._sock = sock ? I think I'll just change the other one as no one should use the sock directly anyway. Line 249: Line 250: self._map = {} Line 251: # Because we don't know how large the frames are Line 252: # we have to use non bolocking IO http://gerrit.ovirt.org/#/c/26750/21/lib/yajsonrpc/stompReactor.py File lib/yajsonrpc/stompReactor.py: Line 308: Line 309: Line 310: class StompReactor(object): Line 311: def __init__(self, sslctx=None): Line 312: self.sslctx = sslctx > also private or property ? could be property, SSL was moved to protocol detector anyway. Line 313: self._map = {} Line 314: self._isRunning = False Line 315: self._wakeupEvent = _AsyncoreEvent(self._map) Line 316: http://gerrit.ovirt.org/#/c/26750/21/tests/jsonRpcTests.py File tests/jsonRpcTests.py: Line 151: class _DummyBridge(object): Line 152: log = logging.getLogger("tests.DummyBridge") Line 153: Line 154: def echo(self, text): Line 155: self.log.info("ECHO: '%s'", text) > we don't load any logger.conf on tests. how do you see those logs? They are captured by nose and displayed with -vvv. Very helpful. Line 156: return text Line 157: Line 158: def ping(self): Line 159: return None http://gerrit.ovirt.org/#/c/26750/21/vdsm/clientIF.py File vdsm/clientIF.py: Line 167: port = config.getint('addresses', 'json_port') Line 168: truststore_path = None Line 169: if config.getboolean('vars', 'ssl'): Line 170: truststore_path = config.get('vars', 'trust_store_path') Line 171: # TODO: update config.py > update config py ... , and change port to json_port This is changed to something completely different by protocol detection anyway. Line 172: conf = [('tcp', {"ip": ip, "port": port}), Line 173: ('amqp', {"ip": ip, "port": 5672}), Line 174: ('stomp', {"ip": ip, "port": 61613})] Line 175: self.bindings['json'] = BindingJsonRpc(DynamicBridge(), conf, -- To view, visit http://gerrit.ovirt.org/26750 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I22bcae1e150dea7bc7d9fecefb6847c48bfe8949 Gerrit-PatchSet: 21 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Saggi Mizrahi <smizr...@redhat.com> Gerrit-Reviewer: Barak Azulay <bazu...@redhat.com> Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczew...@gmail.com> Gerrit-Reviewer: Saggi Mizrahi <smizr...@redhat.com> Gerrit-Reviewer: Yaniv Bronhaim <ybron...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
