Francesco Romani has posted comments on this change.
Change subject: stomp: client side subscription ......................................................................
Patch Set 16:
(7 comments)
initial review, I need (much) more time to properly understand this big and complex patch.
https://gerrit.ovirt.org/#/c/36368/16//COMMIT_MSG Commit Message:
Line 7: stomp: client side subscription Line 8: Line 9: In this patch we introduce concept of subscription for client Line 10: perspective. We move queuing functionality out of AsyncDispatcher to Line 11: frame_handler. New StompRpcClient class is repsonsible for sending typo: "repsonsible" vs "responsible" Line 12: subscriptions and ClientRpcTransportAdapter class adds 'reply-to' header Line 13: using subscriptions id. Line 14: Line 15:
https://gerrit.ovirt.org/#/c/36368/16/lib/yajsonrpc/stomp.py File lib/yajsonrpc/stomp.py:
Line 51: ERROR = "ERROR" Line 52: RECEIPT = "RECEIPT" Line 53: Line 54: Line 55: class Headers: even though for enumeration-like classes is no harm, we should always innherit from object until we switch to python 3. Line 56: CONTENT_LENGTH = "content-length" Line 57: CONTENT_TYPE = "content-type" Line 58: SUBSCRIPTION = "subscription" Line 59: DESTINATION = "destination"
Line 505: return sub Line 506: Line 507: def unsubscribe(self, sub): Line 508: self.queue_frame(Frame(Command.UNSUBSCRIBE, Line 509: {"id": sub.id})) Who cleans self._subscriptions ? Line 510: Line 511: Line 512: class _Subscription(object): Line 513:
https://gerrit.ovirt.org/#/c/36368/16/lib/yajsonrpc/stompReactor.py File lib/yajsonrpc/stompReactor.py:
Line 97: cy = max(cy, 1000) Line 98: Line 99: # The server can send a heart-beat every cy ms and doesn't want Line 100: # to receive any heart-beat from the client. Line 101: resp.headers["heart-beat"] = "%d,0" % (cy,) minor nit: no need of use a tuple here for 'cy', this should work fine and save a few characters:
resp.headers["heart-beat"] = "%d,0" % cy
If you want to do this change, let's do in a (far) future different patch. Line 102: dispatcher.setHeartBeat(cy) Line 103: Line 104: self.queue_frame(resp) Line 105: self._reactor.wakeup()
Line 127: self._socket = sock Line 128: self._reactor = reactor Line 129: self._messageHandler = None Line 130: Line 131: self._aclient = aclient maybe _async_client is a bit more explanatory here Line 132: adisp = self._adisp = stomp.AsyncDispatcher(aclient) Line 133: self._dispatcher = Dispatcher(adisp, sock=sock, map=reactor._map) Line 134: Line 135: def send_raw(self, msg):
Line 198: return self._socket.getsockname()[0] Line 199: Line 200: Line 201: class StompClient(object): Line 202: log = logging.getLogger("jsonrpc.AsyncoreClient") for a future patch: this seems to get a stale logger now. Line 203: Line 204: def __init__(self, sock, reactor): Line 205: self._reactor = reactor Line 206: self._messageHandler = None
Line 219: Line 220: def connect(self): Line 221: self._stompConn.connect() Line 222: Line 223: def handle_message(self, sub, frame): why renamed? moreover, it seems unused (also in the old code) Line 224: if self._messageHandler is not None: Line 225: self._messageHandler((self, frame.body)) Line 226: Line 227: def setMessageHandler(self, msgHandler):