Yaniv Bronhaim has posted comments on this change.
Change subject: jsonrpc: Stomp support ......................................................................
Patch Set 6:
(9 comments)
http://gerrit.ovirt.org/#/c/26750/6//COMMIT_MSG Commit Message:
Line 6: Line 7: jsonrpc: Stomp support Line 8: Line 9: The mini broker turned up to be much more work than I expected. Line 10: This is an intermediate solution. VDSM will accept broker like "this is an intermediate solution" is enough, no need for introduction Line 11: message but will ignore most of them and not enforce policy. Line 12: Line 13: This is so that we can get the engine talking in the correct way and Line 14: work on the mini broker stress free. This also means that we can release
http://gerrit.ovirt.org/#/c/26750/6/lib/yajsonrpc/stomp.py File lib/yajsonrpc/stomp.py:
Line 74: s = s.replace(escaped, raw) Line 75: Line 76: # TODO : Throw error on invalid escape char Line 77: # spec: Undefined escape sequences such as \t (octet 92 and 116) MUST be Line 78: # treated as a fatal protocol error. don't you want to treat that ^ ? Line 79: return s Line 80: Line 81: Line 82: def encodeValue(s):
Line 150: self._contentLength = int(headers.get('content-length', -1)) Line 151: self._state = self._STATE_BODY Line 152: return True Line 153: Line 154: # TODO: Proper error on missing or to many ':' do that ^ Line 155: key, value = header.split(":") Line 156: key = decodeValue(key) Line 157: value = decodeValue(value) Line 158:
Line 212: def parse(self, data): Line 213: states = self._states Line 214: self._buffer.write(data) Line 215: while states[self._state](): Line 216: pass why holding states? just call the functions so it will be clear what step you are performing. holding functions pointers for states for what reason ? if one step fails at least report it Line 217: Line 218: def popFrame(self): Line 219: try: Line 220: return self._frames.popleft()
Line 415: Line 416: Line 417: class AsyncClient(object): Line 418: log = logging.getLogger("yajsonrpc.protocols.stomp.AsyncClient") Line 419: i'd like to have proper logger for yajsonrpc. it'll might be helpful for debugging the protocol Line 420: def __init__(self, frameHandler, hostname): Line 421: self._hostname = hostname Line 422: self._frameHandler = frameHandler Line 423: self._connected = False
http://gerrit.ovirt.org/#/c/26750/6/lib/yajsonrpc/stompReactor.py File lib/yajsonrpc/stompReactor.py:
Line 48: protocol=ctxdef.protocol) Line 49: Line 50: Line 51: class StompAdapterImpl(object): Line 52: log = logging.getLogger("Broker.StompAdapter") use the yajsonrpc handler Line 53: Line 54: def __init__(self, reactor, messageHandler): Line 55: self._reactor = reactor Line 56: self._messageHandler = messageHandler
Line 83: self.log.debug("Passing incoming message") Line 84: self._messageHandler(self, frame.body) Line 85: Line 86: def handle_frame(self, dispatcher, frame): Line 87: self.log.debug("Handling message %s", frame) do you intent to leave those debugging messages? Line 88: try: Line 89: self._commands[frame.command](dispatcher, frame) Line 90: except KeyError: Line 91: self.log.warn("Unknown command %s", frame)
http://gerrit.ovirt.org/#/c/26750/6/tests/jsonRpcTests.py File tests/jsonRpcTests.py:
Line 148: data[:plen], data[-plen:], len(data))) Line 149: Line 150: Line 151: class _DummyBridge(object): Line 152: log = logging.getLogger("tests.DummyBridge") we don't initialize logging during nose tests, just use stdout and when you run the tests use -s to see outputs Line 153: Line 154: def echo(self, text): Line 155: self.log.info("ECHO: '%s'", text) Line 156: return text
http://gerrit.ovirt.org/#/c/26750/6/vdsm/clientIF.py File vdsm/clientIF.py:
Line 166: port = config.getint('addresses', 'json_port') Line 167: truststore_path = None Line 168: if config.getboolean('vars', 'ssl'): Line 169: truststore_path = config.get('vars', 'trust_store_path') Line 170: #TODO: update config.py better to update config.py in this patch Line 171: conf = [('tcp', {"ip": ip, "port": port}), Line 172: ('amqp', {"ip": ip, "port": 5672}), Line 173: ('stomp', {"ip": ip, "port": 61613})] Line 174: self.bindings['json'] = BindingJsonRpc(DynamicBridge(), conf,