1:42 a.m.
Piotr Kliczewski has uploaded a new change for review.
Change subject: stomp: broken unsubscribe process
......................................................................
stomp: broken unsubscribe process
There is a logic common for incoming and outgoing connection which
cleans the subscription. In case on incoming connection the code is
broken due missing unsubscribe method on _StompConnection.
Change-Id: I45c5741f4d94b7af138e98661cfe418cbb0a2b6b
Signed-off-by: pkliczewski <piotr.kliczewski(a)gmail.com>
---
M lib/yajsonrpc/stompreactor.py
1 file changed, 3 insertions(+), 0 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/17/45617/1
diff --git a/lib/yajsonrpc/stompreactor.py b/lib/yajsonrpc/stompreactor.py
index fa6994b..59bdcc0 100644
--- a/lib/yajsonrpc/stompreactor.py
+++ b/lib/yajsonrpc/stompreactor.py
@@ -281,6 +281,9 @@
def is_closed(self):
return not self._dispatcher.connected
+ def unsubscribe(self, sub):
+ pass
+
class StompServer(object):
log = logging.getLogger("yajsonrpc.StompServer")
--
To view, visit https://gerrit.ovirt.org/45617
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I45c5741f4d94b7af138e98661cfe418cbb0a2b6b
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Piotr Kliczewski <piotr.kliczewski(a)gmail.com>
1:42 a.m.
automation(a)ovirt.org has posted comments on this change.
Change subject: stomp: broken unsubscribe process
......................................................................
Patch Set 1:
* Update tracker::IGNORE, no Bug-Url found
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and
is a valid url.
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5',
'ovirt-3.4', 'ovirt-3.3'])
--
To view, visit https://gerrit.ovirt.org/45617
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I45c5741f4d94b7af138e98661cfe418cbb0a2b6b
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Piotr Kliczewski <piotr.kliczewski(a)gmail.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: No
1:44 a.m.
Piotr Kliczewski has posted comments on this change.
Change subject: stomp: broken unsubscribe process
......................................................................
Patch Set 1: Verified+1
Verified together with jsonrpc fd leak patch and split later from it. Verification flag
copied from this patch.
--
To view, visit https://gerrit.ovirt.org/45617
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I45c5741f4d94b7af138e98661cfe418cbb0a2b6b
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Piotr Kliczewski <piotr.kliczewski(a)gmail.com>
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski(a)gmail.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: No
10:14 a.m.
Nir Soffer has posted comments on this change.
Change subject: stomp: broken unsubscribe process
......................................................................
Patch Set 1:
(1 comment)
https://gerrit.ovirt.org/#/c/45617/1/lib/yajsonrpc/stompreactor.py
File lib/yajsonrpc/stompreactor.py:
Line 281: def is_closed(self):
Line 282: return not self._dispatcher.connected
Line 283:
Line 284: def unsubscribe(self, sub):
Line 285: pass
Why this class need default empty implementation? I don't see any other mention of
subscriptions handling here.
Line 286:
Line 287:
Line 288: class StompServer(object):
Line 289: log = logging.getLogger("yajsonrpc.StompServer")
--
To view, visit https://gerrit.ovirt.org/45617
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I45c5741f4d94b7af138e98661cfe418cbb0a2b6b
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Piotr Kliczewski <piotr.kliczewski(a)gmail.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski(a)gmail.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com>
Gerrit-Reviewer: Yeela Kaplan <ykaplan(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: Yes
2:29 a.m.
Piotr Kliczewski has posted comments on this change.
Change subject: stomp: broken unsubscribe process
......................................................................
Patch Set 1:
(1 comment)
https://gerrit.ovirt.org/#/c/45617/1/lib/yajsonrpc/stompreactor.py
File lib/yajsonrpc/stompreactor.py:
Line 281: def is_closed(self):
Line 282: return not self._dispatcher.connected
Line 283:
Line 284: def unsubscribe(self, sub):
Line 285: pass
Why this class need default empty implementation? I don't see any
other men
Unsubscribe process was created for outgoing connection and as you know we
tend to reuse bunch of code and we miss this method on incoming connection. We talk about
here "bad" client when the connection is closed without unsubscription.
For incoming connection we clean subscriptions when calling dispatcher#handle_close.
Line 286:
Line 287:
Line 288: class StompServer(object):
Line 289: log = logging.getLogger("yajsonrpc.StompServer")
--
To view, visit https://gerrit.ovirt.org/45617
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I45c5741f4d94b7af138e98661cfe418cbb0a2b6b
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Piotr Kliczewski <piotr.kliczewski(a)gmail.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski(a)gmail.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com>
Gerrit-Reviewer: Yeela Kaplan <ykaplan(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: Yes
3162
days inactive
3163
days old
vdsm-patches@lists.fedorahosted.org
4 comments
3 participants
participants (3)
-
automation＠ovirt.org -
Nir Soffer -
Piotr Kliczewski