Merge lp:~robru/friends/refresh-fixup into lp:friends

Proposed by Robert Bruce Park
Status: Merged
Merge reported by: Robert Bruce Park
Merged at revision: not available
Proposed branch: lp:~robru/friends/refresh-fixup
Merge into: lp:friends
Diff against target: 833 lines (+160/-139)
24 files modified
friends-service.sh (+1/-1)
friends/main.py (+4/-2)
friends/protocols/facebook.py (+1/-1)
friends/protocols/flickr.py (+1/-1)
friends/protocols/foursquare.py (+1/-1)
friends/protocols/twitter.py (+6/-7)
friends/service/connection.py (+1/-1)
friends/service/dispatcher.py (+2/-1)
friends/service/shortener.py (+1/-1)
friends/testing/mocks.py (+3/-3)
friends/testing/service.py (+1/-1)
friends/tests/test_authentication.py (+15/-9)
friends/tests/test_download.py (+14/-8)
friends/tests/test_facebook.py (+2/-0)
friends/tests/test_flickr.py (+16/-2)
friends/tests/test_logging.py (+16/-37)
friends/utils/account.py (+1/-1)
friends/utils/authentication.py (+15/-15)
friends/utils/avatar.py (+1/-1)
friends/utils/base.py (+8/-3)
friends/utils/download.py (+4/-11)
friends/utils/logging.py (+24/-11)
friends/utils/menus.py (+1/-1)
tools/debug_live.py (+21/-20)
To merge this branch: bzr merge lp:~robru/friends/refresh-fixup
Reviewer Review Type Date Requested Status
Robert Bruce Park Approve
Review via email: mp+130456@code.launchpad.net

Commit message

Workaround for Authentication.login hanging, which was also causing Dispatcher.Refresh to hang thanks to it's use of thread.join(). Authentication.login was using an extraneous GObject MainLoop for the purposes of blocking it's thread synchronously until an async callback got called, but that was a dain bramaged approach because not only does it interfere with the primary mainloop, but Signon.AuthSession.process seems to have a bug where the callback often won't be called, causing the mainloop to block forever.

I've replaced this mainloop with a while loop that calls time.sleep() a limited number of times. The advantage here is that we have direct control over how many times time.sleep() gets called and never have to worry about waiting for a callback that might never come to unblock us.

This commit also makes sweeping changes to the logging infrastructure, which were necessary to track down this bug, and generally beneficial, so they're being included rather than reverted. Logging is now more verbose, and makes extensive use of the "logging.getLogger(__name__)" idiom for finer-grained data on which log is coming from where. debug_live.py also received an overhaul in order to better accomodate testing.

Description of the change

This branch fixes one of the major causes of dispatcher.Refresh hanging, in particular with Authentication.login, in the event that the server did not return any authentication data, it would just hang forever rather than returning properly. This branch converts the incorrect usage of a seconday GObject MainLoop into a much simpler threading.Lock, which serves the same purpose, doesn't interfere with the primary MainLoop, and allows to code to go on to raise existing exceptions when no authentication data is received.

I'm afraid there may be more conditions in which dispatcher.Refresh will hang, but they've been much more intermittent and difficult to pinpoint thus far. But this should still be merged as-is because it's an important fix to a really stupid part of old gwibber code that somehow managed to survive into the friends-service transition.

To post a comment you must log in.
Revision history for this message
Robert Bruce Park (robru) wrote :

This branch also initiates a dispatcher.Refresh on friends-service launch, so that we don't have to wait 15 minutes for the first batch of data to be downloaded ;-)

lp:~robru/friends/refresh-fixup updated
14. By Robert Bruce Park

Drop extraneous instance attribute from Authentication class.

This commit also does some minor cleanups, and introduces some new
debugging statements that may prove helpful in tracking down any
future hangs we discover.

Authentication._authenticating turned out to be redundant, I've
dropped it in favor of testing for the presence of self._reply
directly. Authentication.login sees that self._reply isn't populated
yet, so it blocks, and then the callback populates that value and
stops the lock, allowing login() to return that value.

Revision history for this message
Robert Bruce Park (robru) wrote :

Ken, I've added a couple log.debug statements at the beginning and ending of our Thread.run subclass, so that now it should be really obvious to us in the debugging output when threads are starting and stopping. I still can no longer reproduce any more hangs in dispatcher.Refresh, I've let it run for quite a while with many iterations and no issues.

Please merge and then we'll just have to keep an eye out for any hangs if they return in the future.

lp:~robru/friends/refresh-fixup updated
15. By Robert Bruce Park

Set self._reply sooner, and unconditionally.

16. By Robert Bruce Park

Test suite fixups ;-)

17. By Robert Bruce Park

Replace threading.Lock() with time.sleep()

Yes, yes, I know time.sleep() is ugly, but this is looking like the
only way I can get this working quite right. The previous attempts
using threading.Lock and GObject.MainLoops both had very subtle hangs
that basically screwed up everything.

18. By Robert Bruce Park

Make logging *much* more explicit about what is being logged from where.

19. By Robert Bruce Park

Overhaul debug_live.py to make it compatible with Authentication changes.

20. By Robert Bruce Park

Update tests to no longer expect HTTPError to be raised.

This brings us much more into line with the Soup way of doing things,
rather than trying to force urllib-like behavior on top of Soup.

21. By Robert Bruce Park

Resurrect logging from friends-service.sh

22. By Robert Bruce Park

Overhaul a lot of the logging logic.

Logging now works in both friends-service and in debug_live. I found a
workaround for that duplicate-records bug. Tests have been updated.

23. By Robert Bruce Park

Fix the last of the test failures.

24. By Robert Bruce Park

Greatly simplify the logging logic. Was overthinking it before...

25. By Robert Bruce Park

Simplify debug_live.py

26. By Robert Bruce Park

debug_live.py doesn't need to be on a loop anymore.

The looping of it was primarily useful before we had the dispatcher
actually running. Now that we have the dispatcher, debug_live is more
just for firing single messages.

Revision history for this message
Robert Bruce Park (robru) wrote :

All tests pass, friends-service ran all overnight with no hangs, all good!

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'friends-service.sh'
--- friends-service.sh 2012-10-18 17:48:42 +0000
+++ friends-service.sh 2012-10-20 00:36:21 +0000
@@ -1,3 +1,3 @@
1#! /bin/sh1#! /bin/sh
22
3PYTHONPATH=. python3 -m friends.main -do3PYTHONPATH=. python3 -m friends.main --debug --console
44
=== modified file 'friends/main.py'
--- friends/main.py 2012-10-15 18:56:26 +0000
+++ friends/main.py 2012-10-20 00:36:21 +0000
@@ -26,7 +26,9 @@
26import logging26import logging
2727
28from dbus.mainloop.glib import DBusGMainLoop28from dbus.mainloop.glib import DBusGMainLoop
29from gi.repository import Gio, GLib29from gi.repository import Gio, GLib, GObject
30
31GObject.threads_init(None)
3032
31from friends.service.connection import ConnectionMonitor33from friends.service.connection import ConnectionMonitor
32from friends.service.dispatcher import Dispatcher34from friends.service.dispatcher import Dispatcher
@@ -56,7 +58,7 @@
56 gsettings = Gio.Settings.new('org.gwibber.preferences')58 gsettings = Gio.Settings.new('org.gwibber.preferences')
57 initialize(console=args.console,59 initialize(console=args.console,
58 debug=args.debug or gsettings.get_boolean('debug'))60 debug=args.debug or gsettings.get_boolean('debug'))
59 log = logging.getLogger('friends.service')61 log = logging.getLogger(__name__)
60 log.info('Friends backend service starting')62 log.info('Friends backend service starting')
6163
62 # Set up the DBus main loop.64 # Set up the DBus main loop.
6365
=== modified file 'friends/protocols/facebook.py'
--- friends/protocols/facebook.py 2012-10-16 04:10:50 +0000
+++ friends/protocols/facebook.py 2012-10-20 00:36:21 +0000
@@ -37,7 +37,7 @@
37ME_URL = API_BASE.format(id='me')37ME_URL = API_BASE.format(id='me')
3838
3939
40log = logging.getLogger('friends.service')40log = logging.getLogger(__name__)
4141
4242
43class Facebook(Base):43class Facebook(Base):
4444
=== modified file 'friends/protocols/flickr.py'
--- friends/protocols/flickr.py 2012-10-13 01:27:15 +0000
+++ friends/protocols/flickr.py 2012-10-20 00:36:21 +0000
@@ -28,7 +28,7 @@
28from friends.utils.time import iso8601utc, parsetime28from friends.utils.time import iso8601utc, parsetime
2929
3030
31log = logging.getLogger('friends.service')31log = logging.getLogger(__name__)
3232
3333
34# This is contact information for the Flickr REST service.34# This is contact information for the Flickr REST service.
3535
=== modified file 'friends/protocols/foursquare.py'
--- friends/protocols/foursquare.py 2012-10-13 01:27:15 +0000
+++ friends/protocols/foursquare.py 2012-10-20 00:36:21 +0000
@@ -28,7 +28,7 @@
28from friends.utils.time import iso8601utc28from friends.utils.time import iso8601utc
2929
3030
31log = logging.getLogger('friends.service')31log = logging.getLogger(__name__)
3232
3333
34# The '&v=YYYYMMDD' defines the date that the API was last confirmed to be34# The '&v=YYYYMMDD' defines the date that the API was last confirmed to be
3535
=== modified file 'friends/protocols/twitter.py'
--- friends/protocols/twitter.py 2012-10-16 04:10:50 +0000
+++ friends/protocols/twitter.py 2012-10-20 00:36:21 +0000
@@ -34,17 +34,16 @@
34from friends.utils.time import parsetime, iso8601utc34from friends.utils.time import parsetime, iso8601utc
3535
3636
37log = logging.getLogger('friends.service')37log = logging.getLogger(__name__)
3838
3939
40# https://dev.twitter.com/docs/api/1.140# https://dev.twitter.com/docs/api/1.1
41class Twitter(Base):41class Twitter(Base):
42 # StatusNet claims to mimick the Twitter API very closely (so closely to42 # Identi.ca's API mimicks Twitter's to such a high degree that it
43 # the point that they refer you to the Twitter API reference docs as a43 # is implemented just as a subclass of this, hence we need these
44 # starting point for learning about their API). So these prefixes are44 # constants defined as instance attributes, so that the Identica
45 # defined here as class attributes instead of the usual module globals, in45 # class can override them. If you make any changes to this class
46 # the hopes that the StatusNet class will be able to subclass Twitter and46 # you must confirm that your changes do not break Identi.ca!
47 # change only the URLs, with minimal other changes, and magically work.
48 _api_base = 'https://api.twitter.com/1.1/{endpoint}.json'47 _api_base = 'https://api.twitter.com/1.1/{endpoint}.json'
4948
50 _timeline = _api_base.format(endpoint='statuses/{}_timeline')49 _timeline = _api_base.format(endpoint='statuses/{}_timeline')
5150
=== modified file 'friends/service/connection.py'
--- friends/service/connection.py 2012-10-13 01:27:15 +0000
+++ friends/service/connection.py 2012-10-20 00:36:21 +0000
@@ -50,7 +50,7 @@
50 DISCONNECTED = 450 DISCONNECTED = 4
5151
5252
53log = logging.getLogger('friends.service')53log = logging.getLogger(__name__)
54_State = None54_State = None
5555
5656
5757
=== modified file 'friends/service/dispatcher.py'
--- friends/service/dispatcher.py 2012-10-13 01:27:15 +0000
+++ friends/service/dispatcher.py 2012-10-20 00:36:21 +0000
@@ -34,7 +34,7 @@
34from friends.utils.manager import protocol_manager34from friends.utils.manager import protocol_manager
35from friends.utils.signaler import signaler35from friends.utils.signaler import signaler
3636
37log = logging.getLogger('friends.service')37log = logging.getLogger(__name__)
3838
3939
40class Dispatcher(dbus.service.Object):40class Dispatcher(dbus.service.Object):
@@ -53,6 +53,7 @@
53 signaler.add_signal('ConnectionOnline', self._on_connection_online)53 signaler.add_signal('ConnectionOnline', self._on_connection_online)
54 signaler.add_signal('ConnectionOffline', self._on_connection_offline)54 signaler.add_signal('ConnectionOffline', self._on_connection_offline)
55 self._on_connection_online()55 self._on_connection_online()
56 self.Refresh()
5657
57 def _on_connection_online(self):58 def _on_connection_online(self):
58 if not self._timer_id:59 if not self._timer_id:
5960
=== modified file 'friends/service/shortener.py'
--- friends/service/shortener.py 2012-10-13 01:27:15 +0000
+++ friends/service/shortener.py 2012-10-20 00:36:21 +0000
@@ -27,7 +27,7 @@
2727
28from friends.shorteners import lookup28from friends.shorteners import lookup
2929
30log = logging.getLogger('friends.service')30log = logging.getLogger(__name__)
3131
3232
33class URLShorten(dbus.service.Object):33class URLShorten(dbus.service.Object):
3434
=== modified file 'friends/testing/mocks.py'
--- friends/testing/mocks.py 2012-10-17 18:55:49 +0000
+++ friends/testing/mocks.py 2012-10-20 00:36:21 +0000
@@ -168,7 +168,7 @@
168 """168 """
169 def __init__(self, *modules):169 def __init__(self, *modules):
170 self._queue = Queue()170 self._queue = Queue()
171 self._log = logging.getLogger('friends.test')171 self._log = logging.getLogger(__name__)
172 handler = QueueHandler(self._queue)172 handler = QueueHandler(self._queue)
173 formatter = logging.Formatter(LOG_FORMAT, style='{')173 formatter = logging.Formatter(LOG_FORMAT, style='{')
174 handler.setFormatter(formatter)174 handler.setFormatter(formatter)
@@ -205,8 +205,8 @@
205 self.empty()205 self.empty()
206 for patcher in self._patchers:206 for patcher in self._patchers:
207 patcher.stop()207 patcher.stop()
208 # Get rid of the friends.test logger.208 # Get rid of the mock logger.
209 del logging.Logger.manager.loggerDict['friends.test']209 del logging.Logger.manager.loggerDict[__name__]
210210
211 def empty(self, trim=True):211 def empty(self, trim=True):
212 """Return all the log messages written to this log.212 """Return all the log messages written to this log.
213213
=== modified file 'friends/testing/service.py'
--- friends/testing/service.py 2012-10-13 01:27:15 +0000
+++ friends/testing/service.py 2012-10-20 00:36:21 +0000
@@ -28,7 +28,7 @@
28from friends.utils.signaler import signaler28from friends.utils.signaler import signaler
2929
3030
31log = logging.getLogger('friends.service')31log = logging.getLogger(__name__)
3232
3333
34class TestService(dbus.service.Object):34class TestService(dbus.service.Object):
3535
=== modified file 'friends/tests/test_authentication.py'
--- friends/tests/test_authentication.py 2012-10-13 01:27:15 +0000
+++ friends/tests/test_authentication.py 2012-10-20 00:36:21 +0000
@@ -62,6 +62,11 @@
62 def error(self, message, *args):62 def error(self, message, *args):
63 self.error_messages.append(message.format(*args))63 self.error_messages.append(message.format(*args))
6464
65 reset = __init__
66
67
68logger = Logger()
69
6570
66class TestAuthentication(unittest.TestCase):71class TestAuthentication(unittest.TestCase):
67 """Test authentication."""72 """Test authentication."""
@@ -72,28 +77,29 @@
72 self.account.auth.method = 'some method'77 self.account.auth.method = 'some method'
73 self.account.auth.parameters = 'change me'78 self.account.auth.parameters = 'change me'
74 self.account.auth.mechanism = ['whatever']79 self.account.auth.mechanism = ['whatever']
75 self.logger = Logger()80 logger.reset()
7681
82 @mock.patch('friends.utils.authentication.log', logger)
77 @mock.patch('friends.utils.authentication.Signon', FakeSignon)83 @mock.patch('friends.utils.authentication.Signon', FakeSignon)
78 def test_successful_login(self):84 def test_successful_login(self):
79 # Prevent an error in the callback.85 # Prevent an error in the callback.
80 self.account.auth.parameters = False86 self.account.auth.parameters = False
81 authenticator = Authentication(self.account, self.logger)87 authenticator = Authentication(self.account)
82 reply = authenticator.login()88 reply = authenticator.login()
83 self.assertEqual(reply, 'auth reply')89 self.assertEqual(reply, 'auth reply')
84 self.assertEqual(self.logger.debug_messages,90 self.assertEqual(logger.debug_messages, ['Login completed'])
85 ['Login completed'])91 self.assertEqual(logger.error_messages, [])
86 self.assertEqual(self.logger.error_messages, [])
8792
93 @mock.patch('friends.utils.authentication.log', logger)
88 @mock.patch('friends.utils.authentication.Signon', FakeSignon)94 @mock.patch('friends.utils.authentication.Signon', FakeSignon)
89 def test_failed_login(self):95 def test_failed_login(self):
90 # Trigger an error in the callback.96 # Trigger an error in the callback.
91 class Error:97 class Error:
92 message = 'who are you?'98 message = 'who are you?'
93 self.account.auth.parameters = Error99 self.account.auth.parameters = Error
94 authenticator = Authentication(self.account, self.logger)100 authenticator = Authentication(self.account)
95 reply = authenticator.login()101 reply = authenticator.login()
96 self.assertIsNone(reply)102 self.assertEqual(reply, 'auth reply')
97 self.assertEqual(self.logger.debug_messages, ['Login completed'])103 self.assertEqual(logger.debug_messages, ['Login completed'])
98 self.assertEqual(self.logger.error_messages,104 self.assertEqual(logger.error_messages,
99 ['Got authentication error: who are you?'])105 ['Got authentication error: who are you?'])
100106
=== modified file 'friends/tests/test_download.py'
--- friends/tests/test_download.py 2012-10-13 01:27:15 +0000
+++ friends/tests/test_download.py 2012-10-20 00:36:21 +0000
@@ -33,7 +33,7 @@
33from wsgiref.simple_server import WSGIRequestHandler, make_server33from wsgiref.simple_server import WSGIRequestHandler, make_server
34from wsgiref.util import setup_testing_defaults34from wsgiref.util import setup_testing_defaults
3535
36from friends.testing.mocks import FakeSoupMessage36from friends.testing.mocks import FakeSoupMessage, LogMock
37from friends.utils.download import Downloader, get_json37from friends.utils.download import Downloader, get_json
3838
39try:39try:
@@ -123,6 +123,12 @@
123class TestDownloader(unittest.TestCase):123class TestDownloader(unittest.TestCase):
124 """Test the downloading utilities."""124 """Test the downloading utilities."""
125125
126 def setUp(self):
127 self.log_mock = LogMock('friends.utils.download')
128
129 def tearDown(self):
130 self.log_mock.stop()
131
126 @classmethod132 @classmethod
127 def setUpClass(cls):133 def setUpClass(cls):
128 cls.server = make_server('', 9180, _app, handler_class=_SilentHandler)134 cls.server = make_server('', 9180, _app, handler_class=_SilentHandler)
@@ -231,17 +237,17 @@
231237
232 def test_unauthorized(self):238 def test_unauthorized(self):
233 # Test a URL that requires authorization.239 # Test a URL that requires authorization.
234 with self.assertRaises(HTTPError) as cm:240 Downloader('http://localhost:9180/auth').get_string()
235 Downloader('http://localhost:9180/auth').get_string()241 self.assertEqual(self.log_mock.empty(),
236 self.assertEqual(cm.exception.code, 401)242 'http://localhost:9180/auth: 401 Unauthorized\n')
237243
238 def test_bad_authorization(self):244 def test_bad_authorization(self):
239 # Test a URL that requires authorization, but doesn't have the correct245 # Test a URL that requires authorization, but doesn't have the correct
240 # username or password.246 # username or password.
241 with self.assertRaises(HTTPError) as cm:247 Downloader('http://localhost:9180/auth',
242 Downloader('http://localhost:9180/auth',248 username='bob', password='wrong').get_string()
243 username='bob', password='wrong').get_string()249 self.assertEqual(self.log_mock.empty(),
244 self.assertEqual(cm.exception.code, 401)250 'http://localhost:9180/auth: 401 Unauthorized\n')
245251
246 def test_authorized(self):252 def test_authorized(self):
247 # Test a URL that requires authorization, and has the proper253 # Test a URL that requires authorization, and has the proper
248254
=== modified file 'friends/tests/test_facebook.py'
--- friends/tests/test_facebook.py 2012-10-17 18:55:49 +0000
+++ friends/tests/test_facebook.py 2012-10-20 00:36:21 +0000
@@ -102,9 +102,11 @@
102 self.protocol('receive')102 self.protocol('receive')
103 contents = log_mock.empty(trim=False)103 contents = log_mock.empty(trim=False)
104 self.assertEqual(contents, """\104 self.assertEqual(contents, """\
105Facebook.receive is starting in a new thread.
105Logging in to Facebook106Logging in to Facebook
106Facebook UID: None107Facebook UID: None
107Facebook error (190 OAuthException): Bad access token108Facebook error (190 OAuthException): Bad access token
109Facebook.receive has completed, thread exiting.
108""")110""")
109111
110 @mock.patch('friends.utils.download.Soup.Message',112 @mock.patch('friends.utils.download.Soup.Message',
111113
=== modified file 'friends/tests/test_flickr.py'
--- friends/tests/test_flickr.py 2012-10-17 18:55:49 +0000
+++ friends/tests/test_flickr.py 2012-10-20 00:36:21 +0000
@@ -71,12 +71,14 @@
71 with mock.patch.object(self.protocol, '_get_nsid', return_value=None):71 with mock.patch.object(self.protocol, '_get_nsid', return_value=None):
72 self.protocol('receive')72 self.protocol('receive')
73 self.assertEqual(self.log_mock.empty(), """\73 self.assertEqual(self.log_mock.empty(), """\
74Flickr.receive is starting in a new thread.
74Flickr: No NSID available75Flickr: No NSID available
75Friends operation exception:76Friends operation exception:
76 Traceback (most recent call last):77 Traceback (most recent call last):
77 ...78 ...
78friends.errors.AuthorizationError:\79friends.errors.AuthorizationError:\
79 No Flickr user id available (account: faker/than fake)80 No Flickr user id available (account: faker/than fake)
81Flickr.receive has completed, thread exiting.
80""")82""")
8183
82 @mock.patch('friends.utils.download.Soup.Message',84 @mock.patch('friends.utils.download.Soup.Message',
@@ -90,7 +92,10 @@
90 # AuthorizationError.92 # AuthorizationError.
91 self.protocol('receive')93 self.protocol('receive')
92 # No error messages.94 # No error messages.
93 self.assertEqual(self.log_mock.empty(), '')95 self.assertEqual(self.log_mock.empty(), """\
96Flickr.receive is starting in a new thread.
97Flickr.receive has completed, thread exiting.
98""")
94 # But also no photos.99 # But also no photos.
95 self.assertEqual(TestModel.get_n_rows(), 0)100 self.assertEqual(TestModel.get_n_rows(), 0)
96101
@@ -108,11 +113,13 @@
108 side_effect=side_effect):113 side_effect=side_effect):
109 self.protocol('receive')114 self.protocol('receive')
110 self.assertEqual(self.log_mock.empty(), """\115 self.assertEqual(self.log_mock.empty(), """\
116Flickr.receive is starting in a new thread.
111Friends operation exception:117Friends operation exception:
112 Traceback (most recent call last):118 Traceback (most recent call last):
113 ...119 ...
114friends.errors.AuthorizationError:\120friends.errors.AuthorizationError:\
115 No Flickr user id available (account: faker/than fake)121 No Flickr user id available (account: faker/than fake)
122Flickr.receive has completed, thread exiting.
116""")123""")
117124
118 @mock.patch('friends.utils.download.Soup.Message',125 @mock.patch('friends.utils.download.Soup.Message',
@@ -129,7 +136,10 @@
129 side_effect=side_effect):136 side_effect=side_effect):
130 self.protocol('receive')137 self.protocol('receive')
131 # No error message.138 # No error message.
132 self.assertEqual(self.log_mock.empty(), '')139 self.assertEqual(self.log_mock.empty(), """\
140Flickr.receive is starting in a new thread.
141Flickr.receive has completed, thread exiting.
142""")
133 # But also no photos.143 # But also no photos.
134 self.assertEqual(TestModel.get_n_rows(), 0)144 self.assertEqual(TestModel.get_n_rows(), 0)
135145
@@ -145,6 +155,7 @@
145 # AccessToken, but this fails.155 # AccessToken, but this fails.
146 self.protocol('receive')156 self.protocol('receive')
147 self.assertEqual(self.log_mock.empty(), """\157 self.assertEqual(self.log_mock.empty(), """\
158Flickr.receive is starting in a new thread.
148Logging in to Flickr159Logging in to Flickr
149No AccessToken in Flickr session:\160No AccessToken in Flickr session:\
150 {'TokenSecret': 'abc', 'username': 'Bob Dobbs', 'user_nsid': 'bob'}161 {'TokenSecret': 'abc', 'username': 'Bob Dobbs', 'user_nsid': 'bob'}
@@ -154,6 +165,7 @@
154 ...165 ...
155friends.errors.AuthorizationError:\166friends.errors.AuthorizationError:\
156 No Flickr user id available (account: faker/than fake)167 No Flickr user id available (account: faker/than fake)
168Flickr.receive has completed, thread exiting.
157""")169""")
158170
159 @mock.patch('friends.utils.download.Soup.Message',171 @mock.patch('friends.utils.download.Soup.Message',
@@ -166,6 +178,7 @@
166 # AccessToken, but this fails.178 # AccessToken, but this fails.
167 self.protocol('receive')179 self.protocol('receive')
168 self.assertEqual(self.log_mock.empty(), """\180 self.assertEqual(self.log_mock.empty(), """\
181Flickr.receive is starting in a new thread.
169Logging in to Flickr182Logging in to Flickr
170No Flickr authentication results received.183No Flickr authentication results received.
171Flickr: No NSID available184Flickr: No NSID available
@@ -174,6 +187,7 @@
174 ...187 ...
175friends.errors.AuthorizationError:\188friends.errors.AuthorizationError:\
176 No Flickr user id available (account: faker/than fake)189 No Flickr user id available (account: faker/than fake)
190Flickr.receive has completed, thread exiting.
177""")191""")
178192
179 @mock.patch('friends.utils.download.Soup.Message',193 @mock.patch('friends.utils.download.Soup.Message',
180194
=== modified file 'friends/tests/test_logging.py'
--- friends/tests/test_logging.py 2012-10-13 01:27:15 +0000
+++ friends/tests/test_logging.py 2012-10-20 00:36:21 +0000
@@ -35,30 +35,23 @@
35 def setUp(self):35 def setUp(self):
36 # Preserve the original logger, but restore the global logging system36 # Preserve the original logger, but restore the global logging system
37 # to pre-initialized state.37 # to pre-initialized state.
38 self._logger = logging.getLogger('friends.service')38 self._loggers = logging.Logger.manager.loggerDict.copy()
39 del logging.Logger.manager.loggerDict['friends.service']39 logging.Logger.manager.loggerDict.clear()
4040
41 def tearDown(self):41 def tearDown(self):
42 # Restore the original logger.42 # Restore the original loggers.
43 logging.Logger.manager.loggerDict['friends.service'] = self._logger43 logging.Logger.manager.loggerDict.update(self._loggers)
4444
45 def test_logger_has_filehandler(self):45 def test_logger_has_filehandler(self):
46 initialize()46 initialize()
47 # The top level 'friends.service' logger should be available.47 # The top level logger should be available.
48 log = logging.getLogger('friends.service')48 log = logging.getLogger()
49 # Try to find the file opened by the default file handler.49 # Try to find the file opened by the default file handler.
50 filenames = []50 filenames = []
51 for handler in log.handlers:51 for handler in log.handlers:
52 if hasattr(handler, 'baseFilename'):52 if hasattr(handler, 'baseFilename'):
53 filenames.append(handler.baseFilename)53 filenames.append(handler.baseFilename)
54 self.assertEqual(len(filenames), 1)54 self.assertGreater(len(filenames), 0)
55
56 def _get_log_filename(self, log):
57 filenames = []
58 for handler in log.handlers:
59 if hasattr(handler, 'baseFilename'):
60 filenames.append(handler.baseFilename)
61 return filenames
6255
63 def test_logger_message(self):56 def test_logger_message(self):
64 # Write an error message to the log and test that it shows up.57 # Write an error message to the log and test that it shows up.
@@ -67,11 +60,7 @@
67 logfile = os.path.join(tempdir, 'friends-test.log')60 logfile = os.path.join(tempdir, 'friends-test.log')
68 initialize(filename=logfile)61 initialize(filename=logfile)
69 # Get another handle on the log file.62 # Get another handle on the log file.
70 log = logging.getLogger('friends.service')63 log = logging.getLogger()
71 # Try to find the file opened by the default file handler.
72 filenames = self._get_log_filename(log)
73 self.assertEqual(len(filenames), 1)
74 logfile = filenames[0]
75 self.assertEqual(os.stat(logfile).st_size, 0)64 self.assertEqual(os.stat(logfile).st_size, 0)
76 # Log messages at INFO or higher should be written.65 # Log messages at INFO or higher should be written.
77 log.info('friends at your service')66 log.info('friends at your service')
@@ -81,12 +70,11 @@
81 contents = fp.read()70 contents = fp.read()
82 lines = contents.splitlines()71 lines = contents.splitlines()
83 self.assertEqual(len(lines), 1)72 self.assertEqual(len(lines), 1)
84 # The log message will have a variable timestamp at the front, so73 # The log message will have a variable timestamp, so ignore
85 # ignore that, but check everything else.74 # that, but check everything else.
86 self.assertRegex(75 self.assertRegex(
87 lines[0],76 lines[0],
88 r'friends.service\s+MainThread\s+:\s+INFO\s+-\s+'77 r'INFO\s+MainThread.*root\s+friends at your service')
89 r'friends at your service')
9078
91 def test_console_logger(self):79 def test_console_logger(self):
92 # The logger can support an optional console logger.80 # The logger can support an optional console logger.
@@ -94,10 +82,10 @@
94 self.addCleanup(shutil.rmtree, tempdir)82 self.addCleanup(shutil.rmtree, tempdir)
95 logfile = os.path.join(tempdir, 'friends-test.log')83 logfile = os.path.join(tempdir, 'friends-test.log')
96 initialize(console=True, filename=logfile)84 initialize(console=True, filename=logfile)
97 log = logging.getLogger('friends.service')85 log = logging.getLogger()
98 # Can we do better than testing that there are now two handlers for86 # Can we do better than testing that there are now two handlers for
99 # the logger?87 # the logger?
100 self.assertEqual(2, sum(1 for handler in log.handlers))88 self.assertEqual(3, sum(1 for handler in log.handlers))
10189
102 def test_default_logger_level(self):90 def test_default_logger_level(self):
103 # By default, the logger level is INFO.91 # By default, the logger level is INFO.
@@ -106,11 +94,7 @@
106 logfile = os.path.join(tempdir, 'friends-test.log')94 logfile = os.path.join(tempdir, 'friends-test.log')
107 initialize(filename=logfile)95 initialize(filename=logfile)
108 # Get another handle on the log file.96 # Get another handle on the log file.
109 log = logging.getLogger('friends.service')97 log = logging.getLogger()
110 # Try to find the file opened by the default file handler.
111 filenames = self._get_log_filename(log)
112 self.assertEqual(len(filenames), 1)
113 logfile = filenames[0]
114 # By default, debug messages won't get written since they are less98 # By default, debug messages won't get written since they are less
115 # severe than INFO.99 # severe than INFO.
116 log.debug('friends is ready')100 log.debug('friends is ready')
@@ -123,11 +107,7 @@
123 logfile = os.path.join(tempdir, 'friends-test.log')107 logfile = os.path.join(tempdir, 'friends-test.log')
124 initialize(filename=logfile, debug=True)108 initialize(filename=logfile, debug=True)
125 # Get another handle on the log file.109 # Get another handle on the log file.
126 log = logging.getLogger('friends.service')110 log = logging.getLogger()
127 # Try to find the file opened by the default file handler.
128 filenames = self._get_log_filename(log)
129 self.assertEqual(len(filenames), 1)
130 logfile = filenames[0]
131 log.debug('friends is ready')111 log.debug('friends is ready')
132 self.assertGreater(os.stat(logfile).st_size, 0)112 self.assertGreater(os.stat(logfile).st_size, 0)
133 # Read the contents, which should be just one line of output.113 # Read the contents, which should be just one line of output.
@@ -139,5 +119,4 @@
139 # ignore that, but check everything else.119 # ignore that, but check everything else.
140 self.assertRegex(120 self.assertRegex(
141 lines[0],121 lines[0],
142 r'friends.service\s+MainThread\s+:\s+DEBUG\s+-\s+'122 r'DEBUG\s+MainThread.*root\s+friends is ready')
143 r'friends is ready')
144123
=== modified file 'friends/utils/account.py'
--- friends/utils/account.py 2012-10-18 18:41:58 +0000
+++ friends/utils/account.py 2012-10-20 00:36:21 +0000
@@ -31,7 +31,7 @@
31from friends.utils.model import COLUMN_INDICES, Model31from friends.utils.model import COLUMN_INDICES, Model
3232
3333
34log = logging.getLogger('friends.service')34log = logging.getLogger(__name__)
3535
3636
37class AccountManager:37class AccountManager:
3838
=== modified file 'friends/utils/authentication.py'
--- friends/utils/authentication.py 2012-10-13 01:27:15 +0000
+++ friends/utils/authentication.py 2012-10-20 00:36:21 +0000
@@ -21,37 +21,37 @@
2121
2222
23import logging23import logging
24import time
2425
25from gi.repository import GObject, Signon26from gi.repository import GObject, Signon
26GObject.threads_init(None)27GObject.threads_init(None)
2728
2829
30log = logging.getLogger(__name__)
31
32
29class Authentication:33class Authentication:
30 def __init__(self, account, log=None):34 def __init__(self, account):
31 self.account = account35 self.account = account
32 self.log = (logging.getLogger('friends.service')
33 if log is None
34 else log)
35 self._reply = None36 self._reply = None
36 self._authenticating = False
3737
38 def login(self):38 def login(self):
39 auth = self.account.auth39 auth = self.account.auth
40 self.auth_session = Signon.AuthSession.new(auth.id, auth.method)40 self.auth_session = Signon.AuthSession.new(auth.id, auth.method)
41 self._authenticating = True
42 self._loop = GObject.MainLoop()
43 self.auth_session.process(41 self.auth_session.process(
44 auth.parameters, auth.mechanism,42 auth.parameters, auth.mechanism,
45 self._login_cb, None)43 self._login_cb, None)
46 if self._authenticating:44 timeout = 30
47 self._loop.run()45 while self._reply is None and timeout > 0:
46 # We're building a synchronous API on top of an inherently
47 # async library, so we need to block this thread until the
48 # callback gets called to give us the response to return.
49 time.sleep(0.5)
50 timeout -= 1
48 return self._reply51 return self._reply
4952
50 def _login_cb(self, session, reply, error, user_data):53 def _login_cb(self, session, reply, error, user_data):
51 self._authenticating = False54 self._reply = reply
52 if error:55 if error:
53 self.log.error('Got authentication error: {}', error.message)56 log.error('Got authentication error: {}'.format(error.message))
54 else:57 log.debug('Login completed')
55 self._reply = reply
56 self.log.debug('Login completed')
57 self._loop.quit()
5858
=== modified file 'friends/utils/avatar.py'
--- friends/utils/avatar.py 2012-10-13 01:27:15 +0000
+++ friends/utils/avatar.py 2012-10-20 00:36:21 +0000
@@ -31,7 +31,7 @@
31from friends.utils.download import Downloader31from friends.utils.download import Downloader
3232
3333
34log = logging.getLogger('friends.avatars')34log = logging.getLogger(__name__)
35CACHE_DIR = os.path.realpath(os.path.join(35CACHE_DIR = os.path.realpath(os.path.join(
36 GLib.get_user_cache_dir(), 'friends', 'avatars'))36 GLib.get_user_cache_dir(), 'friends', 'avatars'))
37CACHE_AGE = timedelta(weeks=4)37CACHE_AGE = timedelta(weeks=4)
3838
=== modified file 'friends/utils/base.py'
--- friends/utils/base.py 2012-10-16 18:58:49 +0000
+++ friends/utils/base.py 2012-10-20 00:36:21 +0000
@@ -52,7 +52,7 @@
52_publish_lock = threading.Lock()52_publish_lock = threading.Lock()
5353
5454
55log = logging.getLogger('friends.service')55log = logging.getLogger(__name__)
5656
5757
58def feature(method):58def feature(method):
@@ -105,9 +105,10 @@
105class _OperationThread(threading.Thread):105class _OperationThread(threading.Thread):
106 """Catch, log, and swallow all exceptions in the sub-thread."""106 """Catch, log, and swallow all exceptions in the sub-thread."""
107107
108 def __init__(self, barrier, *args, **kws):108 def __init__(self, barrier, *args, identifier=None, **kws):
109 # The barrier will only be provided when the system is under test.109 # The barrier will only be provided when the system is under test.
110 self._barrier = barrier110 self._barrier = barrier
111 self._id = identifier
111 super().__init__(*args, **kws)112 super().__init__(*args, **kws)
112113
113 # Always run these as daemon threads, so they don't block the main thread,114 # Always run these as daemon threads, so they don't block the main thread,
@@ -115,10 +116,12 @@
115 daemon = True116 daemon = True
116117
117 def run(self):118 def run(self):
119 log.debug('{} is starting in a new thread.'.format(self._id))
118 try:120 try:
119 super().run()121 super().run()
120 except Exception:122 except Exception:
121 log.exception('Friends operation exception:\n')123 log.exception('Friends operation exception:\n')
124 log.debug('{} has completed, thread exiting.'.format(self._id))
122 # If the system is under test, indicate that we've reached the125 # If the system is under test, indicate that we've reached the
123 # barrier, so that the main thread, i.e. the test thread waiting for126 # barrier, so that the main thread, i.e. the test thread waiting for
124 # the results, can then proceed.127 # the results, can then proceed.
@@ -160,6 +163,8 @@
160 # thread to assert the results of the sub-thread.163 # thread to assert the results of the sub-thread.
161 barrier = (threading.Barrier(parties=2) if Base._SYNCHRONIZE else None)164 barrier = (threading.Barrier(parties=2) if Base._SYNCHRONIZE else None)
162 _OperationThread(barrier,165 _OperationThread(barrier,
166 identifier='{}.{}'.format(self.__class__.__name__,
167 operation),
163 target=method, args=args, kwargs=kwargs).start()168 target=method, args=args, kwargs=kwargs).start()
164 # When under synchronous testing, wait until the sub-thread completes169 # When under synchronous testing, wait until the sub-thread completes
165 # before returning.170 # before returning.
@@ -299,7 +304,7 @@
299 log.debug('{} to {}'.format(304 log.debug('{} to {}'.format(
300 'Re-authenticating' if old_token else 'Logging in', protocol))305 'Re-authenticating' if old_token else 'Logging in', protocol))
301306
302 result = Authentication(self._account, log).login()307 result = Authentication(self._account).login()
303 if result is None:308 if result is None:
304 log.error(309 log.error(
305 'No {} authentication results received.'.format(protocol))310 'No {} authentication results received.'.format(protocol))
306311
=== modified file 'friends/utils/download.py'
--- friends/utils/download.py 2012-10-18 18:41:54 +0000
+++ friends/utils/download.py 2012-10-20 00:36:21 +0000
@@ -31,7 +31,7 @@
31from urllib.error import HTTPError31from urllib.error import HTTPError
32from urllib.parse import urlencode32from urllib.parse import urlencode
3333
34log = logging.getLogger('friends.service')34log = logging.getLogger(__name__)
3535
3636
37# Global libsoup session instance.37# Global libsoup session instance.
@@ -139,16 +139,9 @@
139 message = self._build_request()139 message = self._build_request()
140 _soup.send_message(message)140 _soup.send_message(message)
141 if message.status_code != 200:141 if message.status_code != 200:
142 # Even though we're using libsoup instead of Python's urllib142 log.error('{}: {} {}'.format(self.url,
143 # libraries, it's helpful for clients to receive an exception when143 message.status_code,
144 # non-success error codes are received. This lets the clients do144 message.reason_phrase))
145 # additional work if they want, and it prevents rate limiting from
146 # occurring.
147 raise HTTPError(self.url,
148 message.status_code,
149 message.reason_phrase,
150 self.headers,
151 None)
152 yield message145 yield message
153 self._rate_limiter.update(message)146 self._rate_limiter.update(message)
154147
155148
=== modified file 'friends/utils/logging.py'
--- friends/utils/logging.py 2012-10-13 01:27:15 +0000
+++ friends/utils/logging.py 2012-10-20 00:36:21 +0000
@@ -19,15 +19,25 @@
19import errno19import errno
20import logging20import logging
21import logging.handlers21import logging.handlers
22import oauthlib.oauth1
2223
23from gi.repository import GLib24from gi.repository import GLib
2425
2526
27# Set a global default of no logging. This is a workaround for a bug
28# where we were getting duplicated log records.
29logging.basicConfig(filename='/dev/null', level=100)
30
31
32# Disable logging in oauthlib because it is very verbose.
33oauthlib.oauth1.rfc5849.logging.debug = lambda *ignore: None
34
35
26LOG_FILENAME = os.path.join(36LOG_FILENAME = os.path.join(
27 os.path.realpath(os.path.abspath(GLib.get_user_cache_dir())),37 os.path.realpath(os.path.abspath(GLib.get_user_cache_dir())),
28 'friends', 'friends.log')38 'friends', 'friends.log')
29LOG_FORMAT = '{asctime} - {name:12} {threadName:14}: {levelname:8} - {message}'39LOG_FORMAT = '{levelname:5} {threadName:10} {asctime} {name:18} {message}'
30CSL_FORMAT = '{name:12} {threadName:12}: {levelname:8} {message}'40CSL_FORMAT = LOG_FORMAT.replace(' {asctime}', '')
3141
3242
33def initialize(console=False, debug=False, filename=None):43def initialize(console=False, debug=False, filename=None):
@@ -48,11 +58,7 @@
48 except OSError as error:58 except OSError as error:
49 if error.errno != errno.EEXIST:59 if error.errno != errno.EEXIST:
50 raise60 raise
51 log = logging.getLogger('friends.service')61
52 if debug:
53 log.setLevel(logging.DEBUG)
54 else:
55 log.setLevel(logging.INFO)
56 # Install a rotating log file handler. XXX There should be a62 # Install a rotating log file handler. XXX There should be a
57 # configuration file rather than hard-coded values.63 # configuration file rather than hard-coded values.
58 text_handler = logging.handlers.RotatingFileHandler(64 text_handler = logging.handlers.RotatingFileHandler(
@@ -60,9 +66,16 @@
60 # Use str.format() style format strings.66 # Use str.format() style format strings.
61 text_formatter = logging.Formatter(LOG_FORMAT, style='{')67 text_formatter = logging.Formatter(LOG_FORMAT, style='{')
62 text_handler.setFormatter(text_formatter)68 text_handler.setFormatter(text_formatter)
63 log.addHandler(text_handler)69
70 console_handler = logging.StreamHandler()
71 console_formatter = logging.Formatter(CSL_FORMAT, style='{')
72 console_handler.setFormatter(console_formatter)
73
74 log = logging.getLogger()
75 if debug:
76 log.setLevel(logging.DEBUG)
77 else:
78 log.setLevel(logging.INFO)
64 if console:79 if console:
65 console_handler = logging.StreamHandler()
66 console_formatter = logging.Formatter(CSL_FORMAT, style='{')
67 console_handler.setFormatter(console_formatter)
68 log.addHandler(console_handler)80 log.addHandler(console_handler)
81 log.addHandler(text_handler)
6982
=== modified file 'friends/utils/menus.py'
--- friends/utils/menus.py 2012-10-13 01:27:15 +0000
+++ friends/utils/menus.py 2012-10-20 00:36:21 +0000
@@ -34,7 +34,7 @@
3434
3535
36DESKTOP_ID = 'friends.desktop'36DESKTOP_ID = 'friends.desktop'
37log = logging.getLogger('friends.service')37log = logging.getLogger(__name__)
3838
3939
40def helper(executable):40def helper(executable):
4141
=== modified file 'tools/debug_live.py'
--- tools/debug_live.py 2012-10-13 01:27:15 +0000
+++ tools/debug_live.py 2012-10-20 00:36:21 +0000
@@ -20,30 +20,29 @@
20"""20"""
2121
22import sys22import sys
23import time
24import logging
2325
24sys.path.insert(0, '.')26sys.path.insert(0, '.')
2527
26from gi.repository import GObject28from gi.repository import GObject
29from friends.utils.logging import initialize
27from friends.utils.account import AccountManager30from friends.utils.account import AccountManager
28from friends.utils.base import Base31from friends.utils.base import Base
29from friends.utils.model import Model32from friends.utils.model import Model
3033
3134# Print all logs for debugging purposes
32# Ensure synchronous operation of Base.__call__() for easier testing.35initialize(debug=True, console=True)
33Base._SYNCHRONIZE = True36
3437
3538log = logging.getLogger('friends.debug_live')
36def refresh(account):39
40
41def row_added(model, itr):
42 row = model.get_row(itr)
43 print(row)
44 log.info('ROWS: {}'.format(len(model)))
37 print()45 print()
38 print('#' * 80)
39 print('Performing "{}" operation!'.format(args[0]))
40 print('#' * 80)
41
42 account.protocol(*args)
43 for row in Model:
44 print([col for col in row])
45 print()
46 print('ROWS: ', len(Model))
4746
4847
49if __name__ == '__main__':48if __name__ == '__main__':
@@ -56,14 +55,16 @@
56 found = False55 found = False
57 a = AccountManager(None)56 a = AccountManager(None)
5857
58 Model.connect('row-added', row_added)
59
59 for account_id, account in a._accounts.items():60 for account_id, account in a._accounts.items():
60 if account_id.endswith(protocol):61 if account_id.endswith(protocol):
61 found = True62 found = True
62 refresh(account)63 account.protocol(*args)
63 GObject.timeout_add_seconds(300, refresh, account)
6464
65 if not found:65 if not found:
66 print('No {} account found in your Ubuntu Online Accounts!'.format(66 log.error('No {} found in Ubuntu Online Accounts!'.format(protocol))
67 protocol))
68 else:67 else:
69 GObject.MainLoop().run()68 loop = GObject.MainLoop()
69 GObject.timeout_add_seconds(10, loop.quit)
70 loop.run()

Subscribers

People subscribed via source and target branches