Merge lp:~fwierzbicki/txaws/break-out-ec2-parser into lp:txaws
- break-out-ec2-parser
- Merge into trunk
Status: | Merged | ||||
---|---|---|---|---|---|
Approved by: | Thomas Herve | ||||
Approved revision: | 87 | ||||
Merged at revision: | 85 | ||||
Proposed branch: | lp:~fwierzbicki/txaws/break-out-ec2-parser | ||||
Merge into: | lp:txaws | ||||
Diff against target: |
975 lines (+405/-318) 5 files modified
txaws/client/base.py (+4/-1) txaws/client/tests/test_client.py (+2/-1) txaws/ec2/client.py (+396/-314) txaws/ec2/tests/test_client.py (+1/-1) txaws/testing/ec2.py (+2/-1) |
||||
To merge this branch: | bzr merge lp:~fwierzbicki/txaws/break-out-ec2-parser | ||||
Related bugs: |
|
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Thomas Herve | Approve | ||
Duncan McGreggor | Approve | ||
Review via email: mp+58623@code.launchpad.net |
Commit message
Description of the change
This branch breaks out the private parsing methods from EC2Client into a separate Parser class with public methods so that they can be safely overridden. It also adds docstrings to the parser functions that lacked them.
Frank Wierzbicki (fwierzbicki) wrote : | # |
> Man, I can't tell you how badly I've wanted this... since about the second or
> third week Thomas and I were working on the ec2 client support!
>
> 1) Overall, looks awesome -- nice work, and thanks :-)
No problem! It will make my cloud deck work nicer :)
> 2) The Parser class seems to have a superfluous __init__; I'd just recommend
> removing it.
Fixed.
> 3) This is a nit... totally up to you, but now that the parse_* methods are in
> their own Parse class, I'd just remove the "parse_" from each method name.
> With them in there, it looks like C code ;-)
Fixed.
- 85. By Frank Wierzbicki
-
Remove parse_ from Parser methods, remove unneeded __init__.
- 86. By Frank Wierzbicki
-
Merge with trunk.
- 87. By Frank Wierzbicki
-
Add parser parameter to FakeEC2Client.
Thomas Herve (therve) wrote : | # |
[1] Please update the __all__ attribute of client.py with Query and Parser.
It would have been nice to have direct testing of the Parser class, but it will do it for now. Nice branch, +1!
- 88. By Frank Wierzbicki
-
Update __all__
Preview Diff
1 | === modified file 'txaws/client/base.py' |
2 | --- txaws/client/base.py 2011-04-21 14:59:23 +0000 |
3 | +++ txaws/client/base.py 2011-04-26 16:30:59 +0000 |
4 | @@ -57,8 +57,10 @@ |
5 | @param endpoint: The service endpoint URI. |
6 | @param query_factory: The class or function that produces a query |
7 | object for making requests to the EC2 service. |
8 | + @param parser: A parser object for parsing responses from the EC2 service. |
9 | """ |
10 | - def __init__(self, creds=None, endpoint=None, query_factory=None): |
11 | + def __init__(self, creds=None, endpoint=None, query_factory=None, |
12 | + parser=None): |
13 | if creds is None: |
14 | creds = AWSCredentials() |
15 | if endpoint is None: |
16 | @@ -66,6 +68,7 @@ |
17 | self.creds = creds |
18 | self.endpoint = endpoint |
19 | self.query_factory = query_factory |
20 | + self.parser = parser |
21 | |
22 | |
23 | class BaseQuery(object): |
24 | |
25 | === modified file 'txaws/client/tests/test_client.py' |
26 | --- txaws/client/tests/test_client.py 2011-04-21 16:40:58 +0000 |
27 | +++ txaws/client/tests/test_client.py 2011-04-26 16:30:59 +0000 |
28 | @@ -53,10 +53,11 @@ |
29 | class BaseClientTestCase(TXAWSTestCase): |
30 | |
31 | def test_creation(self): |
32 | - client = BaseClient("creds", "endpoint", "query factory") |
33 | + client = BaseClient("creds", "endpoint", "query factory", "parser") |
34 | self.assertEquals(client.creds, "creds") |
35 | self.assertEquals(client.endpoint, "endpoint") |
36 | self.assertEquals(client.query_factory, "query factory") |
37 | + self.assertEquals(client.parser, "parser") |
38 | |
39 | |
40 | class BaseQueryTestCase(TXAWSTestCase): |
41 | |
42 | === modified file 'txaws/ec2/client.py' |
43 | --- txaws/ec2/client.py 2011-04-21 13:10:37 +0000 |
44 | +++ txaws/ec2/client.py 2011-04-26 16:30:59 +0000 |
45 | @@ -16,7 +16,7 @@ |
46 | from txaws.util import iso8601time, XML |
47 | |
48 | |
49 | -__all__ = ["EC2Client"] |
50 | +__all__ = ["EC2Client", "Query", "Parser"] |
51 | |
52 | |
53 | def ec2_error_wrapper(error): |
54 | @@ -26,10 +26,13 @@ |
55 | class EC2Client(BaseClient): |
56 | """A client for EC2.""" |
57 | |
58 | - def __init__(self, creds=None, endpoint=None, query_factory=None): |
59 | + def __init__(self, creds=None, endpoint=None, query_factory=None, |
60 | + parser=None): |
61 | if query_factory is None: |
62 | query_factory = Query |
63 | - super(EC2Client, self).__init__(creds, endpoint, query_factory) |
64 | + if parser is None: |
65 | + parser = Parser() |
66 | + super(EC2Client, self).__init__(creds, endpoint, query_factory, parser) |
67 | |
68 | def describe_instances(self, *instance_ids): |
69 | """Describe current instances.""" |
70 | @@ -40,91 +43,7 @@ |
71 | action="DescribeInstances", creds=self.creds, |
72 | endpoint=self.endpoint, other_params=instances) |
73 | d = query.submit() |
74 | - return d.addCallback(self._parse_describe_instances) |
75 | - |
76 | - def _parse_instances_set(self, root, reservation): |
77 | - """Parse instance data out of an XML payload. |
78 | - |
79 | - @param root: The root node of the XML payload. |
80 | - @param reservation: The L{Reservation} associated with the instances |
81 | - from the response. |
82 | - @return: A C{list} of L{Instance}s. |
83 | - """ |
84 | - instances = [] |
85 | - for instance_data in root.find("instancesSet"): |
86 | - instances.append(self._parse_instance(instance_data, reservation)) |
87 | - return instances |
88 | - |
89 | - def _parse_instance(self, instance_data, reservation): |
90 | - """Parse instance data out of an XML payload. |
91 | - |
92 | - @param instance_data: An XML node containing instance data. |
93 | - @param reservation: The L{Reservation} associated with the instance. |
94 | - @return: An L{Instance}. |
95 | - """ |
96 | - instance_id = instance_data.findtext("instanceId") |
97 | - instance_state = instance_data.find( |
98 | - "instanceState").findtext("name") |
99 | - instance_type = instance_data.findtext("instanceType") |
100 | - image_id = instance_data.findtext("imageId") |
101 | - private_dns_name = instance_data.findtext("privateDnsName") |
102 | - dns_name = instance_data.findtext("dnsName") |
103 | - key_name = instance_data.findtext("keyName") |
104 | - ami_launch_index = instance_data.findtext("amiLaunchIndex") |
105 | - launch_time = instance_data.findtext("launchTime") |
106 | - placement = instance_data.find("placement").findtext( |
107 | - "availabilityZone") |
108 | - products = [] |
109 | - product_codes = instance_data.find("productCodes") |
110 | - if product_codes is not None: |
111 | - for product_data in instance_data.find("productCodes"): |
112 | - products.append(product_data.text) |
113 | - kernel_id = instance_data.findtext("kernelId") |
114 | - ramdisk_id = instance_data.findtext("ramdiskId") |
115 | - instance = model.Instance( |
116 | - instance_id, instance_state, instance_type, image_id, |
117 | - private_dns_name, dns_name, key_name, ami_launch_index, |
118 | - launch_time, placement, products, kernel_id, ramdisk_id, |
119 | - reservation=reservation) |
120 | - return instance |
121 | - |
122 | - def _parse_describe_instances(self, xml_bytes): |
123 | - """ |
124 | - Parse the reservations XML payload that is returned from an AWS |
125 | - describeInstances API call. |
126 | - |
127 | - Instead of returning the reservations as the "top-most" object, we |
128 | - return the object that most developers and their code will be |
129 | - interested in: the instances. In instances reservation is available on |
130 | - the instance object. |
131 | - |
132 | - The following instance attributes are optional: |
133 | - * ami_launch_index |
134 | - * key_name |
135 | - * kernel_id |
136 | - * product_codes |
137 | - * ramdisk_id |
138 | - * reason |
139 | - """ |
140 | - root = XML(xml_bytes) |
141 | - results = [] |
142 | - # May be a more elegant way to do this: |
143 | - for reservation_data in root.find("reservationSet"): |
144 | - # Get the security group information. |
145 | - groups = [] |
146 | - for group_data in reservation_data.find("groupSet"): |
147 | - group_id = group_data.findtext("groupId") |
148 | - groups.append(group_id) |
149 | - # Create a reservation object with the parsed data. |
150 | - reservation = model.Reservation( |
151 | - reservation_id=reservation_data.findtext("reservationId"), |
152 | - owner_id=reservation_data.findtext("ownerId"), |
153 | - groups=groups) |
154 | - # Get the list of instances. |
155 | - instances = self._parse_instances_set( |
156 | - reservation_data, reservation) |
157 | - results.extend(instances) |
158 | - return results |
159 | + return d.addCallback(self.parser.describe_instances) |
160 | |
161 | def run_instances(self, image_id, min_count, max_count, |
162 | security_groups=None, key_name=None, instance_type=None, |
163 | @@ -152,27 +71,7 @@ |
164 | action="RunInstances", creds=self.creds, endpoint=self.endpoint, |
165 | other_params=params) |
166 | d = query.submit() |
167 | - return d.addCallback(self._parse_run_instances) |
168 | - |
169 | - def _parse_run_instances(self, xml_bytes): |
170 | - """ |
171 | - Parse the reservations XML payload that is returned from an AWS |
172 | - RunInstances API call. |
173 | - """ |
174 | - root = XML(xml_bytes) |
175 | - # Get the security group information. |
176 | - groups = [] |
177 | - for group_data in root.find("groupSet"): |
178 | - group_id = group_data.findtext("groupId") |
179 | - groups.append(group_id) |
180 | - # Create a reservation object with the parsed data. |
181 | - reservation = model.Reservation( |
182 | - reservation_id=root.findtext("reservationId"), |
183 | - owner_id=root.findtext("ownerId"), |
184 | - groups=groups) |
185 | - # Get the list of instances. |
186 | - instances = self._parse_instances_set(root, reservation) |
187 | - return instances |
188 | + return d.addCallback(self.parser.run_instances) |
189 | |
190 | def terminate_instances(self, *instance_ids): |
191 | """Terminate some instances. |
192 | @@ -188,20 +87,7 @@ |
193 | action="TerminateInstances", creds=self.creds, |
194 | endpoint=self.endpoint, other_params=instances) |
195 | d = query.submit() |
196 | - return d.addCallback(self._parse_terminate_instances) |
197 | - |
198 | - def _parse_terminate_instances(self, xml_bytes): |
199 | - root = XML(xml_bytes) |
200 | - result = [] |
201 | - # May be a more elegant way to do this: |
202 | - for instance in root.find("instancesSet"): |
203 | - instanceId = instance.findtext("instanceId") |
204 | - previousState = instance.find("previousState").findtext( |
205 | - "name") |
206 | - shutdownState = instance.find("shutdownState").findtext( |
207 | - "name") |
208 | - result.append((instanceId, previousState, shutdownState)) |
209 | - return result |
210 | + return d.addCallback(self.parser.terminate_instances) |
211 | |
212 | def describe_security_groups(self, *names): |
213 | """Describe security groups. |
214 | @@ -219,50 +105,7 @@ |
215 | action="DescribeSecurityGroups", creds=self.creds, |
216 | endpoint=self.endpoint, other_params=group_names) |
217 | d = query.submit() |
218 | - return d.addCallback(self._parse_describe_security_groups) |
219 | - |
220 | - def _parse_describe_security_groups(self, xml_bytes): |
221 | - """Parse the XML returned by the C{DescribeSecurityGroups} function. |
222 | - |
223 | - @param xml_bytes: XML bytes with a C{DescribeSecurityGroupsResponse} |
224 | - root element. |
225 | - @return: A list of L{SecurityGroup} instances. |
226 | - """ |
227 | - root = XML(xml_bytes) |
228 | - result = [] |
229 | - for group_info in root.findall("securityGroupInfo/item"): |
230 | - name = group_info.findtext("groupName") |
231 | - description = group_info.findtext("groupDescription") |
232 | - owner_id = group_info.findtext("ownerId") |
233 | - allowed_groups = [] |
234 | - allowed_ips = [] |
235 | - ip_permissions = group_info.find("ipPermissions") |
236 | - if ip_permissions is None: |
237 | - ip_permissions = () |
238 | - for ip_permission in ip_permissions: |
239 | - ip_protocol = ip_permission.findtext("ipProtocol") |
240 | - from_port = int(ip_permission.findtext("fromPort")) |
241 | - to_port = int(ip_permission.findtext("toPort")) |
242 | - for groups in ip_permission.findall("groups/item") or (): |
243 | - user_id = groups.findtext("userId") |
244 | - group_name = groups.findtext("groupName") |
245 | - if user_id and group_name: |
246 | - if (user_id, group_name) not in allowed_groups: |
247 | - allowed_groups.append((user_id, group_name)) |
248 | - for ip_ranges in ip_permission.findall("ipRanges/item") or (): |
249 | - cidr_ip = ip_ranges.findtext("cidrIp") |
250 | - allowed_ips.append( |
251 | - model.IPPermission( |
252 | - ip_protocol, from_port, to_port, cidr_ip)) |
253 | - |
254 | - allowed_groups = [model.UserIDGroupPair(user_id, group_name) |
255 | - for user_id, group_name in allowed_groups] |
256 | - |
257 | - security_group = model.SecurityGroup( |
258 | - name, description, owner_id=owner_id, |
259 | - groups=allowed_groups, ips=allowed_ips) |
260 | - result.append(security_group) |
261 | - return result |
262 | + return d.addCallback(self.parser.describe_security_groups) |
263 | |
264 | def create_security_group(self, name, description): |
265 | """Create security group. |
266 | @@ -277,11 +120,7 @@ |
267 | action="CreateSecurityGroup", creds=self.creds, |
268 | endpoint=self.endpoint, other_params=parameters) |
269 | d = query.submit() |
270 | - return d.addCallback(self._parse_truth_return) |
271 | - |
272 | - def _parse_truth_return(self, xml_bytes): |
273 | - root = XML(xml_bytes) |
274 | - return root.findtext("return") == "true" |
275 | + return d.addCallback(self.parser.truth_return) |
276 | |
277 | def delete_security_group(self, name): |
278 | """ |
279 | @@ -294,7 +133,7 @@ |
280 | action="DeleteSecurityGroup", creds=self.creds, |
281 | endpoint=self.endpoint, other_params=parameter) |
282 | d = query.submit() |
283 | - return d.addCallback(self._parse_truth_return) |
284 | + return d.addCallback(self.parser.truth_return) |
285 | |
286 | def authorize_security_group( |
287 | self, group_name, source_group_name="", source_group_owner_id="", |
288 | @@ -351,7 +190,7 @@ |
289 | action="AuthorizeSecurityGroupIngress", creds=self.creds, |
290 | endpoint=self.endpoint, other_params=parameters) |
291 | d = query.submit() |
292 | - return d.addCallback(self._parse_truth_return) |
293 | + return d.addCallback(self.parser.truth_return) |
294 | |
295 | def authorize_group_permission( |
296 | self, group_name, source_group_name, source_group_owner_id): |
297 | @@ -436,7 +275,7 @@ |
298 | action="RevokeSecurityGroupIngress", creds=self.creds, |
299 | endpoint=self.endpoint, other_params=parameters) |
300 | d = query.submit() |
301 | - return d.addCallback(self._parse_truth_return) |
302 | + return d.addCallback(self.parser.truth_return) |
303 | |
304 | def revoke_group_permission( |
305 | self, group_name, source_group_name, source_group_owner_id): |
306 | @@ -475,35 +314,7 @@ |
307 | action="DescribeVolumes", creds=self.creds, endpoint=self.endpoint, |
308 | other_params=volumeset) |
309 | d = query.submit() |
310 | - return d.addCallback(self._parse_describe_volumes) |
311 | - |
312 | - def _parse_describe_volumes(self, xml_bytes): |
313 | - root = XML(xml_bytes) |
314 | - result = [] |
315 | - for volume_data in root.find("volumeSet"): |
316 | - volume_id = volume_data.findtext("volumeId") |
317 | - size = int(volume_data.findtext("size")) |
318 | - status = volume_data.findtext("status") |
319 | - availability_zone = volume_data.findtext("availabilityZone") |
320 | - snapshot_id = volume_data.findtext("snapshotId") |
321 | - create_time = volume_data.findtext("createTime") |
322 | - create_time = datetime.strptime( |
323 | - create_time[:19], "%Y-%m-%dT%H:%M:%S") |
324 | - volume = model.Volume( |
325 | - volume_id, size, status, create_time, availability_zone, |
326 | - snapshot_id) |
327 | - result.append(volume) |
328 | - for attachment_data in volume_data.find("attachmentSet"): |
329 | - instance_id = attachment_data.findtext("instanceId") |
330 | - status = attachment_data.findtext("status") |
331 | - device = attachment_data.findtext("device") |
332 | - attach_time = attachment_data.findtext("attachTime") |
333 | - attach_time = datetime.strptime( |
334 | - attach_time[:19], "%Y-%m-%dT%H:%M:%S") |
335 | - attachment = model.Attachment( |
336 | - instance_id, device, status, attach_time) |
337 | - volume.attachments.append(attachment) |
338 | - return result |
339 | + return d.addCallback(self.parser.describe_volumes) |
340 | |
341 | def create_volume(self, availability_zone, size=None, snapshot_id=None): |
342 | """Create a new volume.""" |
343 | @@ -519,29 +330,14 @@ |
344 | action="CreateVolume", creds=self.creds, endpoint=self.endpoint, |
345 | other_params=params) |
346 | d = query.submit() |
347 | - return d.addCallback(self._parse_create_volume) |
348 | - |
349 | - def _parse_create_volume(self, xml_bytes): |
350 | - root = XML(xml_bytes) |
351 | - volume_id = root.findtext("volumeId") |
352 | - size = int(root.findtext("size")) |
353 | - status = root.findtext("status") |
354 | - create_time = root.findtext("createTime") |
355 | - availability_zone = root.findtext("availabilityZone") |
356 | - snapshot_id = root.findtext("snapshotId") |
357 | - create_time = datetime.strptime( |
358 | - create_time[:19], "%Y-%m-%dT%H:%M:%S") |
359 | - volume = model.Volume( |
360 | - volume_id, size, status, create_time, availability_zone, |
361 | - snapshot_id) |
362 | - return volume |
363 | + return d.addCallback(self.parser.create_volume) |
364 | |
365 | def delete_volume(self, volume_id): |
366 | query = self.query_factory( |
367 | action="DeleteVolume", creds=self.creds, endpoint=self.endpoint, |
368 | other_params={"VolumeId": volume_id}) |
369 | d = query.submit() |
370 | - return d.addCallback(self._parse_truth_return) |
371 | + return d.addCallback(self.parser.truth_return) |
372 | |
373 | def describe_snapshots(self, *snapshot_ids): |
374 | """Describe available snapshots.""" |
375 | @@ -552,24 +348,7 @@ |
376 | action="DescribeSnapshots", creds=self.creds, |
377 | endpoint=self.endpoint, other_params=snapshot_set) |
378 | d = query.submit() |
379 | - return d.addCallback(self._parse_snapshots) |
380 | - |
381 | - def _parse_snapshots(self, xml_bytes): |
382 | - root = XML(xml_bytes) |
383 | - result = [] |
384 | - for snapshot_data in root.find("snapshotSet"): |
385 | - snapshot_id = snapshot_data.findtext("snapshotId") |
386 | - volume_id = snapshot_data.findtext("volumeId") |
387 | - status = snapshot_data.findtext("status") |
388 | - start_time = snapshot_data.findtext("startTime") |
389 | - start_time = datetime.strptime( |
390 | - start_time[:19], "%Y-%m-%dT%H:%M:%S") |
391 | - progress = snapshot_data.findtext("progress")[:-1] |
392 | - progress = float(progress or "0") / 100. |
393 | - snapshot = model.Snapshot( |
394 | - snapshot_id, volume_id, status, start_time, progress) |
395 | - result.append(snapshot) |
396 | - return result |
397 | + return d.addCallback(self.parser.snapshots) |
398 | |
399 | def create_snapshot(self, volume_id): |
400 | """Create a new snapshot of an existing volume.""" |
401 | @@ -577,20 +356,7 @@ |
402 | action="CreateSnapshot", creds=self.creds, endpoint=self.endpoint, |
403 | other_params={"VolumeId": volume_id}) |
404 | d = query.submit() |
405 | - return d.addCallback(self._parse_create_snapshot) |
406 | - |
407 | - def _parse_create_snapshot(self, xml_bytes): |
408 | - root = XML(xml_bytes) |
409 | - snapshot_id = root.findtext("snapshotId") |
410 | - volume_id = root.findtext("volumeId") |
411 | - status = root.findtext("status") |
412 | - start_time = root.findtext("startTime") |
413 | - start_time = datetime.strptime( |
414 | - start_time[:19], "%Y-%m-%dT%H:%M:%S") |
415 | - progress = root.findtext("progress")[:-1] |
416 | - progress = float(progress or "0") / 100. |
417 | - return model.Snapshot( |
418 | - snapshot_id, volume_id, status, start_time, progress) |
419 | + return d.addCallback(self.parser.create_snapshot) |
420 | |
421 | def delete_snapshot(self, snapshot_id): |
422 | """Remove a previously created snapshot.""" |
423 | @@ -598,7 +364,7 @@ |
424 | action="DeleteSnapshot", creds=self.creds, endpoint=self.endpoint, |
425 | other_params={"SnapshotId": snapshot_id}) |
426 | d = query.submit() |
427 | - return d.addCallback(self._parse_truth_return) |
428 | + return d.addCallback(self.parser.truth_return) |
429 | |
430 | def attach_volume(self, volume_id, instance_id, device): |
431 | """Attach the given volume to the specified instance at C{device}.""" |
432 | @@ -607,15 +373,7 @@ |
433 | other_params={"VolumeId": volume_id, "InstanceId": instance_id, |
434 | "Device": device}) |
435 | d = query.submit() |
436 | - return d.addCallback(self._parse_attach_volume) |
437 | - |
438 | - def _parse_attach_volume(self, xml_bytes): |
439 | - root = XML(xml_bytes) |
440 | - status = root.findtext("status") |
441 | - attach_time = root.findtext("attachTime") |
442 | - attach_time = datetime.strptime( |
443 | - attach_time[:19], "%Y-%m-%dT%H:%M:%S") |
444 | - return {"status": status, "attach_time": attach_time} |
445 | + return d.addCallback(self.parser.attach_volume) |
446 | |
447 | def describe_keypairs(self, *keypair_names): |
448 | """Returns information about key pairs available.""" |
449 | @@ -626,19 +384,7 @@ |
450 | action="DescribeKeyPairs", creds=self.creds, |
451 | endpoint=self.endpoint, other_params=keypairs) |
452 | d = query.submit() |
453 | - return d.addCallback(self._parse_describe_keypairs) |
454 | - |
455 | - def _parse_describe_keypairs(self, xml_bytes): |
456 | - results = [] |
457 | - root = XML(xml_bytes) |
458 | - keypairs = root.find("keySet") |
459 | - if keypairs is None: |
460 | - return results |
461 | - for keypair_data in keypairs: |
462 | - key_name = keypair_data.findtext("keyName") |
463 | - key_fingerprint = keypair_data.findtext("keyFingerprint") |
464 | - results.append(model.Keypair(key_name, key_fingerprint)) |
465 | - return results |
466 | + return d.addCallback(self.parser.describe_keypairs) |
467 | |
468 | def create_keypair(self, keypair_name): |
469 | """ |
470 | @@ -649,14 +395,7 @@ |
471 | action="CreateKeyPair", creds=self.creds, endpoint=self.endpoint, |
472 | other_params={"KeyName": keypair_name}) |
473 | d = query.submit() |
474 | - return d.addCallback(self._parse_create_keypair) |
475 | - |
476 | - def _parse_create_keypair(self, xml_bytes): |
477 | - keypair_data = XML(xml_bytes) |
478 | - key_name = keypair_data.findtext("keyName") |
479 | - key_fingerprint = keypair_data.findtext("keyFingerprint") |
480 | - key_material = keypair_data.findtext("keyMaterial") |
481 | - return model.Keypair(key_name, key_fingerprint, key_material) |
482 | + return d.addCallback(self.parser.create_keypair) |
483 | |
484 | def delete_keypair(self, keypair_name): |
485 | """Delete a given keypair.""" |
486 | @@ -664,7 +403,7 @@ |
487 | action="DeleteKeyPair", creds=self.creds, endpoint=self.endpoint, |
488 | other_params={"KeyName": keypair_name}) |
489 | d = query.submit() |
490 | - return d.addCallback(self._parse_truth_return) |
491 | + return d.addCallback(self.parser.truth_return) |
492 | |
493 | def import_keypair(self, keypair_name, key_material): |
494 | """ |
495 | @@ -685,14 +424,7 @@ |
496 | other_params={"KeyName": keypair_name, |
497 | "PublicKeyMaterial": b64encode(key_material)}) |
498 | d = query.submit() |
499 | - return d.addCallback(self._parse_import_keypair, key_material) |
500 | - |
501 | - def _parse_import_keypair(self, xml_bytes, key_material): |
502 | - """Extract the key name and the fingerprint from the result.""" |
503 | - keypair_data = XML(xml_bytes) |
504 | - key_name = keypair_data.findtext("keyName") |
505 | - key_fingerprint = keypair_data.findtext("keyFingerprint") |
506 | - return model.Keypair(key_name, key_fingerprint, key_material) |
507 | + return d.addCallback(self.parser.import_keypair, key_material) |
508 | |
509 | def allocate_address(self): |
510 | """ |
511 | @@ -706,11 +438,7 @@ |
512 | action="AllocateAddress", creds=self.creds, endpoint=self.endpoint, |
513 | other_params={}) |
514 | d = query.submit() |
515 | - return d.addCallback(self._parse_allocate_address) |
516 | - |
517 | - def _parse_allocate_address(self, xml_bytes): |
518 | - address_data = XML(xml_bytes) |
519 | - return address_data.findtext("publicIp") |
520 | + return d.addCallback(self.parser.allocate_address) |
521 | |
522 | def release_address(self, address): |
523 | """ |
524 | @@ -722,7 +450,7 @@ |
525 | action="ReleaseAddress", creds=self.creds, endpoint=self.endpoint, |
526 | other_params={"PublicIp": address}) |
527 | d = query.submit() |
528 | - return d.addCallback(self._parse_truth_return) |
529 | + return d.addCallback(self.parser.truth_return) |
530 | |
531 | def associate_address(self, instance_id, address): |
532 | """ |
533 | @@ -736,7 +464,7 @@ |
534 | endpoint=self.endpoint, |
535 | other_params={"InstanceId": instance_id, "PublicIp": address}) |
536 | d = query.submit() |
537 | - return d.addCallback(self._parse_truth_return) |
538 | + return d.addCallback(self.parser.truth_return) |
539 | |
540 | def disassociate_address(self, address): |
541 | """ |
542 | @@ -748,7 +476,7 @@ |
543 | action="DisassociateAddress", creds=self.creds, |
544 | endpoint=self.endpoint, other_params={"PublicIp": address}) |
545 | d = query.submit() |
546 | - return d.addCallback(self._parse_truth_return) |
547 | + return d.addCallback(self.parser.truth_return) |
548 | |
549 | def describe_addresses(self, *addresses): |
550 | """ |
551 | @@ -766,16 +494,7 @@ |
552 | action="DescribeAddresses", creds=self.creds, |
553 | endpoint=self.endpoint, other_params=address_set) |
554 | d = query.submit() |
555 | - return d.addCallback(self._parse_describe_addresses) |
556 | - |
557 | - def _parse_describe_addresses(self, xml_bytes): |
558 | - results = [] |
559 | - root = XML(xml_bytes) |
560 | - for address_data in root.find("addressesSet"): |
561 | - address = address_data.findtext("publicIp") |
562 | - instance_id = address_data.findtext("instanceId") |
563 | - results.append((address, instance_id)) |
564 | - return results |
565 | + return d.addCallback(self.parser.describe_addresses) |
566 | |
567 | def describe_availability_zones(self, names=None): |
568 | zone_names = None |
569 | @@ -786,9 +505,372 @@ |
570 | action="DescribeAvailabilityZones", creds=self.creds, |
571 | endpoint=self.endpoint, other_params=zone_names) |
572 | d = query.submit() |
573 | - return d.addCallback(self._parse_describe_availability_zones) |
574 | - |
575 | - def _parse_describe_availability_zones(self, xml_bytes): |
576 | + return d.addCallback(self.parser.describe_availability_zones) |
577 | + |
578 | + |
579 | +class Parser(object): |
580 | + """A parser for EC2 responses""" |
581 | + |
582 | + def instances_set(self, root, reservation): |
583 | + """Parse instance data out of an XML payload. |
584 | + |
585 | + @param root: The root node of the XML payload. |
586 | + @param reservation: The L{Reservation} associated with the instances |
587 | + from the response. |
588 | + @return: A C{list} of L{Instance}s. |
589 | + """ |
590 | + instances = [] |
591 | + for instance_data in root.find("instancesSet"): |
592 | + instances.append(self.instance(instance_data, reservation)) |
593 | + return instances |
594 | + |
595 | + def instance(self, instance_data, reservation): |
596 | + """Parse instance data out of an XML payload. |
597 | + |
598 | + @param instance_data: An XML node containing instance data. |
599 | + @param reservation: The L{Reservation} associated with the instance. |
600 | + @return: An L{Instance}. |
601 | + """ |
602 | + instance_id = instance_data.findtext("instanceId") |
603 | + instance_state = instance_data.find( |
604 | + "instanceState").findtext("name") |
605 | + instance_type = instance_data.findtext("instanceType") |
606 | + image_id = instance_data.findtext("imageId") |
607 | + private_dns_name = instance_data.findtext("privateDnsName") |
608 | + dns_name = instance_data.findtext("dnsName") |
609 | + key_name = instance_data.findtext("keyName") |
610 | + ami_launch_index = instance_data.findtext("amiLaunchIndex") |
611 | + launch_time = instance_data.findtext("launchTime") |
612 | + placement = instance_data.find("placement").findtext( |
613 | + "availabilityZone") |
614 | + products = [] |
615 | + product_codes = instance_data.find("productCodes") |
616 | + if product_codes is not None: |
617 | + for product_data in instance_data.find("productCodes"): |
618 | + products.append(product_data.text) |
619 | + kernel_id = instance_data.findtext("kernelId") |
620 | + ramdisk_id = instance_data.findtext("ramdiskId") |
621 | + instance = model.Instance( |
622 | + instance_id, instance_state, instance_type, image_id, |
623 | + private_dns_name, dns_name, key_name, ami_launch_index, |
624 | + launch_time, placement, products, kernel_id, ramdisk_id, |
625 | + reservation=reservation) |
626 | + return instance |
627 | + |
628 | + def describe_instances(self, xml_bytes): |
629 | + """ |
630 | + Parse the reservations XML payload that is returned from an AWS |
631 | + describeInstances API call. |
632 | + |
633 | + Instead of returning the reservations as the "top-most" object, we |
634 | + return the object that most developers and their code will be |
635 | + interested in: the instances. In instances reservation is available on |
636 | + the instance object. |
637 | + |
638 | + The following instance attributes are optional: |
639 | + * ami_launch_index |
640 | + * key_name |
641 | + * kernel_id |
642 | + * product_codes |
643 | + * ramdisk_id |
644 | + * reason |
645 | + |
646 | + @param xml_bytes: raw XML payload from AWS. |
647 | + """ |
648 | + root = XML(xml_bytes) |
649 | + results = [] |
650 | + # May be a more elegant way to do this: |
651 | + for reservation_data in root.find("reservationSet"): |
652 | + # Get the security group information. |
653 | + groups = [] |
654 | + for group_data in reservation_data.find("groupSet"): |
655 | + group_id = group_data.findtext("groupId") |
656 | + groups.append(group_id) |
657 | + # Create a reservation object with the parsed data. |
658 | + reservation = model.Reservation( |
659 | + reservation_id=reservation_data.findtext("reservationId"), |
660 | + owner_id=reservation_data.findtext("ownerId"), |
661 | + groups=groups) |
662 | + # Get the list of instances. |
663 | + instances = self.instances_set( |
664 | + reservation_data, reservation) |
665 | + results.extend(instances) |
666 | + return results |
667 | + |
668 | + def run_instances(self, xml_bytes): |
669 | + """ |
670 | + Parse the reservations XML payload that is returned from an AWS |
671 | + RunInstances API call. |
672 | + |
673 | + @param xml_bytes: raw XML payload from AWS. |
674 | + """ |
675 | + root = XML(xml_bytes) |
676 | + # Get the security group information. |
677 | + groups = [] |
678 | + for group_data in root.find("groupSet"): |
679 | + group_id = group_data.findtext("groupId") |
680 | + groups.append(group_id) |
681 | + # Create a reservation object with the parsed data. |
682 | + reservation = model.Reservation( |
683 | + reservation_id=root.findtext("reservationId"), |
684 | + owner_id=root.findtext("ownerId"), |
685 | + groups=groups) |
686 | + # Get the list of instances. |
687 | + instances = self.instances_set(root, reservation) |
688 | + return instances |
689 | + |
690 | + def terminate_instances(self, xml_bytes): |
691 | + """Parse the XML returned by the C{TerminateInstances} function. |
692 | + |
693 | + @param xml_bytes: XML bytes with a C{TerminateInstancesResponse} root |
694 | + element. |
695 | + @return: An iterable of C{tuple} of (instanceId, previousState, |
696 | + shutdownState) for the ec2 instances that where terminated. |
697 | + """ |
698 | + root = XML(xml_bytes) |
699 | + result = [] |
700 | + # May be a more elegant way to do this: |
701 | + for instance in root.find("instancesSet"): |
702 | + instanceId = instance.findtext("instanceId") |
703 | + previousState = instance.find("previousState").findtext( |
704 | + "name") |
705 | + shutdownState = instance.find("shutdownState").findtext( |
706 | + "name") |
707 | + result.append((instanceId, previousState, shutdownState)) |
708 | + return result |
709 | + |
710 | + def describe_security_groups(self, xml_bytes): |
711 | + """Parse the XML returned by the C{DescribeSecurityGroups} function. |
712 | + |
713 | + @param xml_bytes: XML bytes with a C{DescribeSecurityGroupsResponse} |
714 | + root element. |
715 | + @return: A list of L{SecurityGroup} instances. |
716 | + """ |
717 | + root = XML(xml_bytes) |
718 | + result = [] |
719 | + for group_info in root.findall("securityGroupInfo/item"): |
720 | + name = group_info.findtext("groupName") |
721 | + description = group_info.findtext("groupDescription") |
722 | + owner_id = group_info.findtext("ownerId") |
723 | + allowed_groups = [] |
724 | + allowed_ips = [] |
725 | + ip_permissions = group_info.find("ipPermissions") |
726 | + if ip_permissions is None: |
727 | + ip_permissions = () |
728 | + for ip_permission in ip_permissions: |
729 | + ip_protocol = ip_permission.findtext("ipProtocol") |
730 | + from_port = int(ip_permission.findtext("fromPort")) |
731 | + to_port = int(ip_permission.findtext("toPort")) |
732 | + for groups in ip_permission.findall("groups/item") or (): |
733 | + user_id = groups.findtext("userId") |
734 | + group_name = groups.findtext("groupName") |
735 | + if user_id and group_name: |
736 | + if (user_id, group_name) not in allowed_groups: |
737 | + allowed_groups.append((user_id, group_name)) |
738 | + for ip_ranges in ip_permission.findall("ipRanges/item") or (): |
739 | + cidr_ip = ip_ranges.findtext("cidrIp") |
740 | + allowed_ips.append( |
741 | + model.IPPermission( |
742 | + ip_protocol, from_port, to_port, cidr_ip)) |
743 | + |
744 | + allowed_groups = [model.UserIDGroupPair(user_id, group_name) |
745 | + for user_id, group_name in allowed_groups] |
746 | + |
747 | + security_group = model.SecurityGroup( |
748 | + name, description, owner_id=owner_id, |
749 | + groups=allowed_groups, ips=allowed_ips) |
750 | + result.append(security_group) |
751 | + return result |
752 | + |
753 | + def truth_return(self, xml_bytes): |
754 | + """Parse the XML for a truth value. |
755 | + |
756 | + @param xml_bytes: XML bytes. |
757 | + @return: True if the node contains "return" otherwise False. |
758 | + """ |
759 | + root = XML(xml_bytes) |
760 | + return root.findtext("return") == "true" |
761 | + |
762 | + def describe_volumes(self, xml_bytes): |
763 | + """Parse the XML returned by the C{DescribeVolumes} function. |
764 | + |
765 | + @param xml_bytes: XML bytes with a C{DescribeVolumesResponse} root |
766 | + element. |
767 | + @return: A list of L{Volume} instances. |
768 | + """ |
769 | + root = XML(xml_bytes) |
770 | + result = [] |
771 | + for volume_data in root.find("volumeSet"): |
772 | + volume_id = volume_data.findtext("volumeId") |
773 | + size = int(volume_data.findtext("size")) |
774 | + status = volume_data.findtext("status") |
775 | + availability_zone = volume_data.findtext("availabilityZone") |
776 | + snapshot_id = volume_data.findtext("snapshotId") |
777 | + create_time = volume_data.findtext("createTime") |
778 | + create_time = datetime.strptime( |
779 | + create_time[:19], "%Y-%m-%dT%H:%M:%S") |
780 | + volume = model.Volume( |
781 | + volume_id, size, status, create_time, availability_zone, |
782 | + snapshot_id) |
783 | + result.append(volume) |
784 | + for attachment_data in volume_data.find("attachmentSet"): |
785 | + instance_id = attachment_data.findtext("instanceId") |
786 | + status = attachment_data.findtext("status") |
787 | + device = attachment_data.findtext("device") |
788 | + attach_time = attachment_data.findtext("attachTime") |
789 | + attach_time = datetime.strptime( |
790 | + attach_time[:19], "%Y-%m-%dT%H:%M:%S") |
791 | + attachment = model.Attachment( |
792 | + instance_id, device, status, attach_time) |
793 | + volume.attachments.append(attachment) |
794 | + return result |
795 | + |
796 | + def create_volume(self, xml_bytes): |
797 | + """Parse the XML returned by the C{CreateVolume} function. |
798 | + |
799 | + @param xml_bytes: XML bytes with a C{CreateVolumeResponse} root |
800 | + element. |
801 | + @return: The L{Volume} instance created. |
802 | + """ |
803 | + root = XML(xml_bytes) |
804 | + volume_id = root.findtext("volumeId") |
805 | + size = int(root.findtext("size")) |
806 | + status = root.findtext("status") |
807 | + create_time = root.findtext("createTime") |
808 | + availability_zone = root.findtext("availabilityZone") |
809 | + snapshot_id = root.findtext("snapshotId") |
810 | + create_time = datetime.strptime( |
811 | + create_time[:19], "%Y-%m-%dT%H:%M:%S") |
812 | + volume = model.Volume( |
813 | + volume_id, size, status, create_time, availability_zone, |
814 | + snapshot_id) |
815 | + return volume |
816 | + |
817 | + def snapshots(self, xml_bytes): |
818 | + """Parse the XML returned by the C{DescribeSnapshots} function. |
819 | + |
820 | + @param xml_bytes: XML bytes with a C{DescribeSnapshotsResponse} root |
821 | + element. |
822 | + @return: A list of L{Snapshot} instances. |
823 | + """ |
824 | + root = XML(xml_bytes) |
825 | + result = [] |
826 | + for snapshot_data in root.find("snapshotSet"): |
827 | + snapshot_id = snapshot_data.findtext("snapshotId") |
828 | + volume_id = snapshot_data.findtext("volumeId") |
829 | + status = snapshot_data.findtext("status") |
830 | + start_time = snapshot_data.findtext("startTime") |
831 | + start_time = datetime.strptime( |
832 | + start_time[:19], "%Y-%m-%dT%H:%M:%S") |
833 | + progress = snapshot_data.findtext("progress")[:-1] |
834 | + progress = float(progress or "0") / 100. |
835 | + snapshot = model.Snapshot( |
836 | + snapshot_id, volume_id, status, start_time, progress) |
837 | + result.append(snapshot) |
838 | + return result |
839 | + |
840 | + def create_snapshot(self, xml_bytes): |
841 | + """Parse the XML returned by the C{CreateSnapshot} function. |
842 | + |
843 | + @param xml_bytes: XML bytes with a C{CreateSnapshotResponse} root |
844 | + element. |
845 | + @return: The L{Snapshot} instance created. |
846 | + """ |
847 | + root = XML(xml_bytes) |
848 | + snapshot_id = root.findtext("snapshotId") |
849 | + volume_id = root.findtext("volumeId") |
850 | + status = root.findtext("status") |
851 | + start_time = root.findtext("startTime") |
852 | + start_time = datetime.strptime( |
853 | + start_time[:19], "%Y-%m-%dT%H:%M:%S") |
854 | + progress = root.findtext("progress")[:-1] |
855 | + progress = float(progress or "0") / 100. |
856 | + return model.Snapshot( |
857 | + snapshot_id, volume_id, status, start_time, progress) |
858 | + |
859 | + def attach_volume(self, xml_bytes): |
860 | + """Parse the XML returned by the C{AttachVolume} function. |
861 | + |
862 | + @param xml_bytes: XML bytes with a C{AttachVolumeResponse} root |
863 | + element. |
864 | + @return: a C{dict} with status and attach_time keys. |
865 | + """ |
866 | + root = XML(xml_bytes) |
867 | + status = root.findtext("status") |
868 | + attach_time = root.findtext("attachTime") |
869 | + attach_time = datetime.strptime( |
870 | + attach_time[:19], "%Y-%m-%dT%H:%M:%S") |
871 | + return {"status": status, "attach_time": attach_time} |
872 | + |
873 | + def describe_keypairs(self, xml_bytes): |
874 | + """Parse the XML returned by the C{DescribeKeyPairs} function. |
875 | + |
876 | + @param xml_bytes: XML bytes with a C{DescribeKeyPairsResponse} root |
877 | + element. |
878 | + @return: a C{list} of L{Keypair}. |
879 | + """ |
880 | + results = [] |
881 | + root = XML(xml_bytes) |
882 | + keypairs = root.find("keySet") |
883 | + if keypairs is None: |
884 | + return results |
885 | + for keypair_data in keypairs: |
886 | + key_name = keypair_data.findtext("keyName") |
887 | + key_fingerprint = keypair_data.findtext("keyFingerprint") |
888 | + results.append(model.Keypair(key_name, key_fingerprint)) |
889 | + return results |
890 | + |
891 | + def create_keypair(self, xml_bytes): |
892 | + """Parse the XML returned by the C{CreateKeyPair} function. |
893 | + |
894 | + @param xml_bytes: XML bytes with a C{CreateKeyPairResponse} root |
895 | + element. |
896 | + @return: The L{Keypair} instance created. |
897 | + """ |
898 | + keypair_data = XML(xml_bytes) |
899 | + key_name = keypair_data.findtext("keyName") |
900 | + key_fingerprint = keypair_data.findtext("keyFingerprint") |
901 | + key_material = keypair_data.findtext("keyMaterial") |
902 | + return model.Keypair(key_name, key_fingerprint, key_material) |
903 | + |
904 | + def import_keypair(self, xml_bytes, key_material): |
905 | + """Extract the key name and the fingerprint from the result.""" |
906 | + keypair_data = XML(xml_bytes) |
907 | + key_name = keypair_data.findtext("keyName") |
908 | + key_fingerprint = keypair_data.findtext("keyFingerprint") |
909 | + return model.Keypair(key_name, key_fingerprint, key_material) |
910 | + |
911 | + def allocate_address(self, xml_bytes): |
912 | + """Parse the XML returned by the C{AllocateAddress} function. |
913 | + |
914 | + @param xml_bytes: XML bytes with a C{AllocateAddress} root element. |
915 | + @return: The public ip address as a string. |
916 | + """ |
917 | + address_data = XML(xml_bytes) |
918 | + return address_data.findtext("publicIp") |
919 | + |
920 | + def describe_addresses(self, xml_bytes): |
921 | + """Parse the XML returned by the C{DescribeAddresses} function. |
922 | + |
923 | + @param xml_bytes: XML bytes with a C{DescribeAddressesResponse} root |
924 | + element. |
925 | + @return: a C{list} of L{tuple} of (publicIp, instancId). |
926 | + """ |
927 | + results = [] |
928 | + root = XML(xml_bytes) |
929 | + for address_data in root.find("addressesSet"): |
930 | + address = address_data.findtext("publicIp") |
931 | + instance_id = address_data.findtext("instanceId") |
932 | + results.append((address, instance_id)) |
933 | + return results |
934 | + |
935 | + def describe_availability_zones(self, xml_bytes): |
936 | + """Parse the XML returned by the C{DescribeAvailibilityZones} function. |
937 | + |
938 | + @param xml_bytes: XML bytes with a C{DescribeAvailibilityZonesResponse} |
939 | + root element. |
940 | + @return: a C{list} of L{AvailabilityZone}. |
941 | + """ |
942 | results = [] |
943 | root = XML(xml_bytes) |
944 | for zone_data in root.find("availabilityZoneInfo"): |
945 | |
946 | === modified file 'txaws/ec2/tests/test_client.py' |
947 | --- txaws/ec2/tests/test_client.py 2011-04-21 16:14:58 +0000 |
948 | +++ txaws/ec2/tests/test_client.py 2011-04-26 16:30:59 +0000 |
949 | @@ -217,7 +217,7 @@ |
950 | def test_parse_reservation(self): |
951 | creds = AWSCredentials("foo", "bar") |
952 | ec2 = client.EC2Client(creds=creds) |
953 | - results = ec2._parse_describe_instances( |
954 | + results = ec2.parser.describe_instances( |
955 | payload.sample_describe_instances_result) |
956 | self.check_parsed_instances(results) |
957 | |
958 | |
959 | === modified file 'txaws/testing/ec2.py' |
960 | --- txaws/testing/ec2.py 2011-04-19 16:26:26 +0000 |
961 | +++ txaws/testing/ec2.py 2011-04-26 16:30:59 +0000 |
962 | @@ -16,11 +16,12 @@ |
963 | def __init__(self, creds, endpoint, instances=None, keypairs=None, |
964 | volumes=None, key_material="", security_groups=None, |
965 | snapshots=None, addresses=None, availability_zones=None, |
966 | - query_factory=None): |
967 | + query_factory=None, parser=None): |
968 | |
969 | self.creds = creds |
970 | self.endpoint = endpoint |
971 | self.query_factory = query_factory |
972 | + self.parser = parser |
973 | |
974 | self.instances = instances or [] |
975 | self.keypairs = keypairs or [] |
Man, I can't tell you how badly I've wanted this... since about the second or third week Thomas and I were working on the ec2 client support!
1) Overall, looks awesome -- nice work, and thanks :-)
2) The Parser class seems to have a superfluous __init__; I'd just recommend removing it.
3) This is a nit... totally up to you, but now that the parse_* methods are in their own Parse class, I'd just remove the "parse_" from each method name. With them in there, it looks like C code ;-)
All tests pass, +1 for merge; points #2 and #3 are up to you.