Change in vdsm[master]: stomp: server side subscriptions
piotr.kliczewski at gmail.com
piotr.kliczewski at gmail.com
Mon Apr 27 12:21:46 UTC 2015
Piotr Kliczewski has posted comments on this change.
Change subject: stomp: server side subscriptions
......................................................................
Patch Set 15:
(7 comments)
https://gerrit.ovirt.org/#/c/38451/15//COMMIT_MSG
Commit Message:
Line 5: CommitDate: 2015-04-24 10:50:31 +0200
Line 6:
Line 7: stomp: server side subscriptions
Line 8:
Line 9: Vdsm gains abitlity to manage subsctiptions and act as mini broker.
> typos: "abitlity", "subsctiptions"
Done
Line 10: We change model from connection centric to subscription centric.
Line 11: Following use cases are covered by this change:
Line 12: - clients can subscribe and exchange messages without executing vdsm API
Line 13: - legacy (3.5) client can interact with vdsm and messages will be
https://gerrit.ovirt.org/#/c/38451/15/lib/yajsonrpc/stompReactor.py
File lib/yajsonrpc/stompReactor.py:
Line 152: try:
Line 153: del self._sub_ids[sub_id]
Line 154: except KeyError:
Line 155: # nothing to clean
Line 156: return
> then maybe you can just use pop() and simplify lines 145-156 above?
Done
Line 157: subs = self._sub_dests[subscription.destination]
Line 158: if len(subs) == 1:
Line 159: del self._sub_dests[subscription.destination]
Line 160: else:
Line 174: frame.body)
Line 175: return
Line 176:
Line 177: subs = self._sub_dests[destination]
Line 178: if len(subs) == 0:
> if not subs:
Done
Line 179: self._send_error("Subscription not available",
Line 180: dispatcher.connection)
Line 181: return
Line 182:
Line 181: return
Line 182:
Line 183: for subscription in subs:
Line 184: headers = utils.picklecopy(frame.headers)
Line 185: headers[stomp.Headers.SUBSCRIPTION] = subscription.id
> probably faster, probably more readable:
Done
Line 186: res = stomp.Frame(
Line 187: stomp.Command.MESSAGE,
Line 188: headers,
Line 189: frame.body
Line 197: or for standard mode we use 'reply-to' header.
Line 198: """
Line 199: try:
Line 200: self._handle_destination(dispatcher, req_dest, json.loads(request))
Line 201: except:
> please use at least
Done
Line 202: # let json server process issue
Line 203: pass
Line 204: dispatcher.connection.handleMessage(request)
Line 205:
Line 267:
Line 268: def __init__(self, reactor):
Line 269: self._reactor = reactor
Line 270: self._messageHandler = None
Line 271: self._sub_map = defaultdict(list)
> do we need to serve clients in subscriptions order?
We are using it to simplify the code.
Line 272: self._req_dest = {}
Line 273:
Line 274: def add_client(self, sock):
Line 275: adapter = StompAdapterImpl(self._reactor, self._sub_map,
Line 282: destination = _DEFAULT_RESPONSE_DESTINATIOM
Line 283: try:
Line 284: resp = json.loads(message)
Line 285: destination = self._req_dest[resp.get("id")]
Line 286: del self._req_dest[id]
> is 'id' here intentional?
Will fix
Line 287: except:
Line 288: # we could have no reply-to
Line 289: pass
Line 290:
--
To view, visit https://gerrit.ovirt.org/38451
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I1493070f2ba66ca9d39a6661876c82c4727cad62
Gerrit-PatchSet: 15
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Piotr Kliczewski <piotr.kliczewski at gmail.com>
Gerrit-Reviewer: Dima Kuznetsov <dkuznets at redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek at redhat.com>
Gerrit-Reviewer: Michal Skrivanek <mskrivan at redhat.com>
Gerrit-Reviewer: Oved Ourfali <oourfali at redhat.com>
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski at gmail.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei at redhat.com>
Gerrit-Reviewer: Yeela Kaplan <ykaplan at redhat.com>
Gerrit-Reviewer: automation at ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
More information about the vdsm-patches
mailing list