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,
--
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: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: Barak Azulay <bazulay(a)redhat.com>
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski(a)gmail.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes