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

Subscribers

People subscribed via source and target branches