Change in vdsm[master]: stomp: server side subscriptions

piotr.kliczewski at gmail.com piotr.kliczewski at gmail.com
Wed May 13 14:23:06 UTC 2015


Piotr Kliczewski has posted comments on this change.

Change subject: stomp: server side subscriptions
......................................................................


Patch Set 18:

(3 comments)

https://gerrit.ovirt.org/#/c/38451/18/lib/yajsonrpc/stomp.py
File lib/yajsonrpc/stomp.py:

Line 462:         sub_id = frame.headers.get(Headers.SUBSCRIPTION)
Line 463:         if sub_id is None:
Line 464:             self.log.warning(
Line 465:                 "Got message without subscription id '%s'",
Line 466:                 sub_id
> Why do we need to print None?
Will remove
Line 467:             )
Line 468:             return
Line 469:         sub = self._subscriptions.get(sub_id)
Line 470:         if sub is None:


https://gerrit.ovirt.org/#/c/38451/18/lib/yajsonrpc/stompReactor.py
File lib/yajsonrpc/stompReactor.py:

Line 107:         self.queue_frame(resp)
Line 108:         self._reactor.wakeup()
Line 109: 
Line 110:     def _cmd_subscribe(self, dispatcher, frame):
Line 111:         self.log.info("Subscribe command received")
> Wouldn't it be helpful to log the command received with the sender details 
You have this information when a client is connected.
Line 112:         destination = frame.headers.get("destination", None)
Line 113:         sub_id = frame.headers.get("id", None)
Line 114: 
Line 115:         if not destination or not sub_id:


Line 112:         destination = frame.headers.get("destination", None)
Line 113:         sub_id = frame.headers.get("id", None)
Line 114: 
Line 115:         if not destination or not sub_id:
Line 116:             self._send_error("Missing destination or id header",
> I would add 'subscription request is missing...' at the beginning...
Done
Line 117:                              dispatcher.connection)
Line 118:             return
Line 119: 
Line 120:         ack = frame.headers.get("ack", stomp.AckMode.AUTO)


-- 
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: 18
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: Jenkins CI
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-HasComments: Yes


More information about the vdsm-patches mailing list