Merge lp:~dobey/ubuntuone-client/oauth-log-fixes into lp:ubuntuone-client

Proposed by dobey
Status: Merged
Approved by: Guillermo Gonzalez
Approved revision: 241
Merged at revision: not available
Proposed branch: lp:~dobey/ubuntuone-client/oauth-log-fixes
Merge into: lp:ubuntuone-client
Diff against target: 164 lines
5 files modified
bin/ubuntuone-client-applet (+3/-5)
ubuntuone/oauthdesktop/auth.py (+14/-14)
ubuntuone/oauthdesktop/logger.py (+17/-3)
ubuntuone/oauthdesktop/main.py (+1/-3)
ubuntuone/syncdaemon/logger.py (+1/-1)
To merge this branch: bzr merge lp:~dobey/ubuntuone-client/oauth-log-fixes
Reviewer Review Type Date Requested Status
Guillermo Gonzalez Approve
Rick McBride (community) Approve
Review via email: mp+13017@code.launchpad.net

Commit message

Rotate the oauth log and cap at 1MB
Change a few logger.debug messages to logger.error
Clean up the usage of callback_error in the oauth process

To post a comment you must log in.
Revision history for this message
Rick McBride (rmcbride) wrote :

Other than the below, this looks good.
== Python Lint Notices ==

./ubuntuone/oauthdesktop/auth.py:
    261: undefined name 'e'

review: Approve
241. By dobey

Fix typo: should be errors instead of e

Revision history for this message
Guillermo Gonzalez (verterok) :
review: Approve
242. By dobey

Change log level to INFO
Change startup version logging to info level

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'bin/ubuntuone-client-applet'
2--- bin/ubuntuone-client-applet 2009-10-02 17:55:29 +0000
3+++ bin/ubuntuone-client-applet 2009-10-08 13:35:16 +0000
4@@ -45,9 +45,7 @@
5 from urllib import quote
6
7 from ubuntuone.oauthdesktop.logger import setupLogging
8-setupLogging()
9-import logging
10-logger = logging.getLogger("UbuntuOne.Client.Applet")
11+logger = setupLogging("UbuntuOne.Client.Applet")
12
13 DBusGMainLoop(set_as_default=True)
14
15@@ -96,8 +94,8 @@
16 login = Login(dbus.service.BusName(DBUS_IFACE_AUTH_NAME,
17 bus=dbus.SessionBus()))
18
19- logger.debug(_("Starting Ubuntu One client version %s") %
20- clientdefs.VERSION)
21+ logger.info(_("Starting Ubuntu One client version %s") %
22+ clientdefs.VERSION)
23
24 # Whether or not we are authorized
25 self.is_authorized = False
26
27=== modified file 'ubuntuone/oauthdesktop/auth.py'
28--- ubuntuone/oauthdesktop/auth.py 2009-10-07 15:09:56 +0000
29+++ ubuntuone/oauthdesktop/auth.py 2009-10-08 13:35:16 +0000
30@@ -37,9 +37,7 @@
31 from twisted.web import server, resource
32
33 from ubuntuone.oauthdesktop.logger import setupLogging
34-setupLogging()
35-import logging
36-logger = logging.getLogger("UbuntuOne.OAuthDesktop.auth")
37+logger = setupLogging("UbuntuOne.OAuthDesktop.auth")
38
39
40 class NoAccessToken(Exception):
41@@ -144,6 +142,13 @@
42 'oauth-consumer-key':
43 self.consumer.key})
44
45+ def _forward_error_callback(self, error):
46+ """Forward an error through callback_error()"""
47+ if self.callback_error:
48+ self.callback_error(str(error))
49+ else:
50+ raise error
51+
52 def get_access_token(self):
53 """Get the access token from the keyring.
54
55@@ -229,11 +234,8 @@
56 fp = opener.open(oauth_request.http_url, oauth_request.to_postdata())
57 data = fp.read()
58 except IOError, e:
59- if self.callback_error:
60- self.callback_error(str(e))
61- return
62- else:
63- raise e
64+ self._forward_error_callback(e)
65+ return
66
67 # we deliberately trap anything that might go wrong when parsing the
68 # token, because we do not want this to explicitly fail
69@@ -243,8 +245,9 @@
70 logger.debug("Token successfully requested")
71 return out_token
72 except:
73- logger.debug("Token was not successfully retrieved: data was '%s'",
74+ logger.error("Token was not successfully retrieved: data was '%s'",
75 data)
76+ self._forward_error_callback(oauth.OAuthError(data))
77
78 def open_in_browser(self, url):
79 """Open the given URL in the user's web browser."""
80@@ -255,10 +258,7 @@
81 if p.returncode != 0:
82 errors = "".join(p.stderr.readlines())
83 if errors != "":
84- if self.callback_error is not None:
85- self.callback_error(errors)
86- else:
87- raise Exception(errors)
88+ self._forward_error_callback(IOError(errors))
89
90 def acquire_access_token_if_online(self, description=None, store=False):
91 """Check to see if we are online before trying to acquire"""
92@@ -377,7 +377,7 @@
93 logger.debug("Retrieving access token from OAuth")
94 access_token = self.make_token_request(oauth_request)
95 if not access_token:
96- logger.debug("Failed to get access token.")
97+ logger.error("Failed to get access token.")
98 if self.callback_denied is not None:
99 self.callback_denied()
100 else:
101
102=== modified file 'ubuntuone/oauthdesktop/logger.py'
103--- ubuntuone/oauthdesktop/logger.py 2009-06-26 17:01:42 +0000
104+++ ubuntuone/oauthdesktop/logger.py 2009-10-08 13:35:16 +0000
105@@ -20,6 +20,8 @@
106 import logging
107 import xdg.BaseDirectory
108
109+from logging.handlers import RotatingFileHandler
110+
111 home = xdg.BaseDirectory.xdg_cache_home
112 LOGFOLDER = os.path.join(home, 'ubuntuone', 'log')
113 # create log folder if it doesn't exists
114@@ -28,7 +30,19 @@
115
116 LOGFILENAME = os.path.join(LOGFOLDER, 'oauth-login.log')
117
118-def setupLogging():
119+# Only log this level and above
120+LOG_LEVEL = logging.INFO
121+
122+root_formatter = logging.Formatter(
123+ fmt="%(asctime)s:%(msecs)s %(name)s %(message)s")
124+root_handler = RotatingFileHandler(LOGFILENAME, maxBytes=1048576,
125+ backupCount=1)
126+root_handler.setLevel(LOG_LEVEL)
127+
128+def setupLogging(log_domain):
129 """Create basic logger to set filename"""
130- logging.basicConfig(level=logging.DEBUG, filename=LOGFILENAME,
131- format="%(asctime)s:%(msecs)s %(name)s %(message)s")
132+ logger = logging.getLogger(log_domain)
133+ logger.propagate = False
134+ logger.setLevel(LOG_LEVEL)
135+ logger.addHandler(root_handler)
136+ return logger
137
138=== modified file 'ubuntuone/oauthdesktop/main.py'
139--- ubuntuone/oauthdesktop/main.py 2009-09-08 13:28:01 +0000
140+++ ubuntuone/oauthdesktop/main.py 2009-10-08 13:35:16 +0000
141@@ -34,9 +34,7 @@
142 from twisted.internet.threads import deferToThread
143
144 from ubuntuone.oauthdesktop.logger import setupLogging
145-setupLogging()
146-import logging
147-logger = logging.getLogger("UbuntuOne.OAuthDesktop.main")
148+logger = setupLogging("UbuntuOne.OAuthDesktop.main")
149
150 DBusGMainLoop(set_as_default=True)
151
152
153=== modified file 'ubuntuone/syncdaemon/logger.py'
154--- ubuntuone/syncdaemon/logger.py 2009-08-29 01:16:01 +0000
155+++ ubuntuone/syncdaemon/logger.py 2009-10-08 13:35:16 +0000
156@@ -33,7 +33,7 @@
157 # extra levels
158 # be more verbose than logging.DEBUG(10)
159 TRACE = 5
160-# info that we always want to log (logging.ERROR+1)
161+# info that we almost always want to log (logging.ERROR - 1)
162 NOTE = logging.ERROR - 1
163
164 # map names to the extra levels

Subscribers

People subscribed via source and target branches