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

Proposed by Matt Dietz
Status: Merged
Approved by: Eric Day
Approved revision: 500
Merged at revision: 502
Proposed branch: lp:~cerberus/nova/os-api-retune
Merge into: lp:~hudson-openstack/nova/trunk
Diff against target: 219 lines (+65/-28)
5 files modified
nova/api/openstack/__init__.py (+1/-1)
nova/api/openstack/backup_schedules.py (+14/-1)
nova/api/openstack/ratelimiting/__init__.py (+2/-2)
nova/api/openstack/servers.py (+16/-17)
nova/api/openstack/sharedipgroups.py (+32/-7)
To merge this branch: bzr merge lp:~cerberus/nova/os-api-retune
Reviewer Review Type Date Requested Status
Eric Day (community) Approve
Todd Willey (community) Approve
Review via email: mp+44763@code.launchpad.net

Description of the change

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.

To post a comment you must log in.
Revision history for this message
Eric Day (eday) wrote :

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
Revision history for this message
Matt Dietz (cerberus) wrote :

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

Revision history for this message
Rick Harris (rconradharris) wrote :

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.

Revision history for this message
Todd Willey (xtoddx) wrote :

lgtm

review: Approve
Revision history for this message
Matt Dietz (cerberus) wrote :

Rick: good catch. Superfluous line.

lp:~cerberus/nova/os-api-retune updated
500. By Matt Dietz

removed superfluous line

Revision history for this message
Eric Day (eday) 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/__init__.py'
2--- nova/api/openstack/__init__.py 2010-12-23 21:33:16 +0000
3+++ nova/api/openstack/__init__.py 2010-12-28 23:36:26 +0000
4@@ -100,7 +100,7 @@
5 collection={'detail': 'GET'},
6 member=server_members)
7
8- mapper.resource("backup_schedule", "backup_schedules",
9+ mapper.resource("backup_schedule", "backup_schedule",
10 controller=backup_schedules.Controller(),
11 parent_resource=dict(member_name='server',
12 collection_name='servers'))
13
14=== modified file 'nova/api/openstack/backup_schedules.py'
15--- nova/api/openstack/backup_schedules.py 2010-12-22 20:59:53 +0000
16+++ nova/api/openstack/backup_schedules.py 2010-12-28 23:36:26 +0000
17@@ -23,13 +23,25 @@
18 import nova.image.service
19
20
21+def _translate_keys(inst):
22+ """ Coerces the backup schedule into proper dictionary format """
23+ return dict(backupSchedule=inst)
24+
25+
26 class Controller(wsgi.Controller):
27+ """ The backup schedule API controller for the Openstack API """
28+
29+ _serialization_metadata = {
30+ 'application/xml': {
31+ 'attributes': {
32+ 'backupSchedule': []}}}
33
34 def __init__(self):
35 pass
36
37 def index(self, req, server_id):
38- return faults.Fault(exc.HTTPNotFound())
39+ """ Returns the list of backup schedules for a given instance """
40+ return _translate_keys({})
41
42 def create(self, req, server_id):
43 """ No actual update method required, since the existing API allows
44@@ -37,4 +49,5 @@
45 return faults.Fault(exc.HTTPNotFound())
46
47 def delete(self, req, server_id, id):
48+ """ Deletes an existing backup schedule """
49 return faults.Fault(exc.HTTPNotFound())
50
51=== modified file 'nova/api/openstack/ratelimiting/__init__.py'
52--- nova/api/openstack/ratelimiting/__init__.py 2010-12-23 19:17:53 +0000
53+++ nova/api/openstack/ratelimiting/__init__.py 2010-12-28 23:36:26 +0000
54@@ -64,9 +64,9 @@
55 If the request should be rate limited, return a 413 status with a
56 Retry-After header giving the time when the request would succeed.
57 """
58- return self.limited_request(req, self.application)
59+ return self.rate_limited_request(req, self.application)
60
61- def limited_request(self, req, application):
62+ def rate_limited_request(self, req, application):
63 """Rate limit the request.
64
65 If the request should be rate limited, return a 413 status with a
66
67=== modified file 'nova/api/openstack/servers.py'
68--- nova/api/openstack/servers.py 2010-12-23 21:33:16 +0000
69+++ nova/api/openstack/servers.py 2010-12-28 23:36:26 +0000
70@@ -35,14 +35,11 @@
71 LOG.setLevel(logging.DEBUG)
72
73
74-def _entity_list(entities):
75- """ Coerces a list of servers into proper dictionary format """
76- return dict(servers=entities)
77-
78-
79-def _entity_detail(inst):
80- """ Maps everything to Rackspace-like attributes for return"""
81+def _translate_detail_keys(inst):
82+ """ Coerces into dictionary format, mapping everything to Rackspace-like
83+ attributes for return"""
84 power_mapping = {
85+ None: 'build',
86 power_state.NOSTATE: 'build',
87 power_state.RUNNING: 'active',
88 power_state.BLOCKED: 'active',
89@@ -67,8 +64,9 @@
90 return dict(server=inst_dict)
91
92
93-def _entity_inst(inst):
94- """ Filters all model attributes save for id and name """
95+def _translate_keys(inst):
96+ """ Coerces into dictionary format, excluding all model attributes
97+ save for id and name """
98 return dict(server=dict(id=inst['internal_id'], name=inst['display_name']))
99
100
101@@ -87,29 +85,29 @@
102
103 def index(self, req):
104 """ Returns a list of server names and ids for a given user """
105- return self._items(req, entity_maker=_entity_inst)
106+ return self._items(req, entity_maker=_translate_keys)
107
108 def detail(self, req):
109 """ Returns a list of server details for a given user """
110- return self._items(req, entity_maker=_entity_detail)
111+ return self._items(req, entity_maker=_translate_detail_keys)
112
113 def _items(self, req, entity_maker):
114 """Returns a list of servers for a given user.
115
116- entity_maker - either _entity_detail or _entity_inst
117+ entity_maker - either _translate_detail_keys or _translate_keys
118 """
119 instance_list = self.compute_api.get_instances(
120 req.environ['nova.context'])
121 limited_list = common.limited(instance_list, req)
122 res = [entity_maker(inst)['server'] for inst in limited_list]
123- return _entity_list(res)
124+ return dict(servers=res)
125
126 def show(self, req, id):
127 """ Returns server details by server id """
128 try:
129 instance = self.compute_api.get_instance(
130 req.environ['nova.context'], int(id))
131- return _entity_detail(instance)
132+ return _translate_detail_keys(instance)
133 except exception.NotFound:
134 return faults.Fault(exc.HTTPNotFound())
135
136@@ -138,7 +136,7 @@
137 description=env['server']['name'],
138 key_name=key_pair['name'],
139 key_data=key_pair['public_key'])
140- return _entity_inst(instances[0])
141+ return _translate_keys(instances[0])
142
143 def update(self, req, id):
144 """ Updates the server name or password """
145@@ -153,8 +151,9 @@
146 update_dict['display_name'] = inst_dict['server']['name']
147
148 try:
149- self.compute_api.update_instance(req.environ['nova.context'],
150- instance['id'],
151+ ctxt = req.environ['nova.context']
152+ self.compute_api.update_instance(ctxt,
153+ id,
154 **update_dict)
155 except exception.NotFound:
156 return faults.Fault(exc.HTTPNotFound())
157
158=== modified file 'nova/api/openstack/sharedipgroups.py'
159--- nova/api/openstack/sharedipgroups.py 2010-12-23 19:17:53 +0000
160+++ nova/api/openstack/sharedipgroups.py 2010-12-28 23:36:26 +0000
161@@ -15,26 +15,51 @@
162 # License for the specific language governing permissions and limitations
163 # under the License.
164
165+from webob import exc
166+
167 from nova import wsgi
168+from nova.api.openstack import faults
169+
170+
171+def _translate_keys(inst):
172+ """ Coerces a shared IP group instance into proper dictionary format """
173+ return dict(sharedIpGroup=inst)
174+
175+
176+def _translate_detail_keys(inst):
177+ """ Coerces a shared IP group instance into proper dictionary format with
178+ correctly mapped attributes """
179+ return dict(sharedIpGroup=inst)
180
181
182 class Controller(wsgi.Controller):
183 """ The Shared IP Groups Controller for the Openstack API """
184
185+ _serialization_metadata = {
186+ 'application/xml': {
187+ 'attributes': {
188+ 'sharedIpGroup': []}}}
189+
190 def index(self, req):
191- raise NotImplementedError
192+ """ Returns a list of Shared IP Groups for the user """
193+ return dict(sharedIpGroups=[])
194
195 def show(self, req, id):
196- raise NotImplementedError
197+ """ Shows in-depth information on a specific Shared IP Group """
198+ return _translate_keys({})
199
200 def update(self, req, id):
201- raise NotImplementedError
202+ """ You can't update a Shared IP Group """
203+ raise faults.Fault(exc.HTTPNotImplemented())
204
205 def delete(self, req, id):
206- raise NotImplementedError
207+ """ Deletes a Shared IP Group """
208+ raise faults.Fault(exc.HTTPNotFound())
209
210- def detail(self, req):
211- raise NotImplementedError
212+ def detail(self, req, id):
213+ """ Returns a complete list of Shared IP Groups """
214+ return _translate_detail_keys({})
215
216 def create(self, req):
217- raise NotImplementedError
218+ """ Creates a new Shared IP group """
219+ raise faults.Fault(exc.HTTPNotFound())