Dan Kenigsberg has posted comments on this change.

Change subject: stomp: message tracking do not clean properly
......................................................................


Patch Set 1: Code-Review-1

(1 comment)

https://gerrit.ovirt.org/#/c/48616/1/lib/yajsonrpc/stompreactor.py
File lib/yajsonrpc/stompreactor.py:

Line 324:             response_id = resp.get("id")
Line 325:             destination = self._req_dest[response_id]
Line 326:             del self._req_dest[response_id]
Line 327:         except KeyError:
Line 328:             # we could have no reply-to
we could have found this bug much earlier if we had not had this evil 
swallowing of KeyError.

Piotr, how much noise is expected if we are to log something here? When is the 
response_id expected not to exist in the _req_dest dictionary?

Can we make the try-except block smaller, to catch ONLY the problem we care 
about? json.loads() should clearly be out of it.
Line 329:             pass
Line 330: 
Line 331:         try:
Line 332:             connections = self._sub_map[destination]


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic2ca7c7589fea2d7ad5a477123b14df17924fda0
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Piotr Kliczewski <[email protected]>
Gerrit-Reviewer: Dan Kenigsberg <[email protected]>
Gerrit-Reviewer: Francesco Romani <[email protected]>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Piotr Kliczewski <[email protected]>
Gerrit-Reviewer: Yaniv Bronhaim <[email protected]>
Gerrit-Reviewer: Yeela Kaplan <[email protected]>
Gerrit-Reviewer: gerrit-hooks <[email protected]>
Gerrit-HasComments: Yes
_______________________________________________
vdsm-patches mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Reply via email to