Nir Soffer has posted comments on this change. Change subject: stomp: make sure to send error message when no subscription ......................................................................
Patch Set 3: (1 comment) https://gerrit.ovirt.org/#/c/43724/3/lib/yajsonrpc/stompreactor.py File lib/yajsonrpc/stompreactor.py: Line 194: self._send_error("Subscription not available", Line 195: dispatcher.connection) Line 196: return Line 197: Line 198: if not subs: > """It is expected behavior. If there was no-one who subscribed for particul "if not something" is the best way to test for emptiness. if subs == [] is more correct only if we care about the type (list), and we don't care about it at all. If we can remove empty subs lists from the dictionary when the last subscription is removed, it may be nicer, since we will never get an empty list. But this requires locking in case one thread remove the last subscription while another thread is trying to get the subscription list. For example, you may get the list when it is not empty, switched to another thread removing the last sub, switch back and find an empty list again. Line 199: self._send_error("Subscription not available", Line 200: dispatcher.connection) Line 201: return Line 202: -- To view, visit https://gerrit.ovirt.org/43724 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1880caa04bb5e506679da046a58491d3e29e10e2 Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Piotr Kliczewski <piotr.kliczew...@gmail.com> Gerrit-Reviewer: Dima Kuznetsov <dmitryk...@gmail.com> Gerrit-Reviewer: Francesco Romani <from...@redhat.com> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer <nsof...@redhat.com> Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczew...@gmail.com> Gerrit-Reviewer: Yaniv Bronhaim <ybron...@redhat.com> Gerrit-Reviewer: Yeela Kaplan <ykap...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
