Change in vdsm[master]: stomp: client side subscription

ykaplan at redhat.com ykaplan at redhat.com
Wed May 13 11:15:32 UTC 2015


Yeela Kaplan has posted comments on this change.

Change subject: stomp: client side subscription
......................................................................


Patch Set 19: Code-Review-1

(6 comments)

https://gerrit.ovirt.org/#/c/36368/19/lib/yajsonrpc/stomp.py
File lib/yajsonrpc/stomp.py:

Line 297: self
shouldn't is_empty return True when deque is empty, meaning len==0?

Looks like it's doing the opposite...


Line 379: _frame_handler
why not use pop_message in the first place?


Line 484: None
I think 'if headers' might be enough...


https://gerrit.ovirt.org/#/c/36368/19/lib/yajsonrpc/stompReactor.py
File lib/yajsonrpc/stompReactor.py:

Line 234:     def subscribe(
Line 235:         self,
Line 236:         *args,
Line 237:         **kwargs
Line 238:     ):
Why are the lines for the arguments separated?
That's different than any other vdsm modules.
And much less readable... 

I think it would be better to change it everywhere around this patch..
Line 239:         return self._aclient.subscribe(*args, **kwargs)
Line 240: 
Line 241:     def send(
Line 242:         self,


Line 242:         self,
Line 243:         message,
Line 244:         destination=None,
Line 245:         headers=None
Line 246:     ):
same
Line 247:         if destination is None:
Line 248:             destination = _DEFAULT_REQUEST_DESTINATION
Line 249: 
Line 250:         self.log.debug("Sending response")


Line 357: def StompRpcClient(
Line 358:     stomp_client,
Line 359:     request_queue,
Line 360:     response_queue
Line 361: ):
same
Line 362:     sub_id = _FAKE_SUB_ID if request_queue == _FAKE_SUB_ID else None
Line 363: 
Line 364:     return JsonRpcClient(
Line 365:         ClientRpcTransportAdapter(


-- 
To view, visit https://gerrit.ovirt.org/36368
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I8e43658f1cebd637ea3abf4396d388afa041ae71
Gerrit-PatchSet: 19
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Saggi Mizrahi <smizrahi at redhat.com>
Gerrit-Reviewer: Dima Kuznetsov <dkuznets at redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski at gmail.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei at redhat.com>
Gerrit-Reviewer: Yaniv Bronheim <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