Change in vdsm[master]: RPC: Recieve and parse messages from multiple subscriptions

[Code Review]

From Yaniv Bronhaim <ybronhei(a)redhat.com&gt;:

Yaniv Bronhaim has posted comments on this change.

Change subject: RPC: Recieve and parse messages from multiple subscriptions
......................................................................


Patch Set 23:

(6 comments)

https://gerrit.ovirt.org/#/c/78656/23//COMMIT_MSG
Commit Message:

Line 7: RPC: Recieve and parse messages from multiple subscriptions
Line 8: 
Line 9: JsonRpcClient parses json messages
Line 10: from multiple subscriptions and calls
Line 11: the registered callback.
I really don't understand what you're doing in that patch from the commit msg..
please explain - from the text here I understand that JsonRpcClient parse msgs, and call
callbacks.. wasn't it like that before? do you add something new?
Line 12: 
Line 13: Change-Id: I4f5df1a5208281241be82f9e651788eca520be64


https://gerrit.ovirt.org/#/c/78656/23/lib/yajsonrpc/__init__.py
File lib/yajsonrpc/__init__.py:

Line 363:     def __init__(self, transport):
Line 364:         self.log = logging.getLogger("jsonrpc.JsonRpcClient")
Line 365:         transport.set_message_handler(
Line 366:             lambda msg: self._handleMessage(msg)
Line 367:         )
why is it needed?
Line 368: 
Line 369:         self._transport = transport
Line 370:         self._runningRequests = {}
Line 371:         self._lock = Lock()


Line 417:             lambda msg: self._handleMessage(msg, callback)
Line 418:         )
Line 419: 
Line 420:     def unsubscribe(self, sub_id):
Line 421:         self._transport.unsubscribe(sub_id)
renaming - separate patch
you can make one patch for all renames. I'd love that
Line 422: 
Line 423:     def notify(self, event_id, dest, event_schema, params=None):
Line 424:         """
Line 425:         JsonRpcClient notify method, sends an event on a spesific queue


Line 454:         if isinstance(obj, list):
Line 455:             map(self._processIncomingResponse, obj)
Line 456:             return
Line 457: 
Line 458:         resp = JsonRpcResponse.fromRawObject(obj)
renaming - separate patch
Line 459:         with self._lock:
Line 460:             if resp.id is None:
Line 461:                 self.log.warning(
Line 462:                     "Got an error from server without an ID (%s)",


Line 487:             mobj = json.loads(message)
Line 488:             isResponse = self._isResponse(mobj)
Line 489:         except:
Line 490:             self.log.exception("Problem parsing message from client")
Line 491:             return
this is big change\fix... you must explain. it worth separate patch
Line 492: 
Line 493:         if isResponse:
Line 494:             self._processIncomingResponse(mobj)
Line 495:         else:


https://gerrit.ovirt.org/#/c/78656/23/lib/yajsonrpc/stompreactor.py
File lib/yajsonrpc/stompreactor.py:

Line 610
Line 611
Line 612
Line 613
Line 614
no more _handle_message calls?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I4f5df1a5208281241be82f9e651788eca520be64
Gerrit-PatchSet: 23
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Andrej Krejcir <akrejcir(a)redhat.com&gt;
Gerrit-Reviewer: Andrej Krejcir <akrejcir(a)redhat.com&gt;
Gerrit-Reviewer: Denis Chaplygin <dchaplyg(a)redhat.com&gt;
Gerrit-Reviewer: Irit Goihman <igoihman(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Jenny Tokar <jtokar(a)redhat.com&gt;
Gerrit-Reviewer: Martin Sivák <msivak(a)redhat.com&gt;
Gerrit-Reviewer: Phillip Bailey <phbailey(a)redhat.com&gt;
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski(a)gmail.com&gt;
Gerrit-Reviewer: Roman Mohr <rmohr(a)redhat.com&gt;
Gerrit-Reviewer: Yanir Quinn <yquinn(a)redhat.com&gt;
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org&gt;
Gerrit-HasComments: Yes