Linaro Android Bot Review

Merge lp:~fgiff/linaro-android-bot-review/884390 into lp:linaro-android-bot-review

  1. 884390
  2. Merge into trunk
Proposed by Frans Gifford
Status: Merged
Approved by: Frans Gifford
Approved revision: 77
Merged at revision: 77
Proposed branch: lp:~fgiff/linaro-android-bot-review/884390
Merge into: lp:linaro-android-bot-review
Diff against target: 128 lines (+21/-21)
3 files modified
gerritconnection.py (+10/-10)
jenkinsconnection.py (+5/-5)
lavaconnection.py (+6/-6)
To merge this branch: bzr merge lp:~fgiff/linaro-android-bot-review/884390
Reviewer Review Type Date Requested Status
Paul Sokolovsky Approve
Review via email: mp+80847@code.launchpad.net
To post a comment you must log in.
Revision history for this message
Paul Sokolovsky (pfalcon) wrote :

Just a note - in exception handling context (in "except:" suite), you can use log.exception("message") to make it log message and associated exception stack trace. Unless you of course want to suppress stack trace logging, then using log.error() is the way to go. I don't think the distinction is critical here, so looks ok, thanks.

review: Approve
Revision history for this message
Frans Gifford (fgiff) wrote :

> Just a note - in exception handling context (in "except:" suite), you can use
> log.exception("message") to make it log message and associated exception stack
> trace. Unless you of course want to suppress stack trace logging, then using
> log.error() is the way to go. I don't think the distinction is critical here,
> so looks ok, thanks.

I didn't about log.exception, which is why I used log.error instead. We can change it later if necessary.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'gerritconnection.py'
--- gerritconnection.py 2011-10-17 21:14:56 +0000
+++ gerritconnection.py 2011-10-31 21:34:25 +0000
@@ -31,8 +31,8 @@
31 try:31 try:
32 self.client.load_system_host_keys(self.review_known_hosts)32 self.client.load_system_host_keys(self.review_known_hosts)
33 except IOError:33 except IOError:
34 print "Error: Failed to load known_hosts file so unable to \34 self.log.error("Error: Failed to load known_hosts file so unable to \
35connect to gerrit."35connect to gerrit.")
36 raise36 raise
37 try:37 try:
38 self.client.connect(self.review_host,38 self.client.connect(self.review_host,
@@ -40,19 +40,19 @@
40 self.ssh_user,40 self.ssh_user,
41 key_filename=self.ssh_key_file)41 key_filename=self.ssh_key_file)
42 except paramiko.BadHostKeyException:42 except paramiko.BadHostKeyException:
43 print "Error: Failed verification of gerrit's SSH key. \43 self.log.error("Error: Failed verification of gerrit's SSH key. \
44If the gerrit key has changed, please reflect this in known_hosts."44If the gerrit key has changed, please reflect this in known_hosts.")
45 raise45 raise
46 except paramiko.AuthenticationException:46 except paramiko.AuthenticationException:
47 print "Error: Failed authentication to gerrit server. Please \47 self.log.error("Error: Failed authentication to gerrit server. Please \
48check the validity of username %s and SSH key %s." % (self.ssh_user,48check the validity of username %s and SSH key %s." % (self.ssh_user,
49 self.ssh_key_file)49 self.ssh_key_file))
50 raise50 raise
51 except paramiko.SSHException:51 except paramiko.SSHException:
52 print "Error: Failed to complete SSH session with gerrit."52 self.log.error("Error: Failed to complete SSH session with gerrit.")
53 raise53 raise
54 except socket.error:54 except socket.error:
55 print "Error: Socket error when connecting to gerrit."55 self.log.error("Error: Socket error when connecting to gerrit.")
56 raise56 raise
57 return retval57 return retval
5858
@@ -128,8 +128,8 @@
128 sha1)128 sha1)
129 self.exec_command(cmd)129 self.exec_command(cmd)
130 except paramiko.SSHException:130 except paramiko.SSHException:
131 print "Error: Failed to complete SSH session with gerrit \131 self.log.error("Error: Failed to complete SSH session with gerrit \
132for %s %s." % (project, sha1)132for %s %s." % (project, sha1))
133 raise133 raise
134134
135135
136136
=== modified file 'jenkinsconnection.py'
--- jenkinsconnection.py 2011-10-14 00:36:41 +0000
+++ jenkinsconnection.py 2011-10-31 21:34:25 +0000
@@ -21,7 +21,7 @@
21 try:21 try:
22 f = open(jenkins_passwd_file)22 f = open(jenkins_passwd_file)
23 except IOError:23 except IOError:
24 print "Error: could not open %s." % (jenkins_passwd_file)24 log.error("Error: could not open %s." % (jenkins_passwd_file))
25 self.jenkins_auth = ""25 self.jenkins_auth = ""
26 else:26 else:
27 self.jenkins_auth = {'Authorization': 'Basic %s' % (27 self.jenkins_auth = {'Authorization': 'Basic %s' % (
@@ -132,18 +132,18 @@
132 # Send build command.132 # Send build command.
133 resp = urllib2.urlopen(req)133 resp = urllib2.urlopen(req)
134 except urllib2.URLError:134 except urllib2.URLError:
135 print "Error: failed to start build."135 log.error("Error: failed to start build.")
136136
137 # TODO: Do we care about redirects?137 # TODO: Do we care about redirects?
138 if resp.getcode() == 200:138 if resp.getcode() == 200:
139 build_url = "%sjob/%s/%s/" % (self.jenkins_url, job[0], job[2])139 build_url = "%sjob/%s/%s/" % (self.jenkins_url, job[0], job[2])
140 print "Build url: %s" % (build_url)140 log.error("Build url: %s" % (build_url))
141141
142 else:142 else:
143 # Give some indication that stuff failed.143 # Give some indication that stuff failed.
144 print "Failed to queue build: %s gave HTTP code %s." % (144 log.error( "Failed to queue build: %s gave HTTP code %s." % (
145 req.get_full_url(),145 req.get_full_url(),
146 resp.getcode())146 resp.getcode()))
147 return build_url147 return build_url
148148
149if __name__ == "__main__":149if __name__ == "__main__":
150150
=== modified file 'lavaconnection.py'
--- lavaconnection.py 2011-10-13 17:51:54 +0000
+++ lavaconnection.py 2011-10-31 21:34:25 +0000
@@ -10,7 +10,7 @@
1010
11class LoggingTransport(xmlrpclib.Transport):11class LoggingTransport(xmlrpclib.Transport):
12 def send_request(self, connection, handler, request_body):12 def send_request(self, connection, handler, request_body):
13 xmlroc_log.debug("%s", (connection, handler, request_body))13 xmlrpc_log.debug("%s", (connection, handler, request_body))
14 return xmlrpclib.Transport.send_request(self, connection, handler, request_body)14 return xmlrpclib.Transport.send_request(self, connection, handler, request_body)
1515
1616
@@ -70,10 +70,10 @@
70 bundles = self.server.dashboard.bundles("%s" % (self.lava_stream))70 bundles = self.server.dashboard.bundles("%s" % (self.lava_stream))
71 except xmlrpclib.Fault:71 except xmlrpclib.Fault:
72 # Problem with LAVA xmlrpc call.72 # Problem with LAVA xmlrpc call.
73 print "Error: Couldn't get test results from LAVA."73 self.log.error("Error: Couldn't get test results from LAVA.")
74 except xmlrpclib.ProtocolError:74 except xmlrpclib.ProtocolError:
75 # Problem connecting to LAVA.75 # Problem connecting to LAVA.
76 print "Error: Couldn't connect to LAVA."76 self.log.error("Error: Couldn't connect to LAVA.")
77 lava_results = []77 lava_results = []
78 # Since we're operating on fresh builds, we know the bundle will 78 # Since we're operating on fresh builds, we know the bundle will
79 # have been uploaded in the last 48 hrs, so prune out older ones.79 # have been uploaded in the last 48 hrs, so prune out older ones.
@@ -96,12 +96,12 @@
96 # Not a user-friendly identifier, but we'd need to download96 # Not a user-friendly identifier, but we'd need to download
97 # the bundle to work out which build it corresponds to97 # the bundle to work out which build it corresponds to
98 # and this error occurs when we can't do that.98 # and this error occurs when we can't do that.
99 print "Error: Couldn't fetch test results for %s" % (99 self.log.error("Error: Couldn't fetch test results for %s" % (
100 bundle["content_sha1"])100 bundle["content_sha1"]))
101 continue101 continue
102 except xmlrpclib.ProtocolError:102 except xmlrpclib.ProtocolError:
103 # Problem connecting to LAVA.103 # Problem connecting to LAVA.
104 print "Error: Couldn't connect to LAVA."104 self.log.error("Error: Couldn't connect to LAVA.")
105 continue105 continue
106 return lava_results106 return lava_results
107107

Subscribers

People subscribed via source and target branches

to all changes: