Merge lp:~pfalcon/tcwg-web/handle-lava-errors into lp:tcwg-web

Proposed by Paul Sokolovsky
Status: Merged
Merged at revision: 247
Proposed branch: lp:~pfalcon/tcwg-web/handle-lava-errors
Merge into: lp:tcwg-web
Diff against target: 93 lines (+32/-6)
3 files modified
common.py (+7/-2)
lava.py (+10/-1)
scheduler.py (+15/-3)
To merge this branch: bzr merge lp:~pfalcon/tcwg-web/handle-lava-errors
Reviewer Review Type Date Requested Status
Matthew Gretton-Dann Approve
Review via email: mp+165437@code.launchpad.net

Description of the change

This fixes CBuild 500 Internal Errors when LAVA request fails, and shows a proper error message to the user instead.

To post a comment you must log in.
Revision history for this message
Will Newton (will-newton) wrote :

Line 10 should say "if successful". Other than that looks ok to me, but I am not familiar with LAVA internals.

Revision history for this message
Matthew Gretton-Dann (matthew-gretton-dann) wrote :

Looks good to me.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'common.py'
2--- common.py 2013-04-19 11:56:30 +0000
3+++ common.py 2013-05-23 16:33:28 +0000
4@@ -468,7 +468,8 @@
5
6 def schedule_job(queue, job, contents=None, use_template_file=False):
7 """Schedule a job. All job scheduling should happen via this
8- function, which encapsulates support for various schdulers in use."""
9+ function, which encapsulates support for various schedulers in use.
10+ Returns error message or None is successful."""
11 queue_dir = map_filename('queue/%s/' % queue, 'scheduler') + '/'
12
13 job_file = os.path.join(queue_dir, job + '.job')
14@@ -488,7 +489,10 @@
15 request_env = None
16 if hasattr(web.ctx, 'env'):
17 request_env = web.ctx.env
18- lava_job_id, lava_job_url = lava.submit_job(lava_template, queue, job, request_env)
19+ try:
20+ lava_job_id, lava_job_url = lava.submit_job(lava_template, queue, job, request_env)
21+ except lava.LavaError, e:
22+ return "Error submitting to LAVA queue '%s': %s" % (queue, e.message)
23
24 jobfile = '%s%s.job' % (queue_dir, job)
25 lockfile = '%s.lock' % jobfile
26@@ -500,6 +504,7 @@
27 print "Queue %s: scheduled %s for LAVA, job id: %d, url: %s" % \
28 (queue, job, lava_job_id, lava_job_url)
29
30+ return None
31
32 def create_job_id(jobfile):
33
34
35=== modified file 'lava.py'
36--- lava.py 2013-04-26 19:27:01 +0000
37+++ lava.py 2013-05-23 16:33:28 +0000
38@@ -4,6 +4,10 @@
39 import common
40
41
42+class LavaError(Exception):
43+ pass
44+
45+
46 def get_api(url):
47 user = common.get_config('lava:' + url, 'user')
48 if not user:
49@@ -65,7 +69,12 @@
50 api_url = submit_url
51
52 lava_api = get_api(api_url)
53- job_id = lava_api.scheduler.submit_job(job_def)
54+ try:
55+ job_id = lava_api.scheduler.submit_job(job_def)
56+ except xmlrpclib.ProtocolError, e:
57+ # Note that ProtocolError may include sensitive information
58+ # like auth credentials
59+ raise LavaError("%s %s" % (e.errcode, e.errmsg))
60 job_url = ''
61 if submit_url:
62 if submit_url[-1] == '/':
63
64=== modified file 'scheduler.py'
65--- scheduler.py 2013-01-17 18:00:48 +0000
66+++ scheduler.py 2013-05-23 16:33:28 +0000
67@@ -457,11 +457,23 @@
68 job = form.d.job.strip()
69 extras = form.d.extras
70
71+ errors = []
72+ successful = []
73 for queue in selected:
74- common.schedule_job(queue, job, contents=extras)
75+ msg = common.schedule_job(queue, job, contents=extras)
76+ if msg:
77+ errors.append(msg)
78+ else:
79+ successful.append(queue)
80
81- msg = 'Spawned %s into %s' % (job, ', '.join(selected))
82- return web.seeother('/helpers/scheduler?%s' % urllib.urlencode({'msg': msg}), absolute=True)
83+ msg = ''
84+ ret_url = '/helpers/scheduler'
85+ if errors:
86+ msg += '. '.join(errors) + '. '
87+ ret_url = '/helpers/scheduler/spawn'
88+ if successful:
89+ msg += 'Spawned %s into %s' % (job, ', '.join(successful))
90+ return web.seeother(ret_url + '?' + urllib.urlencode({'msg': msg}), absolute=True)
91
92
93 class lava_testdef:

Subscribers

People subscribed via source and target branches