Merge lp:~mvo/software-center/dbus-idle-timeout into lp:software-center

Proposed by Michael Vogt on 2012-10-01
Status: Merged
Merged at revision: 3217
Proposed branch: lp:~mvo/software-center/dbus-idle-timeout
Merge into: lp:software-center
Diff against target: 219 lines (+113/-8)
3 files modified
software-center-dbus (+3/-1)
softwarecenter/db/dataprovider.py (+63/-6)
tests/test_dataprovider.py (+47/-1)
To merge this branch: bzr merge lp:~mvo/software-center/dbus-idle-timeout
Reviewer Review Type Date Requested Status
Gary Lasker (community) 2012-10-01 Approve on 2012-10-05
Review via email: mp+127208@code.launchpad.net

Description of the change

This branch addresses bug #1058567 to ensure that the data provider
exits after a certain amount of time.

To post a comment you must log in.
Gary Lasker (gary-lasker) wrote :

Hi Michael, thanks for your branch! Strangely, I'm getting a bunch of errors when I run the associated unit test, test_dataprovider.py, as shown at the following pastebin:

  http://paste.ubuntu.com/1255206/

I'm not seeing these errors in current trunk. Do you have an idea about why I might be seeing these?

Many thanks!

3212. By Michael Vogt on 2012-10-02

move implementation of the dbus methods into a seperate function so that we can add decorators - the dbus.service.method decorator seems to not like additional decroators

Michael Vogt (mvo) wrote :

Thanks, indeed, it looks like the dbus decorator does not like other decorators at the same time. I fixed this now and the tests work now on my box.

Michael Vogt (mvo) wrote :

Anything holding this one back?

Gary Lasker (gary-lasker) wrote :

Hi Michael, I'm sorry for the delay on this one! Ok, I just tried it and I'm still getting an error in the test. It seems that SoftwareCenterDataProvider's __init__ doesn't like the Mock self.IDLE_CHECK_INTERVAL being passed into the GObject.timeout_add_seconds() that happens in test_idle_timeout:

======================================================================
ERROR: test_idle_timeout (__main__.IdleTimeoutTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "tests/test_dataprovider.py", line 195, in test_idle_timeout
    self.bus_name, main_loop=self.loop)
  File "/home/tremolux/Projects/quantal/software-center_review_mvo_dbus_idle_timeout/dbus-idle-timeout/softwarecenter/db/dataprovider.py", line 89, in __init__
    self.IDLE_CHECK_INTERVAL, self._check_inactivity)
TypeError: an integer is required

----------------------------------------------------------------------
Ran 15 tests in 8.076s

FAILED (errors=1)

Gary Lasker (gary-lasker) wrote :

Note that the above is when testing on Precise. I'll try this on a Quantal machine and see if I still get the error.

Gary Lasker (gary-lasker) wrote :

Meanwhile I will approve this as the only issue is the failing test case. Michael, please feel free to merge this branch when the test is ready.

Thanks!

review: Approve
Michael Vogt (mvo) wrote :

Thanks for the review, I just double checked, it fails on precise for me as well but works on quantal.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'software-center-dbus'
2--- software-center-dbus 2012-07-18 12:45:55 +0000
3+++ software-center-dbus 2012-10-02 07:44:21 +0000
4@@ -18,9 +18,11 @@
5 # 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
6
7 import logging
8+import sys
9 from softwarecenter.db.dataprovider import dbus_main
10
11
12 if __name__ == "__main__":
13- logging.basicConfig(level=logging.DEBUG)
14+ if "--debug" in sys.argv:
15+ logging.basicConfig(level=logging.DEBUG)
16 dbus_main()
17
18=== modified file 'softwarecenter/db/dataprovider.py'
19--- softwarecenter/db/dataprovider.py 2012-09-18 11:36:15 +0000
20+++ softwarecenter/db/dataprovider.py 2012-10-02 07:44:21 +0000
21@@ -19,6 +19,7 @@
22 import dbus
23 import dbus.service
24 import logging
25+import time
26
27 from dbus.mainloop.glib import DBusGMainLoop
28 DBusGMainLoop(set_as_default=True)
29@@ -50,11 +51,27 @@
30 DBUS_DATA_PROVIDER_PATH = '/com/ubuntu/SoftwareCenterDataProvider'
31
32
33+def update_activity_timestamp(fn):
34+ def wrapped(*args, **kwargs):
35+ self = args[0]
36+ self._update_activity_timestamp()
37+ return fn(*args, **kwargs)
38+ return wrapped
39+
40+
41 class SoftwareCenterDataProvider(dbus.service.Object):
42
43- def __init__(self, bus_name, object_path=DBUS_DATA_PROVIDER_PATH):
44+ # 5 min by default
45+ IDLE_TIMEOUT = 60 * 5
46+ IDLE_CHECK_INTERVAL = 60
47+
48+ def __init__(self, bus_name, object_path=DBUS_DATA_PROVIDER_PATH,
49+ main_loop=None):
50 dbus.service.Object.__init__(self, bus_name, object_path)
51 self.bus_name = bus_name
52+ if main_loop is None:
53+ main_loop = GObject.MainLoop(GObject.main_context_default())
54+ self.main_loop = main_loop
55 # the database
56 self.db = StoreDatabase()
57 self.db.open()
58@@ -66,16 +83,41 @@
59 self.review_loader.refresh_review_stats()
60 # ensure we query new applications
61 run_software_center_agent(self.db)
62+ # setup inactivity timer
63+ self._update_activity_timestamp()
64+ self._idle_timeout = GObject.timeout_add_seconds(
65+ self.IDLE_CHECK_INTERVAL, self._check_inactivity)
66
67 def stop(self):
68 """ stop the dbus controller and remove from the bus """
69- self.remove_from_connection()
70-
71+ LOG.debug("stop() called")
72+ self.main_loop.quit()
73+ LOG.debug("exited")
74+
75+ # internal helper
76+ def _check_inactivity(self):
77+ """ Check for activity """
78+ now = time.time()
79+ if (self._activity_timestamp + self.IDLE_TIMEOUT) < now:
80+ LOG.info("stopping after %s inactivity" % self.IDLE_TIMEOUT)
81+ self.stop()
82+ return True
83+
84+ def _update_activity_timestamp(self):
85+ self._activity_timestamp = time.time()
86+
87+ # public dbus methods with their implementations, the dbus decorator
88+ # does not like additional decorators so we use a seperate function
89+ # for the actual implementation
90 @dbus.service.method(DBUS_DATA_PROVIDER_IFACE,
91 in_signature='ss', out_signature='a{sv}')
92 def GetAppDetails(self, appname, pkgname):
93 LOG.debug("GetAppDetails() called with ('%s', '%s')" % (
94 appname, pkgname))
95+ return self._get_app_details(appname, pkgname)
96+
97+ @update_activity_timestamp
98+ def _get_app_details(self, appname, pkgname):
99 app = Application(appname, pkgname)
100 appdetails = app.get_details(self.db)
101 return appdetails.as_dbus_property_dict()
102@@ -84,12 +126,20 @@
103 in_signature='', out_signature='as')
104 def GetAvailableCategories(self):
105 LOG.debug("GetAvailableCategories() called")
106+ return self._get_available_categories()
107+
108+ @update_activity_timestamp
109+ def _get_available_categories(self):
110 return [cat.name for cat in self.categories]
111
112 @dbus.service.method(DBUS_DATA_PROVIDER_IFACE,
113 in_signature='s', out_signature='as')
114 def GetAvailableSubcategories(self, category_name):
115 LOG.debug("GetAvailableSubcategories() called")
116+ return self._get_available_subcategories(category_name)
117+
118+ @update_activity_timestamp
119+ def _get_available_subcategories(self, category_name):
120 cat = get_category_by_name(self.categories, category_name)
121 return [subcat.name for subcat in cat.subcategories]
122
123@@ -97,6 +147,10 @@
124 in_signature='s', out_signature='a(ssss)')
125 def GetItemsForCategory(self, category_name):
126 LOG.debug("GetItemsForCategory() called with ('%s')" % category_name)
127+ return self._get_items_for_category(category_name)
128+
129+ @update_activity_timestamp
130+ def _get_items_for_category(self, category_name):
131 result = []
132 cat = get_category_by_name(self.categories, category_name)
133 for doc in cat.get_documents(self.db):
134@@ -112,10 +166,13 @@
135 def dbus_main(bus=None):
136 if bus is None:
137 bus = dbus.SessionBus()
138+
139+ main_context = GObject.main_context_default()
140+ main_loop = GObject.MainLoop(main_context)
141+
142 bus_name = dbus.service.BusName(DBUS_BUS_NAME, bus)
143- data_provider = SoftwareCenterDataProvider(bus_name)
144+ data_provider = SoftwareCenterDataProvider(bus_name, main_loop=main_loop)
145 data_provider # pyflakes
146
147- main_context = GObject.main_context_default()
148- main_loop = GObject.MainLoop(main_context)
149+ # run it
150 main_loop.run()
151
152=== modified file 'tests/test_dataprovider.py'
153--- tests/test_dataprovider.py 2012-09-20 01:05:10 +0000
154+++ tests/test_dataprovider.py 2012-10-02 07:44:21 +0000
155@@ -4,10 +4,15 @@
156 import time
157 import unittest
158
159+from gi.repository import GObject
160+
161 from dbus.mainloop.glib import DBusGMainLoop
162 DBusGMainLoop(set_as_default=True)
163
164-from mock import Mock
165+from mock import (
166+ Mock,
167+ patch,
168+ )
169
170 from tests.utils import (
171 kill_process,
172@@ -162,6 +167,47 @@
173 self.assertEqual(len(result), 20)
174
175
176+class IdleTimeoutTestCase(unittest.TestCase):
177+
178+ def setUp(self):
179+ self.loop = GObject.MainLoop(GObject.main_context_default())
180+
181+ # setup bus
182+ dbus_service_name = DBUS_BUS_NAME
183+ proc, dbus_address = start_dbus_daemon()
184+ bus = dbus.bus.BusConnection(dbus_address)
185+ # run the checks
186+ self.bus_name = dbus.service.BusName(dbus_service_name, bus)
187+
188+ def tearDown(self):
189+ self.loop.quit()
190+
191+ @patch.object(SoftwareCenterDataProvider, "IDLE_TIMEOUT")
192+ @patch.object(SoftwareCenterDataProvider, "IDLE_CHECK_INTERVAL")
193+ def test_idle_timeout(self, mock_timeout, mock_interval):
194+ mock_timeout = 1
195+ mock_timeout # pyflakes
196+ mock_interval = 1
197+ mock_interval # pyflakes
198+
199+ now = time.time()
200+ provider = SoftwareCenterDataProvider(
201+ self.bus_name, main_loop=self.loop)
202+ provider # pyflakes
203+ self.loop.run()
204+ # ensure this exited within a reasonable timeout
205+ self.assertTrue((time.time() - now) < 5)
206+
207+ def test_idle_timeout_updates(self):
208+ provider = SoftwareCenterDataProvider(
209+ self.bus_name, main_loop=self.loop)
210+ t1 = provider._activity_timestamp
211+ time.sleep(0.1)
212+ provider.GetAvailableCategories()
213+ t2 = provider._activity_timestamp
214+ self.assertTrue(t1 < t2)
215+
216+
217 if __name__ == "__main__":
218 #import logging
219 #logging.basicConfig(level=logging.DEBUG)

Subscribers

People subscribed via source and target branches