Merge lp:~alecu/ubuntuone-control-panel/webclient-shutdowns into lp:ubuntuone-control-panel

Proposed by Alejandro J. Cura
Status: Rejected
Rejected by: Alejandro J. Cura
Proposed branch: lp:~alecu/ubuntuone-control-panel/webclient-shutdowns
Merge into: lp:ubuntuone-control-panel
Diff against target: 226 lines (+190/-1)
3 files modified
ubuntuone/controlpanel/web_client/tests/__init__.py (+19/-0)
ubuntuone/controlpanel/web_client/tests/test_txwebclient.py (+153/-0)
ubuntuone/controlpanel/web_client/txwebclient.py (+18/-1)
To merge this branch: bzr merge lp:~alecu/ubuntuone-control-panel/webclient-shutdowns
Reviewer Review Type Date Requested Status
Natalia Bidart Needs Fixing
Review via email: mp+74708@code.launchpad.net

Commit message

- Do not throw a webclient error when closing (LP: #845105).

Description of the change

Do not throw a webclient error when closing (LP: #845105)

To post a comment you must log in.
Natalia Bidart (nataliabidart) wrote :

The branch looks good, but I'm getting this in windows:

===============================================================================
[ERROR]
Traceback (most recent call last):
Failure: twisted.trial.util.DirtyReactorAggregateError: Reactor was unclean.
DelayedCalls: (set twisted.internet.base.DelayedCall.debug = True to debug)
<DelayedCall 0x5dad710 [29.004999876s] called=0 cancelled=0 Client.failIfNotConn
ected(TimeoutError('',))>

ubuntuone.controlpanel.web_client.tests.test_txwebclient.WebClientShutdownTestCa
se.test_shutdown
===============================================================================
[ERROR]
Traceback (most recent call last):
Failure: twisted.trial.util.DirtyReactorAggregateError: Reactor was unclean.
Selectables:
<<class 'twisted.internet.tcp.Client'> to ('localhost', 50832) at 5dac6d0>

ubuntuone.controlpanel.web_client.tests.test_txwebclient.WebClientShutdownTestCa
se.test_shutdown
-------------------------------------------------------------------------------

As far as I can see, if the test finished because d2 was fired, the deferred d3 leaves the reactor unclean with the callLater thing added.

review: Needs Fixing
Natalia Bidart (nataliabidart) wrote :

It works great IRL!!!

Unmerged revisions

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== added directory 'ubuntuone/controlpanel/web_client/tests'
2=== added file 'ubuntuone/controlpanel/web_client/tests/__init__.py'
3--- ubuntuone/controlpanel/web_client/tests/__init__.py 1970-01-01 00:00:00 +0000
4+++ ubuntuone/controlpanel/web_client/tests/__init__.py 2011-09-09 00:01:23 +0000
5@@ -0,0 +1,19 @@
6+# -*- coding: utf-8 -*-
7+
8+# Authors: Alejandro J. Cura <alecu@canonical.com>
9+#
10+# Copyright 2011 Canonical Ltd.
11+#
12+# This program is free software: you can redistribute it and/or modify it
13+# under the terms of the GNU General Public License version 3, as published
14+# by the Free Software Foundation.
15+#
16+# This program is distributed in the hope that it will be useful, but
17+# WITHOUT ANY WARRANTY; without even the implied warranties of
18+# MERCHANTABILITY, SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR
19+# PURPOSE. See the GNU General Public License for more details.
20+#
21+# You should have received a copy of the GNU General Public License along
22+# with this program. If not, see <http://www.gnu.org/licenses/>.
23+
24+"""Unit tests for the control panel backend webservice clients."""
25
26=== added file 'ubuntuone/controlpanel/web_client/tests/test_txwebclient.py'
27--- ubuntuone/controlpanel/web_client/tests/test_txwebclient.py 1970-01-01 00:00:00 +0000
28+++ ubuntuone/controlpanel/web_client/tests/test_txwebclient.py 2011-09-09 00:01:23 +0000
29@@ -0,0 +1,153 @@
30+# -*- coding: utf-8 -*-
31+
32+# Authors: Alejandro J. Cura <alecu@canonical.com>
33+#
34+# Copyright 2011 Canonical Ltd.
35+#
36+# This program is free software: you can redistribute it and/or modify it
37+# under the terms of the GNU General Public License version 3, as published
38+# by the Free Software Foundation.
39+#
40+# This program is distributed in the hope that it will be useful, but
41+# WITHOUT ANY WARRANTY; without even the implied warranties of
42+# MERCHANTABILITY, SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR
43+# PURPOSE. See the GNU General Public License for more details.
44+#
45+# You should have received a copy of the GNU General Public License along
46+# with this program. If not, see <http://www.gnu.org/licenses/>.
47+
48+"""Unit tests for the control panel backend twisted webservice client."""
49+
50+from twisted.application import internet, service
51+from twisted.internet import defer, reactor
52+from twisted.internet.defer import inlineCallbacks
53+from twisted.web import server, resource
54+
55+from ubuntuone.controlpanel.web_client import txwebclient
56+from ubuntuone.controlpanel.tests import TestCase
57+
58+
59+SAMPLE_KEY = "result"
60+SAMPLE_VALUE = "sample result"
61+SAMPLE_RESOURCE = '{"%s": "%s"}' % (SAMPLE_KEY, SAMPLE_VALUE)
62+SAMPLE_CREDENTIALS = dict(
63+ consumer_key="consumer key",
64+ consumer_secret="consumer secret",
65+ token="the real token",
66+ token_secret="the token secret",
67+)
68+
69+
70+def sample_get_credentials():
71+ """Will return the sample credentials right now."""
72+ return defer.succeed(SAMPLE_CREDENTIALS)
73+
74+
75+class MockResource(resource.Resource):
76+ """A simple web resource."""
77+ isLeaf = True
78+ contents = ""
79+
80+ # pylint: disable=C0103
81+ # t.w.resource methods have freeform cased names
82+
83+ def getChild(self, name, request):
84+ """Get a given child resource."""
85+ if name == '':
86+ return self
87+ return resource.Resource.getChild(self, name, request)
88+
89+ def render_GET(self, request):
90+ """Make a bit of html out of these resource's content."""
91+ return self.contents
92+
93+
94+class MockWebService(object):
95+ """A mock webservice for testing"""
96+
97+ def __init__(self):
98+ """Start up this instance."""
99+ root = resource.Resource()
100+ devices_resource = MockResource()
101+ devices_resource.contents = SAMPLE_RESOURCE
102+ root.putChild("devices", devices_resource)
103+ root.putChild("throwerror", resource.NoResource())
104+ unauthorized = resource.ErrorPage(resource.http.UNAUTHORIZED,
105+ "Unauthrorized", "Unauthrorized")
106+ root.putChild("unauthorized", unauthorized)
107+
108+ site = server.Site(root)
109+ application = service.Application('web')
110+ self.service_collection = service.IServiceCollection(application)
111+ #pylint: disable=E1101
112+ self.tcpserver = internet.TCPServer(0, site)
113+ self.tcpserver.setServiceParent(self.service_collection)
114+ self.service_collection.startService()
115+
116+ def get_url(self):
117+ """Build the url for this mock server."""
118+ #pylint: disable=W0212
119+ port_num = self.tcpserver._port.getHost().port
120+ return "http://localhost:%d/" % port_num
121+
122+ def stop(self):
123+ """Shut it down."""
124+ #pylint: disable=E1101
125+ self.service_collection.stopService()
126+
127+
128+class WebClientTestCase(TestCase):
129+ """Test for the webservice client."""
130+
131+ timeout = 8
132+
133+ def setUp(self):
134+ super(WebClientTestCase, self).setUp()
135+ self.ws = MockWebService()
136+ test_base_url = self.ws.get_url()
137+ self.wc = txwebclient.WebClient(sample_get_credentials, test_base_url)
138+
139+ @inlineCallbacks
140+ def tearDown(self):
141+ super(WebClientTestCase, self).tearDown()
142+ yield self.ws.stop()
143+ self.wc.shutdown()
144+
145+ @inlineCallbacks
146+ def test_get_url(self):
147+ """A method is successfully called in the mock webservice."""
148+ result = yield self.wc.call_api("devices")
149+ self.assertIn(SAMPLE_KEY, result)
150+ self.assertEqual(SAMPLE_VALUE, result[SAMPLE_KEY])
151+
152+ @inlineCallbacks
153+ def test_get_url_error(self):
154+ """The errback is called when there's some error."""
155+ yield self.assertFailure(self.wc.call_api("throwerror"),
156+ txwebclient.WebClientError)
157+
158+ @inlineCallbacks
159+ def test_unauthorized(self):
160+ """Detect when a request failed with UNAUTHORIZED."""
161+ yield self.assertFailure(self.wc.call_api("unauthorized"),
162+ txwebclient.UnauthorizedError)
163+
164+
165+class WebClientShutdownTestCase(TestCase):
166+ """The webclient behaviour during shutdown."""
167+
168+ @inlineCallbacks
169+ def test_shutdown(self):
170+ """The webclient behaves well during shutdown."""
171+ d3 = defer.Deferred()
172+ # pylint: disable=E1101
173+ reactor.callLater(1, d3.callback, None)
174+ ws = MockWebService()
175+ test_base_url = ws.get_url()
176+ wc = txwebclient.WebClient(sample_get_credentials, test_base_url)
177+ d1 = wc.call_api("throwerror")
178+ d2 = ws.stop()
179+ wc.shutdown()
180+ yield d2
181+ yield defer.DeferredList([d1, d3], fireOnOneCallback=True,
182+ fireOnOneErrback=True)
183
184=== modified file 'ubuntuone/controlpanel/web_client/txwebclient.py'
185--- ubuntuone/controlpanel/web_client/txwebclient.py 2011-06-02 13:14:51 +0000
186+++ ubuntuone/controlpanel/web_client/txwebclient.py 2011-09-09 00:01:23 +0000
187@@ -20,6 +20,7 @@
188
189 import simplejson
190
191+from twisted.internet import defer, reactor
192 from twisted.web import client, error, http
193
194 from ubuntuone.controlpanel import WEBSERVICE_BASE_URL
195@@ -39,6 +40,10 @@
196 """Initialize the webclient."""
197 self.base_url = base_url
198 self.get_credentials = get_credentials
199+ self.running = True
200+ # pylint: disable=E1101
201+ self.trigger_id = reactor.addSystemEventTrigger("before", "shutdown",
202+ self.shutdown)
203
204 def _handle_response(self, result):
205 """Handle the response of the webservice call."""
206@@ -74,7 +79,19 @@
207 d = self.get_credentials()
208 d.addErrback(self._handle_error)
209 d.addCallback(self._call_api_with_creds, api_name)
210- return d
211+ d2 = defer.Deferred()
212+ d.addCallback(d2.callback)
213+
214+ def mask_errors_on_shutdown(failure):
215+ """Do not fire the errbacks if we are shutting down."""
216+ if self.running:
217+ d2.errback(failure)
218+
219+ d.addErrback(mask_errors_on_shutdown)
220+ return d2
221
222 def shutdown(self):
223 """End the pending webclient calls."""
224+ self.running = False
225+ # pylint: disable=E1101
226+ reactor.removeSystemEventTrigger(self.trigger_id)

Subscribers

People subscribed via source and target branches