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