Merge lp:~brian.curtin/ubuntuone-control-panel/updater-logging into lp:ubuntuone-control-panel

Proposed by Brian Curtin on 2013-01-25
Status: Merged
Approved by: Brian Curtin on 2013-01-28
Approved revision: 402
Merged at revision: 394
Proposed branch: lp:~brian.curtin/ubuntuone-control-panel/updater-logging
Merge into: lp:ubuntuone-control-panel
Diff against target: 171 lines (+37/-27)
2 files modified
bin/ubuntuone-updater (+35/-25)
ubuntuone/controlpanel/tests/test_web_client.py (+2/-2)
To merge this branch: bzr merge lp:~brian.curtin/ubuntuone-control-panel/updater-logging
Reviewer Review Type Date Requested Status
dobey (community) Approve on 2013-01-28
Paul Hummer (community) Approve on 2013-01-28
Mike McCracken (community) 2013-01-25 Approve on 2013-01-25
Review via email: mp+145030@code.launchpad.net

Commit message

- Use logging functionality instead of printing messages to stdout (lp:1105447)

Description of the change

In order to make it easy (or even possible) to figure out what the updater is doing in a real environment, we need to move away from printing and to logging.

To post a comment you must log in.
Mike McCracken (mikemc) wrote :

Looks good, works for me on osx CLI

review: Approve
Paul Hummer (rockstar) :
review: Approve
dobey (dobey) :
review: Needs Fixing
dobey (dobey) wrote :

8 -from __future__ import print_function

We should leave these, even when not calling print(), so that if anyone does add a print call in the future, it will fail if the old print statement style is used. I'd also ask to use unicode_literals here, but I'm not sure if it will cause problems in this code or not.

dobey (dobey) :
review: Approve
Ubuntu One Auto Pilot (otto-pilot) wrote :
Download full text (46.8 KiB)

The attempt to merge lp:~brian.curtin/ubuntuone-control-panel/updater-logging into lp:ubuntuone-control-panel failed. Below is the output from the failed tests.

*** Running DBus test suite ***
ubuntuone.controlpanel.dbustests.test_dbus_service
  BaseTestCase
    runTest ... [OK]
  DBusServiceMainTestCase
    test_dbus_service_cant_register ... Control panel backend already running.
                                   [OK]
    test_dbus_service_main ... [OK]
  DBusServiceTestCase
    test_cant_register_twice ... [SKIPPED]
    test_dbus_busname_created ... [OK]
    test_error_handler_default ... [OK]
    test_error_handler_with_exception ... [OK]
    test_error_handler_with_failure ... [OK]
    test_error_handler_with_non_string_dict ... [OK]
    test_error_handler_with_string_dict ... [OK]
    test_register_service ... [OK]
  FileSyncTestCase
    test_file_sync_status_changed ... [OK]
    test_file_sync_status_disabled ... [OK]
    test_file_sync_status_disconnected ... [OK]
    test_file_sync_status_error ... [OK]
    test_file_sync_status_idle ... [OK]
    test_file_sync_status_starting ... [OK]
    test_file_sync_status_stopped ... [OK]
    test_file_sync_status_syncing ... [OK]
    test_file_sync_status_unknown ... [OK]
    test_status_changed_handler ... [OK]
    test_status_changed_handler_after_status_requested ... [OK]
    test_status_changed_handler_after_status_requested_twice ... [OK]
  OperationsAuthErrorTestCase
    test_account_info_returned ... [OK]
    test_change_device_settings ... [OK]
    test_change_replication_settings ... [OK]
    test_change_volume_settings ... [OK]
    test_connect_files ... [OK]
    test_devices_info_returned ... [OK]
    test_disable_files ... [OK]
    test_disconnect_files ... [OK]
    test_enable_files ... [OK]
    test_remove_device ... [OK]
    test_replications_info ... [OK]
    test_restart_files ... [OK]
    ...

Ubuntu One Auto Pilot (otto-pilot) wrote :
Download full text (167.7 KiB)

The attempt to merge lp:~brian.curtin/ubuntuone-control-panel/updater-logging into lp:ubuntuone-control-panel failed. Below is the output from the failed tests.

*** Running DBus test suite ***
ubuntuone.controlpanel.dbustests.test_dbus_service
  BaseTestCase
    runTest ... [OK]
  DBusServiceMainTestCase
    test_dbus_service_cant_register ... Control panel backend already running.
                                   [OK]
    test_dbus_service_main ... [OK]
  DBusServiceTestCase
    test_cant_register_twice ... [SKIPPED]
    test_dbus_busname_created ... [OK]
    test_error_handler_default ... [OK]
    test_error_handler_with_exception ... [OK]
    test_error_handler_with_failure ... [OK]
    test_error_handler_with_non_string_dict ... [OK]
    test_error_handler_with_string_dict ... [OK]
    test_register_service ... [OK]
  FileSyncTestCase
    test_file_sync_status_changed ... [OK]
    test_file_sync_status_disabled ... [OK]
    test_file_sync_status_disconnected ... [OK]
    test_file_sync_status_error ... [OK]
    test_file_sync_status_idle ... [OK]
    test_file_sync_status_starting ... [OK]
    test_file_sync_status_stopped ... [OK]
    test_file_sync_status_syncing ... [OK]
    test_file_sync_status_unknown ... [OK]
    test_status_changed_handler ... [OK]
    test_status_changed_handler_after_status_requested ... [OK]
    test_status_changed_handler_after_status_requested_twice ... [OK]
  OperationsAuthErrorTestCase
    test_account_info_returned ... [OK]
    test_change_device_settings ... [OK]
    test_change_replication_settings ... [OK]
    test_change_volume_settings ... [OK]
    test_connect_files ... [OK]
    test_devices_info_returned ... [OK]
    test_disable_files ... [OK]
    test_disconnect_files ... [OK]
    test_enable_files ... [OK]
    test_remove_device ... [OK]
    test_replications_info ... [OK]
    test_restart_files ... [OK]
    ...

402. By Brian Curtin on 2013-01-28

Remove unused import

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'bin/ubuntuone-updater'
2--- bin/ubuntuone-updater 2012-12-11 22:18:08 +0000
3+++ bin/ubuntuone-updater 2013-01-28 18:15:26 +0000
4@@ -20,10 +20,10 @@
5 from __future__ import print_function
6
7 try:
8- from urllib.request import urlopen, URLError
9+ from urllib.request import urlopen
10 from urllib.parse import urljoin
11 except ImportError:
12- from urllib2 import urlopen, URLError
13+ from urllib2 import urlopen
14 from urlparse import urljoin
15
16 import argparse
17@@ -36,13 +36,17 @@
18
19 from dirspec.utils import get_program_path
20
21+from ubuntuone.controlpanel.logger import setup_logging
22+
23+
24+logger = setup_logging("updater")
25
26 U1SDTOOL_EXECUTABLE = 'u1sdtool'
27 DARWIN_APP_NAMES = {U1SDTOOL_EXECUTABLE: 'U1SDTool.app'}
28
29
30 if sys.platform == "darwin":
31- from Cocoa import NSRunningApplication, NSLog
32+ from Cocoa import NSRunningApplication
33 # IDs to terminate: Leave syncdaemon off this list, it is
34 # terminated separately.
35 BUNDLE_IDS = ["com.ubuntu.sso.login-qt",
36@@ -58,8 +62,9 @@
37 def _do_download(url):
38 """Download a URL and return the data it contains."""
39 resource = urlopen(url)
40- if resource:
41- data = resource.read()
42+ logger.info("Downloading {}".format(url))
43+ data = resource.read()
44+ logger.info("Downloaded {}".format(url))
45 return data
46
47
48@@ -72,7 +77,10 @@
49
50 md5 = hashlib.md5()
51 md5.update(data)
52- if checksum != md5.hexdigest():
53+ digest = md5.hexdigest()
54+ if checksum != digest:
55+ logger.error("Checksum mismatch. Expected {}, computed {}".format(
56+ checksum, digest))
57 raise Exception("Checksum mismatch")
58
59 fd, name = tempfile.mkstemp()
60@@ -89,7 +97,7 @@
61 args.insert(0, 'open')
62 subprocess.Popen(args)
63 except Exception as exc:
64- print("Unable to install {}: {}".format(url, exc))
65+ logger.error("Unable to install {}: {}".format(url, exc))
66 return False
67 return True
68
69@@ -100,8 +108,8 @@
70
71 try:
72 data = _do_download(args.url)
73- except URLError as err:
74- print("Download failed: {}".format(err))
75+ except Exception as exc:
76+ logger.error("Download failed: {}".format(exc))
77 return error
78
79 try:
80@@ -110,21 +118,20 @@
81 avail_ver = available["version"]
82 upgrade_available = avail_ver > args.current
83 if upgrade_available:
84- msg = "A new {} version is available at {}."
85- print(msg.format(args.release, available["url"]))
86+ logger.info("A new {} version is available at {}.".format(
87+ args.release, available["url"]))
88 elif avail_ver == args.current:
89- print("Version {} is the latest {}.".format(args.current,
90- args.release))
91+ logger.info("Version {} is the latest {}.".format(args.current,
92+ args.release))
93 else:
94- print("Warning: installed version {} newer than advertised"
95- " available version {} somehow.".format(args.current,
96- avail_ver))
97+ logger.info("Installed version {} newer than advertised"
98+ " available version {}.".format(args.current, avail_ver))
99
100 except Exception as exc:
101 # Catch everything here.
102 # KeyError is most likely, but we can't let this take us down.
103- print("Exception while reading {}:".format(args.url),
104- exc.__class__, exc.message)
105+ logger.error("Exception while reading {}: {}, {}".format(
106+ args.url, exc.__class__, exc.message))
107 return error
108
109 return upgrade_available, available["url"], available["checksum"]
110@@ -134,21 +141,24 @@
111 """Kill any running u1 processes."""
112 if sys.platform != "darwin":
113 return
114- NSLog("in kill")
115+ logger.info("Killing running processes")
116
117 try:
118 u1sdtoolpath = get_program_path(U1SDTOOL_EXECUTABLE,
119 app_names=DARWIN_APP_NAMES)
120-
121- out = subprocess.check_output([u1sdtoolpath, '-q'])
122- NSLog("u1sdtool said: {}".format(out))
123 except Exception as e:
124- NSLog("exception running {} -q: {}".format(u1sdtoolpath, e))
125+ logger.error("exception getting u1sdtool's path: {}".format(e))
126+ else:
127+ try:
128+ out = subprocess.check_output([u1sdtoolpath, '-q'])
129+ logger.info("u1sdtool said: {}".format(out))
130+ except Exception as e:
131+ logger.error("exception running {} -q: {}".format(u1sdtoolpath, e))
132
133 for id in BUNDLE_IDS:
134 NSRA = NSRunningApplication
135 apps = NSRA.runningApplicationsWithBundleIdentifier_(id)
136- NSLog("id: {} apps: {}".format(id, apps))
137+ logger.info("id: {} apps: {}".format(id, apps))
138
139 for app in apps:
140 app.terminate()
141@@ -175,7 +185,7 @@
142
143 if args.install and new_available:
144 installer = urljoin(args.url, file)
145- print("Downloading and installing", installer)
146+ logger.info("Downloading and installing {}".format(installer))
147 kill_running_processes()
148 return 1 if run_install(installer, checksum) else 0
149
150
151=== modified file 'ubuntuone/controlpanel/tests/test_web_client.py'
152--- ubuntuone/controlpanel/tests/test_web_client.py 2012-10-26 15:55:18 +0000
153+++ ubuntuone/controlpanel/tests/test_web_client.py 2013-01-28 18:15:26 +0000
154@@ -19,7 +19,7 @@
155 from urlparse import urlparse, parse_qs
156
157 from twisted.internet import defer
158-from twisted.web import resource
159+from twisted.web import http, resource
160
161 from ubuntuone.devtools.testing.txwebserver import HTTPWebServer
162
163@@ -83,7 +83,7 @@
164 devices_resource.contents = SAMPLE_RESOURCE
165 root.putChild("devices", devices_resource)
166 root.putChild("throwerror", resource.NoResource())
167- unauthorized = resource.ErrorPage(resource.http.UNAUTHORIZED,
168+ unauthorized = resource.ErrorPage(http.UNAUTHORIZED,
169 "Unauthrorized", "Unauthrorized")
170 root.putChild("unauthorized", unauthorized)
171 super(MockWebServer, self).__init__(root)

Subscribers

People subscribed via source and target branches