Merge lp:~rackspace-titan/nova/fix_faults into lp:~hudson-openstack/nova/trunk

Proposed by Dan Prince
Status: Merged
Approved by: Devin Carlen
Approved revision: 1278
Merged at revision: 1291
Proposed branch: lp:~rackspace-titan/nova/fix_faults
Merge into: lp:~hudson-openstack/nova/trunk
Diff against target: 316 lines (+37/-31)
6 files modified
nova/api/openstack/consoles.py (+2/-1)
nova/api/openstack/contrib/multinic.py (+3/-2)
nova/api/openstack/contrib/volumes.py (+3/-2)
nova/api/openstack/faults.py (+1/-0)
nova/api/openstack/servers.py (+23/-26)
nova/tests/api/openstack/test_faults.py (+5/-0)
To merge this branch: bzr merge lp:~rackspace-titan/nova/fix_faults
Reviewer Review Type Date Requested Status
Devin Carlen (community) Approve
Jason Kölker (community) Approve
Vish Ishaya (community) Approve
Review via email: mp+68585@code.launchpad.net

Description of the change

Set the status_int on fault wrapped exceptions. Fixes WSGI logging issues when faults are returned.

Updated so that webob exceptions aren't used for the happy path (HTTP 200 responses). We now return a proper webob object response in these cases. This fixes issues where HTML/XML would get incorrectly returned with the old style happy path exceptions.

To post a comment you must log in.
Revision history for this message
Vish Ishaya (vishvananda) wrote :

looks good dan

review: Approve
Revision history for this message
Jason Kölker (jason-koelker) wrote :

Is bueno. I like using the response object instead of returning exceptions.

review: Approve
Revision history for this message
Devin Carlen (devcamcar) wrote :

lgtm

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'nova/api/openstack/consoles.py'
2--- nova/api/openstack/consoles.py 2011-07-06 20:28:10 +0000
3+++ nova/api/openstack/consoles.py 2011-07-20 17:57:32 +0000
4@@ -16,6 +16,7 @@
5 # under the License.
6
7 from webob import exc
8+import webob
9
10 from nova import console
11 from nova import exception
12@@ -86,7 +87,7 @@
13 int(id))
14 except exception.NotFound:
15 return faults.Fault(exc.HTTPNotFound())
16- return exc.HTTPAccepted()
17+ return webob.Response(status_int=202)
18
19
20 def create_resource():
21
22=== modified file 'nova/api/openstack/contrib/multinic.py'
23--- nova/api/openstack/contrib/multinic.py 2011-07-12 01:59:01 +0000
24+++ nova/api/openstack/contrib/multinic.py 2011-07-20 17:57:32 +0000
25@@ -16,6 +16,7 @@
26 """The multinic extension."""
27
28 from webob import exc
29+import webob
30
31 from nova import compute
32 from nova import log as logging
33@@ -103,7 +104,7 @@
34 except Exception, e:
35 LOG.exception(_("Error in addFixedIp %s"), e)
36 return faults.Fault(exc.HTTPBadRequest())
37- return exc.HTTPAccepted()
38+ return webob.Response(status_int=202)
39
40 def _remove_fixed_ip(self, input_dict, req, id):
41 """Removes an IP from an instance."""
42@@ -122,4 +123,4 @@
43 except Exception, e:
44 LOG.exception(_("Error in removeFixedIp %s"), e)
45 return faults.Fault(exc.HTTPBadRequest())
46- return exc.HTTPAccepted()
47+ return webob.Response(status_int=202)
48
49=== modified file 'nova/api/openstack/contrib/volumes.py'
50--- nova/api/openstack/contrib/volumes.py 2011-06-24 12:01:51 +0000
51+++ nova/api/openstack/contrib/volumes.py 2011-07-20 17:57:32 +0000
52@@ -16,6 +16,7 @@
53 """The volumes extension."""
54
55 from webob import exc
56+import webob
57
58 from nova import compute
59 from nova import exception
60@@ -104,7 +105,7 @@
61 self.volume_api.delete(context, volume_id=id)
62 except exception.NotFound:
63 return faults.Fault(exc.HTTPNotFound())
64- return exc.HTTPAccepted()
65+ return webob.Response(status_int=202)
66
67 def index(self, req):
68 """Returns a summary list of volumes."""
69@@ -279,7 +280,7 @@
70 self.compute_api.detach_volume(context,
71 volume_id=volume_id)
72
73- return exc.HTTPAccepted()
74+ return webob.Response(status_int=202)
75
76 def _items(self, req, server_id, entity_maker):
77 """Returns a list of attachments, transformed through entity_maker."""
78
79=== modified file 'nova/api/openstack/faults.py'
80--- nova/api/openstack/faults.py 2011-05-19 20:16:06 +0000
81+++ nova/api/openstack/faults.py 2011-07-20 17:57:32 +0000
82@@ -40,6 +40,7 @@
83 def __init__(self, exception):
84 """Create a Fault for the given webob.exc.exception."""
85 self.wrapped_exc = exception
86+ self.status_int = exception.status_int
87
88 @webob.dec.wsgify(RequestClass=wsgi.Request)
89 def __call__(self, req):
90
91=== modified file 'nova/api/openstack/servers.py'
92--- nova/api/openstack/servers.py 2011-07-14 18:47:14 +0000
93+++ nova/api/openstack/servers.py 2011-07-20 17:57:32 +0000
94@@ -17,6 +17,7 @@
95 import traceback
96
97 from webob import exc
98+import webob
99
100 from nova import compute
101 from nova import db
102@@ -189,7 +190,7 @@
103 except Exception, e:
104 LOG.exception(_("Error in revert-resize %s"), e)
105 return faults.Fault(exc.HTTPBadRequest())
106- return exc.HTTPAccepted()
107+ return webob.Response(status_int=202)
108
109 def _action_resize(self, input_dict, req, id):
110 return exc.HTTPNotImplemented()
111@@ -207,7 +208,7 @@
112 except Exception, e:
113 LOG.exception(_("Error in reboot %s"), e)
114 return faults.Fault(exc.HTTPUnprocessableEntity())
115- return exc.HTTPAccepted()
116+ return webob.Response(status_int=202)
117
118 def _action_migrate(self, input_dict, req, id):
119 try:
120@@ -215,7 +216,7 @@
121 except Exception, e:
122 LOG.exception(_("Error in migrate %s"), e)
123 return faults.Fault(exc.HTTPBadRequest())
124- return exc.HTTPAccepted()
125+ return webob.Response(status_int=202)
126
127 @scheduler_api.redirect_handler
128 def lock(self, req, id):
129@@ -231,7 +232,7 @@
130 readable = traceback.format_exc()
131 LOG.exception(_("Compute.api::lock %s"), readable)
132 return faults.Fault(exc.HTTPUnprocessableEntity())
133- return exc.HTTPAccepted()
134+ return webob.Response(status_int=202)
135
136 @scheduler_api.redirect_handler
137 def unlock(self, req, id):
138@@ -247,7 +248,7 @@
139 readable = traceback.format_exc()
140 LOG.exception(_("Compute.api::unlock %s"), readable)
141 return faults.Fault(exc.HTTPUnprocessableEntity())
142- return exc.HTTPAccepted()
143+ return webob.Response(status_int=202)
144
145 @scheduler_api.redirect_handler
146 def get_lock(self, req, id):
147@@ -262,7 +263,7 @@
148 readable = traceback.format_exc()
149 LOG.exception(_("Compute.api::get_lock %s"), readable)
150 return faults.Fault(exc.HTTPUnprocessableEntity())
151- return exc.HTTPAccepted()
152+ return webob.Response(status_int=202)
153
154 @scheduler_api.redirect_handler
155 def reset_network(self, req, id, body):
156@@ -277,7 +278,7 @@
157 readable = traceback.format_exc()
158 LOG.exception(_("Compute.api::reset_network %s"), readable)
159 return faults.Fault(exc.HTTPUnprocessableEntity())
160- return exc.HTTPAccepted()
161+ return webob.Response(status_int=202)
162
163 @scheduler_api.redirect_handler
164 def inject_network_info(self, req, id, body):
165@@ -292,7 +293,7 @@
166 readable = traceback.format_exc()
167 LOG.exception(_("Compute.api::inject_network_info %s"), readable)
168 return faults.Fault(exc.HTTPUnprocessableEntity())
169- return exc.HTTPAccepted()
170+ return webob.Response(status_int=202)
171
172 @scheduler_api.redirect_handler
173 def pause(self, req, id, body):
174@@ -304,7 +305,7 @@
175 readable = traceback.format_exc()
176 LOG.exception(_("Compute.api::pause %s"), readable)
177 return faults.Fault(exc.HTTPUnprocessableEntity())
178- return exc.HTTPAccepted()
179+ return webob.Response(status_int=202)
180
181 @scheduler_api.redirect_handler
182 def unpause(self, req, id, body):
183@@ -316,7 +317,7 @@
184 readable = traceback.format_exc()
185 LOG.exception(_("Compute.api::unpause %s"), readable)
186 return faults.Fault(exc.HTTPUnprocessableEntity())
187- return exc.HTTPAccepted()
188+ return webob.Response(status_int=202)
189
190 @scheduler_api.redirect_handler
191 def suspend(self, req, id, body):
192@@ -328,7 +329,7 @@
193 readable = traceback.format_exc()
194 LOG.exception(_("compute.api::suspend %s"), readable)
195 return faults.Fault(exc.HTTPUnprocessableEntity())
196- return exc.HTTPAccepted()
197+ return webob.Response(status_int=202)
198
199 @scheduler_api.redirect_handler
200 def resume(self, req, id, body):
201@@ -340,7 +341,7 @@
202 readable = traceback.format_exc()
203 LOG.exception(_("compute.api::resume %s"), readable)
204 return faults.Fault(exc.HTTPUnprocessableEntity())
205- return exc.HTTPAccepted()
206+ return webob.Response(status_int=202)
207
208 @scheduler_api.redirect_handler
209 def rescue(self, req, id):
210@@ -352,7 +353,7 @@
211 readable = traceback.format_exc()
212 LOG.exception(_("compute.api::rescue %s"), readable)
213 return faults.Fault(exc.HTTPUnprocessableEntity())
214- return exc.HTTPAccepted()
215+ return webob.Response(status_int=202)
216
217 @scheduler_api.redirect_handler
218 def unrescue(self, req, id):
219@@ -364,7 +365,7 @@
220 readable = traceback.format_exc()
221 LOG.exception(_("compute.api::unrescue %s"), readable)
222 return faults.Fault(exc.HTTPUnprocessableEntity())
223- return exc.HTTPAccepted()
224+ return webob.Response(status_int=202)
225
226 @scheduler_api.redirect_handler
227 def get_ajax_console(self, req, id):
228@@ -374,7 +375,7 @@
229 int(id))
230 except exception.NotFound:
231 return faults.Fault(exc.HTTPNotFound())
232- return exc.HTTPAccepted()
233+ return webob.Response(status_int=202)
234
235 @scheduler_api.redirect_handler
236 def get_vnc_console(self, req, id):
237@@ -384,7 +385,7 @@
238 int(id))
239 except exception.NotFound:
240 return faults.Fault(exc.HTTPNotFound())
241- return exc.HTTPAccepted()
242+ return webob.Response(status_int=202)
243
244 @scheduler_api.redirect_handler
245 def diagnostics(self, req, id):
246@@ -416,7 +417,7 @@
247 self.compute_api.delete(req.environ['nova.context'], id)
248 except exception.NotFound:
249 return faults.Fault(exc.HTTPNotFound())
250- return exc.HTTPAccepted()
251+ return webob.Response(status_int=202)
252
253 def _image_ref_from_req_data(self, data):
254 return data['server']['imageId']
255@@ -450,7 +451,7 @@
256 except Exception, e:
257 LOG.exception(_("Error in resize %s"), e)
258 return faults.Fault(exc.HTTPBadRequest())
259- return exc.HTTPAccepted()
260+ return webob.Response(status_int=202)
261
262 def _action_rebuild(self, info, request, instance_id):
263 context = request.environ['nova.context']
264@@ -470,9 +471,7 @@
265 LOG.debug(msg)
266 return faults.Fault(exc.HTTPConflict(explanation=msg))
267
268- response = exc.HTTPAccepted()
269- response.empty_body = True
270- return response
271+ return webob.Response(status_int=202)
272
273 def _get_server_admin_password(self, server):
274 """ Determine the admin password for a server on creation """
275@@ -519,7 +518,7 @@
276 msg = _("Invalid adminPass")
277 return exc.HTTPBadRequest(explanation=msg)
278 self.compute_api.set_admin_password(context, id, password)
279- return exc.HTTPAccepted()
280+ return webob.Response(status_int=202)
281
282 def _limit_items(self, items, req):
283 return common.limited_by_marker(items, req)
284@@ -565,7 +564,7 @@
285 except Exception, e:
286 LOG.exception(_("Error in resize %s"), e)
287 return faults.Fault(exc.HTTPBadRequest())
288- return exc.HTTPAccepted()
289+ return webob.Response(status_int=202)
290
291 def _action_rebuild(self, info, request, instance_id):
292 context = request.environ['nova.context']
293@@ -594,9 +593,7 @@
294 LOG.debug(msg)
295 return faults.Fault(exc.HTTPConflict(explanation=msg))
296
297- response = exc.HTTPAccepted()
298- response.empty_body = True
299- return response
300+ return webob.Response(status_int=202)
301
302 def get_default_xmlns(self, req):
303 return common.XML_NS_V11
304
305=== modified file 'nova/tests/api/openstack/test_faults.py'
306--- nova/tests/api/openstack/test_faults.py 2011-03-30 18:15:16 +0000
307+++ nova/tests/api/openstack/test_faults.py 2011-07-20 17:57:32 +0000
308@@ -139,3 +139,8 @@
309 self.assertEqual(resp.content_type, "application/xml")
310 self.assertEqual(resp.status_int, 404)
311 self.assertTrue('whut?' in resp.body)
312+
313+ def test_fault_has_status_int(self):
314+ """Ensure the status_int is set correctly on faults"""
315+ fault = faults.Fault(webob.exc.HTTPBadRequest(explanation='what?'))
316+ self.assertEqual(fault.status_int, 400)