OpenStack Compute (nova)

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

  1. lp802621
  2. Merge into trunk
Proposed by William Wolf
Status: Work in progress
Proposed branch: lp://qastaging/~rackspace-titan/nova/lp802621
Merge into: lp://qastaging/~hudson-openstack/nova/trunk
Diff against target: 120 lines (+22/-9)
5 files modified
nova/api/openstack/images.py (+5/-1)
nova/api/openstack/servers.py (+4/-1)
nova/api/openstack/wsgi.py (+6/-0)
nova/tests/api/openstack/test_images.py (+4/-4)
nova/tests/api/openstack/test_servers.py (+3/-3)
To merge this branch: bzr merge lp://qastaging/~rackspace-titan/nova/lp802621
Reviewer Review Type Date Requested Status
Dan Prince (community) Needs Fixing
Alex Meade (community) Approve
Mark Washenberger (community) Approve
Review via email: mp+66029@code.qastaging.launchpad.net

Description of the change

I was looking for other examples in the code where we return 202 status code, and it seems we are doing it by returning webob.exc.HTTPAccepted() object. The problem was that nowhere else that we use that needs to return a response body as well, so I've added a response.body_obj property to my responses, and I check for that property. If the property is there, I will serialize it accordingly and attach it to the response body. If it isn't there (most cases for now), it'll do just what it did before.

This seemed the least invasive way to do this for now, let me know what you think.

To post a comment you must log in.
Revision history for this message
Jay Pipes (jaypipes) wrote :

"I was looking for other examples in the code where we return 202 status code, and it seems we are doing it by returning webob.exc.HTTPAccepted() object. The problem was that nowhere else that we use that needs to return a response body as well"

Well, all the places where we are returning a 202, we *should* be returning an entity that describes the status of the request. See: http://www.w3.org/Protocols/rfc2616/rfc2616-sec10.html.

-jay

Reply
Revision history for this message
William Wolf (throughnothing) wrote :

>
> Well, all the places where we are returning a 202, we *should* be returning an
> entity that describes the status of the request. See:
> http://www.w3.org/Protocols/rfc2616/rfc2616-sec10.html.

Jay, I couldn't agree more :) Doesn't look like what we're actually doing anywhere else (that I could find....please prove me wrong :)). As for this merge prop, this at least fixes create server, and im gonna do a similar merge prop shortly to fix create image requests.

Reply
lp://qastaging/~rackspace-titan/nova/lp802621 updated
1218. By William Wolf

fix image create response codes also

Revision history for this message
William Wolf (throughnothing) wrote :

I put the same changes for image create in this merge prop also, I can separate it out to a separate merge prop if needed, but it requires the same piece of code in api.openstack.wsgi

Reply
Revision history for this message
Jay Pipes (jaypipes) wrote :

Heya, you might want to chat with Brian Waldon about the work he's done in refactoring the Glance WSGI layer. He resolved the problem you are dealing with here by making the Controllers only return Python dictionaries and not having any interaction with the webob.Response object. Just a suggestion. Brian can elaborate, of course :)
-jay

Reply
Revision history for this message
Brian Waldon (bcwaldon) wrote :

> Heya, you might want to chat with Brian Waldon about the work he's done in
> refactoring the Glance WSGI layer. He resolved the problem you are dealing
> with here by making the Controllers only return Python dictionaries and not
> having any interaction with the webob.Response object. Just a suggestion.
> Brian can elaborate, of course :)

I think this is an acceptable short-term fix, but I definitely want to get the new wsgi stuff in here.

Reply
Revision history for this message
Mark Washenberger (markwash) wrote :

This merge smokestacks fine and is a suitable fix for the time being. Go working tests!

review: Approve
Reply
Revision history for this message
Alex Meade (alex-meade) wrote :

I like this, lgtm

review: Approve
Reply
Revision history for this message
Dan Prince (dan-prince) wrote :

Hi Will,

Man. I really wish didn't use exceptions when returning 202 responses. What you have here looks reasonable though.

I am getting the following conflicts with lp:nova:

Text conflict in nova/api/openstack/images.py
Text conflict in nova/tests/api/openstack/test_images.py

review: Needs Fixing
Reply

Unmerged revisions

1218. By William Wolf

fix image create response codes also

1217. By William Wolf

pass action to serializer

1216. By William Wolf

make create server requests return prope 202 instead of 200 response code

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.