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

[Brian Waldon]

- fixes bug that prevented custom wsgi serialization

[Devin Carlen (devcamcar)]

lgtm

review: Approve

[Brian Lamar (blamar)]

Seems like this could be simplified:

9 + if action is None:
10 + action_method = self.default
11 + else:
12 + action_method = getattr(self, action, self.default)

Wouldn't str(action) do just as well? The case you're checking for is someone explicitly passing in None as the action parameter, but wouldn't any object also break getattr?

9 + return getattr(self, str(action), self.default)(data)

Tests?

review: Needs Fixing

[Brian Waldon (bcwaldon)]

> Wouldn't str(action) do just as well? The case you're checking for is someone
> explicitly passing in None as the action parameter, but wouldn't any object
> also break getattr?
>
> 9 + return getattr(self, str(action), self.default)(data)

Yeah, you're right. Solving this generically is better than explicitly checking for None. Fixed.

> Tests?

Yeah, I added some more to send an action with None.

[Brian Lamar (blamar)]

Looks good, going to run tests and smokestack before approval.

review: Approve

[Trey Morris (tr3buchet)]

blamar: did you review-approve, then plan to run the tests and such, and then afterwards change the status to approved?
I worry someone may see two core dev approves and mark it approved out from under you

[Brian Lamar (blamar)]

Smokestacked and tests pass, approving.

[OpenStack Infra (hudson-openstack)]

The attempt to merge lp:~rackspace-titan/nova/lp795772 into lp:nova failed. Below is the output from the failed tests.

AccountsTest
    test_account_create OK
    test_account_delete OK
    test_account_update OK
    test_get_account OK
AdminAPITest
    test_admin_disabled OK
    test_admin_enabled OK
APITest
    test_exceptions_are_converted_to_faults OK
Test
    test_authorize_project OK
    test_authorize_token OK
    test_authorize_user OK
    test_bad_project OK
    test_bad_token OK
    test_bad_user_bad_key OK
    test_bad_user_good_key OK
    test_no_user OK
    test_not_existing_project OK
    test_token_expiry OK
TestFunctional
    test_token_doesnotexist OK
    test_token_expiry OK
TestLimiter
    test_authorize_token OK
LimiterTest
    test_limiter_custom_max_limit OK
    test_limiter_limit_and_offset OK
    test_limiter_limit_medium OK
    test_limiter_limit_over_max OK
    test_limiter_limit_zero OK
    test_limiter_negative_limit OK
    test_limiter_negative_offset OK
    test_limiter_nothing OK
    test_limiter_offset_bad OK
    test_limiter_offset_blank OK
    test_limiter_offset_medium OK
    test_limiter_offset_over_max OK
    test_limiter_offset_zero OK
PaginationParamsTest
    test_invalid_limit OK
    test_invalid_marker OK
    test_no_params OK
    test_valid_limit OK
    test_valid_marker OK
ActionExtensionTest
    test_extended_action OK
    test_invalid_action OK
    test_invalid_action_body OK
ExtensionControllerTest
    test_get_by_alias OK
    test_index OK
ExtensionManagerTest
    test_get_resources ...

[lp:~rackspace-titan/nova/lp795772]

1173. By Brian Waldon on 2011-06-13

merging trunk, fixing pep8

[Brian Waldon (bcwaldon)]

I swear I ran pep8!

Fixed now.

=== modified file 'nova/api/openstack/wsgi.py'
--- nova/api/openstack/wsgi.py	2011-06-13 12:54:21 +0000
+++ nova/api/openstack/wsgi.py	2011-06-13 20:01:23 +0000
@@ -60,7 +60,7 @@
 
     def deserialize(self, datastring, action='default'):
         """Find local deserialization method and parse request body."""
-        action_method = getattr(self, action, self.default)
+        action_method = getattr(self, str(action), self.default)
         return action_method(datastring)
 
     def default(self, datastring):
@@ -189,7 +189,7 @@
 
     def serialize(self, data, action='default'):
         """Find local serialization method and encode response body."""
-        action_method = getattr(self, action, self.default)
+        action_method = getattr(self, str(action), self.default)
         return action_method(data)
 
     def default(self, data):
@@ -296,7 +296,7 @@
         }
         self.serializers.update(serializers or {})
 
-    def serialize(self, response_data, content_type):
+    def serialize(self, response_data, content_type, action='default'):
         """Serialize a dict into a string and wrap in a wsgi.Request object.
 
         :param response_data: dict produced by the Controller
@@ -307,7 +307,7 @@
         response.headers['Content-Type'] = content_type
 
         serializer = self.get_serializer(content_type)
-        response.body = serializer.serialize(response_data)
+        response.body = serializer.serialize(response_data, action)
 
         return response
 
@@ -358,7 +358,7 @@
 
         #TODO(bcwaldon): find a more elegant way to pass through non-dict types
         if type(action_result) is dict:
-            response = self.serializer.serialize(action_result, accept)
+            response = self.serializer.serialize(action_result, accept, action)
         else:
             response = action_result
 
 
=== modified file 'nova/tests/api/openstack/test_wsgi.py'
--- nova/tests/api/openstack/test_wsgi.py	2011-05-26 18:09:59 +0000
+++ nova/tests/api/openstack/test_wsgi.py	2011-06-13 20:01:23 +0000
@@ -89,6 +89,12 @@
         serializer.default = lambda x: 'trousers'
         self.assertEqual(serializer.serialize({}, 'update'), 'trousers')
 
+    def test_dispatch_action_None(self):
+        serializer = wsgi.DictSerializer()
+        serializer.create = lambda x: 'pants'
+        serializer.default = lambda x: 'trousers'
+        self.assertEqual(serializer.serialize({}, None), 'trousers')
+
 
 class XMLDictSerializerTest(test.TestCase):
     def test_xml(self):
@@ -123,6 +129,12 @@
         deserializer.default = lambda x: 'trousers'
         self.assertEqual(deserializer.deserialize({}, 'update'), 'trousers')
 
+    def test_dispatch_action_None(self):
+        deserializer = wsgi.TextDeserializer()
+        deserializer.create = lambda x: 'pants'
+        deserializer.default = lambda x: 'trousers'
+        self.assertEqual(deserializer.deserialize({}, None), 'trousers')
+
 
 class JSONDeserializerTest(test.TestCase):
     def test_json(self):
@@ -171,11 +183,11 @@
 class ResponseSerializerTest(test.TestCase):
     def setUp(self):
         class JSONSerializer(object):
-            def serialize(self, data):
+            def serialize(self, data, action='default'):
                 return 'pew_json'
 
         class XMLSerializer(object):
-            def serialize(self, data):
+            def serialize(self, data, action='default'):
                 return 'pew_xml'
 
         self.serializers = {
@@ -211,11 +223,11 @@
 class RequestDeserializerTest(test.TestCase):
     def setUp(self):
         class JSONDeserializer(object):
-            def deserialize(self, data):
+            def deserialize(self, data, action='default'):
                 return 'pew_json'
 
         class XMLDeserializer(object):
-            def deserialize(self, data):
+            def deserialize(self, data, action='default'):
                 return 'pew_xml'
 
         self.deserializers = {