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