Merge lp:~cerberus/nova/os-api-retune into lp:~hudson-openstack/nova/trunk

[Matt Dietz]

Changes and error fixes to help ensure basic parity with the Rackspace API. Some features are still missing, such as shared ip groups, and will be added in a later patch set.

[Eric Day (eday)]

So I see you renamed _entity_* to _translate_* in servers, but in other files you introduced the equivalent functions as _entity_*. It would be nice to have consistency in the naming, whichever one it is you prefer. :)

review: Needs Fixing

[Matt Dietz (cerberus)]

Yeah, I meant to get after that today. Thanks for the reminder :-)

[Rick Harris (rconradharris)]

Overall, lgtm.

> + inst_ref = self.compute_api.get_instance(ctxt, id)

This line is added in update(), but inst_ref isn't referenced later. Is this to trigger an exception as a side-effect? If that's the case, that might merit a comment.

[Todd Willey (xtoddx)]

lgtm

review: Approve

[Matt Dietz (cerberus)]

Rick: good catch. Superfluous line.

[lp:~cerberus/nova/os-api-retune]

500. By Matt Dietz on 2010-12-28

removed superfluous line

[Eric Day (eday)]

lgtm

review: Approve

=== modified file 'nova/api/openstack/__init__.py'
--- nova/api/openstack/__init__.py	2010-12-23 21:33:16 +0000
+++ nova/api/openstack/__init__.py	2010-12-28 23:36:26 +0000
@@ -100,7 +100,7 @@
                         collection={'detail': 'GET'},
                         member=server_members)
 
-        mapper.resource("backup_schedule", "backup_schedules",
+        mapper.resource("backup_schedule", "backup_schedule",
                         controller=backup_schedules.Controller(),
                         parent_resource=dict(member_name='server',
                         collection_name='servers'))
 
=== modified file 'nova/api/openstack/backup_schedules.py'
--- nova/api/openstack/backup_schedules.py	2010-12-22 20:59:53 +0000
+++ nova/api/openstack/backup_schedules.py	2010-12-28 23:36:26 +0000
@@ -23,13 +23,25 @@
 import nova.image.service
 
 
+def _translate_keys(inst):
+    """ Coerces the backup schedule into proper dictionary format """
+    return dict(backupSchedule=inst)
+
+
 class Controller(wsgi.Controller):
+    """ The backup schedule API controller for the Openstack API """
+
+    _serialization_metadata = {
+        'application/xml': {
+            'attributes': {
+                'backupSchedule': []}}}
 
     def __init__(self):
         pass
 
     def index(self, req, server_id):
-        return faults.Fault(exc.HTTPNotFound())
+        """ Returns the list of backup schedules for a given instance """
+        return _translate_keys({})
 
     def create(self, req, server_id):
         """ No actual update method required, since the existing API allows
@@ -37,4 +49,5 @@
         return faults.Fault(exc.HTTPNotFound())
 
     def delete(self, req, server_id, id):
+        """ Deletes an existing backup schedule """
         return faults.Fault(exc.HTTPNotFound())
 
=== modified file 'nova/api/openstack/ratelimiting/__init__.py'
--- nova/api/openstack/ratelimiting/__init__.py	2010-12-23 19:17:53 +0000
+++ nova/api/openstack/ratelimiting/__init__.py	2010-12-28 23:36:26 +0000
@@ -64,9 +64,9 @@
         If the request should be rate limited, return a 413 status with a
         Retry-After header giving the time when the request would succeed.
         """
-        return self.limited_request(req, self.application)
+        return self.rate_limited_request(req, self.application)
 
-    def limited_request(self, req, application):
+    def rate_limited_request(self, req, application):
         """Rate limit the request.
 
         If the request should be rate limited, return a 413 status with a
 
=== modified file 'nova/api/openstack/servers.py'
--- nova/api/openstack/servers.py	2010-12-23 21:33:16 +0000
+++ nova/api/openstack/servers.py	2010-12-28 23:36:26 +0000
@@ -35,14 +35,11 @@
 LOG.setLevel(logging.DEBUG)
 
 
-def _entity_list(entities):
-    """ Coerces a list of servers into proper dictionary format """
-    return dict(servers=entities)
-
-
-def _entity_detail(inst):
-    """ Maps everything to Rackspace-like attributes for return"""
+def _translate_detail_keys(inst):
+    """ Coerces into dictionary format, mapping everything to Rackspace-like
+    attributes for return"""
     power_mapping = {
+        None: 'build',
         power_state.NOSTATE: 'build',
         power_state.RUNNING: 'active',
         power_state.BLOCKED: 'active',
@@ -67,8 +64,9 @@
     return dict(server=inst_dict)
 
 
-def _entity_inst(inst):
-    """ Filters all model attributes save for id and name """
+def _translate_keys(inst):
+    """ Coerces into dictionary format, excluding all model attributes
+    save for id and name """
     return dict(server=dict(id=inst['internal_id'], name=inst['display_name']))
 
 
@@ -87,29 +85,29 @@
 
     def index(self, req):
         """ Returns a list of server names and ids for a given user """
-        return self._items(req, entity_maker=_entity_inst)
+        return self._items(req, entity_maker=_translate_keys)
 
     def detail(self, req):
         """ Returns a list of server details for a given user """
-        return self._items(req, entity_maker=_entity_detail)
+        return self._items(req, entity_maker=_translate_detail_keys)
 
     def _items(self, req, entity_maker):
         """Returns a list of servers for a given user.
 
-        entity_maker - either _entity_detail or _entity_inst
+        entity_maker - either _translate_detail_keys or _translate_keys
         """
         instance_list = self.compute_api.get_instances(
             req.environ['nova.context'])
         limited_list = common.limited(instance_list, req)
         res = [entity_maker(inst)['server'] for inst in limited_list]
-        return _entity_list(res)
+        return dict(servers=res)
 
     def show(self, req, id):
         """ Returns server details by server id """
         try:
             instance = self.compute_api.get_instance(
                 req.environ['nova.context'], int(id))
-            return _entity_detail(instance)
+            return _translate_detail_keys(instance)
         except exception.NotFound:
             return faults.Fault(exc.HTTPNotFound())
 
@@ -138,7 +136,7 @@
             description=env['server']['name'],
             key_name=key_pair['name'],
             key_data=key_pair['public_key'])
-        return _entity_inst(instances[0])
+        return _translate_keys(instances[0])
 
     def update(self, req, id):
         """ Updates the server name or password """
@@ -153,8 +151,9 @@
             update_dict['display_name'] = inst_dict['server']['name']
 
         try:
-            self.compute_api.update_instance(req.environ['nova.context'],
-                                             instance['id'],
+            ctxt = req.environ['nova.context']
+            self.compute_api.update_instance(ctxt,
+                                             id,
                                              **update_dict)
         except exception.NotFound:
             return faults.Fault(exc.HTTPNotFound())
 
=== modified file 'nova/api/openstack/sharedipgroups.py'
--- nova/api/openstack/sharedipgroups.py	2010-12-23 19:17:53 +0000
+++ nova/api/openstack/sharedipgroups.py	2010-12-28 23:36:26 +0000
@@ -15,26 +15,51 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
+from webob import exc
+
 from nova import wsgi
+from nova.api.openstack import faults
+
+
+def _translate_keys(inst):
+    """ Coerces a shared IP group instance into proper dictionary format """
+    return dict(sharedIpGroup=inst)
+
+
+def _translate_detail_keys(inst):
+    """ Coerces a shared IP group instance into proper dictionary format with
+    correctly mapped attributes """
+    return dict(sharedIpGroup=inst)
 
 
 class Controller(wsgi.Controller):
     """ The Shared IP Groups Controller for the Openstack API """
 
+    _serialization_metadata = {
+        'application/xml': {
+            'attributes': {
+                'sharedIpGroup': []}}}
+
     def index(self, req):
-        raise NotImplementedError
+        """ Returns a list of Shared IP Groups for the user """
+        return dict(sharedIpGroups=[])
 
     def show(self, req, id):
-        raise NotImplementedError
+        """ Shows in-depth information on a specific Shared IP Group """
+        return _translate_keys({})
 
     def update(self, req, id):
-        raise NotImplementedError
+        """ You can't update a Shared IP Group """
+        raise faults.Fault(exc.HTTPNotImplemented())
 
     def delete(self, req, id):
-        raise NotImplementedError
+        """ Deletes a Shared IP Group """
+        raise faults.Fault(exc.HTTPNotFound())
 
-    def detail(self, req):
-        raise NotImplementedError
+    def detail(self, req, id):
+        """ Returns a complete list of Shared IP Groups """
+        return _translate_detail_keys({})
 
     def create(self, req):
-        raise NotImplementedError
+        """ Creates a new Shared IP group """
+        raise faults.Fault(exc.HTTPNotFound())