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(