Merge lp:~fwierzbicki/txaws/break-out-ec2-parser into lp:txaws

[Frank Wierzbicki]

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.

[Duncan McGreggor (oubiwann)]

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.

review: Approve

[Frank Wierzbicki (fwierzbicki)]

> 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.

[lp:~fwierzbicki/txaws/break-out-ec2-parser]

85. By Frank Wierzbicki on 2011-04-21

Remove parse_ from Parser methods, remove unneeded __init__.

86. By Frank Wierzbicki on 2011-04-21

Merge with trunk.

87. By Frank Wierzbicki on 2011-04-25

Add parser parameter to FakeEC2Client.

[Thomas Herve (therve)]

[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!

review: Approve

[lp:~fwierzbicki/txaws/break-out-ec2-parser]

88. By Frank Wierzbicki on 2011-04-26

Update __all__

=== modified file 'txaws/client/base.py'
--- txaws/client/base.py	2011-04-21 14:59:23 +0000
+++ txaws/client/base.py	2011-04-26 16:30:59 +0000
@@ -57,8 +57,10 @@
     @param endpoint: The service endpoint URI.
     @param query_factory: The class or function that produces a query
         object for making requests to the EC2 service.
+    @param parser: A parser object for parsing responses from the EC2 service.
     """
-    def __init__(self, creds=None, endpoint=None, query_factory=None):
+    def __init__(self, creds=None, endpoint=None, query_factory=None,
+                 parser=None):
         if creds is None:
             creds = AWSCredentials()
         if endpoint is None:
@@ -66,6 +68,7 @@
         self.creds = creds
         self.endpoint = endpoint
         self.query_factory = query_factory
+        self.parser = parser
 
 
 class BaseQuery(object):
 
=== modified file 'txaws/client/tests/test_client.py'
--- txaws/client/tests/test_client.py	2011-04-21 16:40:58 +0000
+++ txaws/client/tests/test_client.py	2011-04-26 16:30:59 +0000
@@ -53,10 +53,11 @@
 class BaseClientTestCase(TXAWSTestCase):
 
     def test_creation(self):
-        client = BaseClient("creds", "endpoint", "query factory")
+        client = BaseClient("creds", "endpoint", "query factory", "parser")
         self.assertEquals(client.creds, "creds")
         self.assertEquals(client.endpoint, "endpoint")
         self.assertEquals(client.query_factory, "query factory")
+        self.assertEquals(client.parser, "parser")
 
 
 class BaseQueryTestCase(TXAWSTestCase):
 
=== modified file 'txaws/ec2/client.py'
--- txaws/ec2/client.py	2011-04-21 13:10:37 +0000
+++ txaws/ec2/client.py	2011-04-26 16:30:59 +0000
@@ -16,7 +16,7 @@
 from txaws.util import iso8601time, XML
 
 
-__all__ = ["EC2Client"]
+__all__ = ["EC2Client", "Query", "Parser"]
 
 
 def ec2_error_wrapper(error):
@@ -26,10 +26,13 @@
 class EC2Client(BaseClient):
     """A client for EC2."""
 
-    def __init__(self, creds=None, endpoint=None, query_factory=None):
+    def __init__(self, creds=None, endpoint=None, query_factory=None,
+                 parser=None):
         if query_factory is None:
             query_factory = Query
-        super(EC2Client, self).__init__(creds, endpoint, query_factory)
+        if parser is None:
+            parser = Parser()
+        super(EC2Client, self).__init__(creds, endpoint, query_factory, parser)
 
     def describe_instances(self, *instance_ids):
         """Describe current instances."""
@@ -40,91 +43,7 @@
             action="DescribeInstances", creds=self.creds,
             endpoint=self.endpoint, other_params=instances)
         d = query.submit()
-        return d.addCallback(self._parse_describe_instances)
-
-    def _parse_instances_set(self, root, reservation):
-        """Parse instance data out of an XML payload.
-
-        @param root: The root node of the XML payload.
-        @param reservation: The L{Reservation} associated with the instances
-            from the response.
-        @return: A C{list} of L{Instance}s.
-        """
-        instances = []
-        for instance_data in root.find("instancesSet"):
-            instances.append(self._parse_instance(instance_data, reservation))
-        return instances
-
-    def _parse_instance(self, instance_data, reservation):
-        """Parse instance data out of an XML payload.
-
-        @param instance_data: An XML node containing instance data.
-        @param reservation: The L{Reservation} associated with the instance.
-        @return: An L{Instance}.
-        """
-        instance_id = instance_data.findtext("instanceId")
-        instance_state = instance_data.find(
-            "instanceState").findtext("name")
-        instance_type = instance_data.findtext("instanceType")
-        image_id = instance_data.findtext("imageId")
-        private_dns_name = instance_data.findtext("privateDnsName")
-        dns_name = instance_data.findtext("dnsName")
-        key_name = instance_data.findtext("keyName")
-        ami_launch_index = instance_data.findtext("amiLaunchIndex")
-        launch_time = instance_data.findtext("launchTime")
-        placement = instance_data.find("placement").findtext(
-            "availabilityZone")
-        products = []
-        product_codes = instance_data.find("productCodes")
-        if product_codes is not None:
-            for product_data in instance_data.find("productCodes"):
-                products.append(product_data.text)
-        kernel_id = instance_data.findtext("kernelId")
-        ramdisk_id = instance_data.findtext("ramdiskId")
-        instance = model.Instance(
-            instance_id, instance_state, instance_type, image_id,
-            private_dns_name, dns_name, key_name, ami_launch_index,
-            launch_time, placement, products, kernel_id, ramdisk_id,
-            reservation=reservation)
-        return instance
-
-    def _parse_describe_instances(self, xml_bytes):
-        """
-        Parse the reservations XML payload that is returned from an AWS
-        describeInstances API call.
-
-        Instead of returning the reservations as the "top-most" object, we
-        return the object that most developers and their code will be
-        interested in: the instances. In instances reservation is available on
-        the instance object.
-
-        The following instance attributes are optional:
-            * ami_launch_index
-            * key_name
-            * kernel_id
-            * product_codes
-            * ramdisk_id
-            * reason
-        """
-        root = XML(xml_bytes)
-        results = []
-        # May be a more elegant way to do this:
-        for reservation_data in root.find("reservationSet"):
-            # Get the security group information.
-            groups = []
-            for group_data in reservation_data.find("groupSet"):
-                group_id = group_data.findtext("groupId")
-                groups.append(group_id)
-            # Create a reservation object with the parsed data.
-            reservation = model.Reservation(
-                reservation_id=reservation_data.findtext("reservationId"),
-                owner_id=reservation_data.findtext("ownerId"),
-                groups=groups)
-            # Get the list of instances.
-            instances = self._parse_instances_set(
-                reservation_data, reservation)
-            results.extend(instances)
-        return results
+        return d.addCallback(self.parser.describe_instances)
 
     def run_instances(self, image_id, min_count, max_count,
         security_groups=None, key_name=None, instance_type=None,
@@ -152,27 +71,7 @@
             action="RunInstances", creds=self.creds, endpoint=self.endpoint,
             other_params=params)
         d = query.submit()
-        return d.addCallback(self._parse_run_instances)
-
-    def _parse_run_instances(self, xml_bytes):
-        """
-        Parse the reservations XML payload that is returned from an AWS
-        RunInstances API call.
-        """
-        root = XML(xml_bytes)
-        # Get the security group information.
-        groups = []
-        for group_data in root.find("groupSet"):
-            group_id = group_data.findtext("groupId")
-            groups.append(group_id)
-        # Create a reservation object with the parsed data.
-        reservation = model.Reservation(
-            reservation_id=root.findtext("reservationId"),
-            owner_id=root.findtext("ownerId"),
-            groups=groups)
-        # Get the list of instances.
-        instances = self._parse_instances_set(root, reservation)
-        return instances
+        return d.addCallback(self.parser.run_instances)
 
     def terminate_instances(self, *instance_ids):
         """Terminate some instances.
@@ -188,20 +87,7 @@
             action="TerminateInstances", creds=self.creds,
             endpoint=self.endpoint, other_params=instances)
         d = query.submit()
-        return d.addCallback(self._parse_terminate_instances)
-
-    def _parse_terminate_instances(self, xml_bytes):
-        root = XML(xml_bytes)
-        result = []
-        # May be a more elegant way to do this:
-        for instance in root.find("instancesSet"):
-            instanceId = instance.findtext("instanceId")
-            previousState = instance.find("previousState").findtext(
-                "name")
-            shutdownState = instance.find("shutdownState").findtext(
-                "name")
-            result.append((instanceId, previousState, shutdownState))
-        return result
+        return d.addCallback(self.parser.terminate_instances)
 
     def describe_security_groups(self, *names):
         """Describe security groups.
@@ -219,50 +105,7 @@
             action="DescribeSecurityGroups", creds=self.creds,
             endpoint=self.endpoint, other_params=group_names)
         d = query.submit()
-        return d.addCallback(self._parse_describe_security_groups)
-
-    def _parse_describe_security_groups(self, xml_bytes):
-        """Parse the XML returned by the C{DescribeSecurityGroups} function.
-
-        @param xml_bytes: XML bytes with a C{DescribeSecurityGroupsResponse}
-            root element.
-        @return: A list of L{SecurityGroup} instances.
-        """
-        root = XML(xml_bytes)
-        result = []
-        for group_info in root.findall("securityGroupInfo/item"):
-            name = group_info.findtext("groupName")
-            description = group_info.findtext("groupDescription")
-            owner_id = group_info.findtext("ownerId")
-            allowed_groups = []
-            allowed_ips = []
-            ip_permissions = group_info.find("ipPermissions")
-            if ip_permissions is None:
-                ip_permissions = ()
-            for ip_permission in ip_permissions:
-                ip_protocol = ip_permission.findtext("ipProtocol")
-                from_port = int(ip_permission.findtext("fromPort"))
-                to_port = int(ip_permission.findtext("toPort"))
-                for groups in ip_permission.findall("groups/item") or ():
-                    user_id = groups.findtext("userId")
-                    group_name = groups.findtext("groupName")
-                    if user_id and group_name:
-                        if (user_id, group_name) not in allowed_groups:
-                            allowed_groups.append((user_id, group_name))
-                for ip_ranges in ip_permission.findall("ipRanges/item") or ():
-                    cidr_ip = ip_ranges.findtext("cidrIp")
-                    allowed_ips.append(
-                        model.IPPermission(
-                            ip_protocol, from_port, to_port, cidr_ip))
-
-            allowed_groups = [model.UserIDGroupPair(user_id, group_name)
-                              for user_id, group_name in allowed_groups]
-
-            security_group = model.SecurityGroup(
-                name, description, owner_id=owner_id,
-                groups=allowed_groups, ips=allowed_ips)
-            result.append(security_group)
-        return result
+        return d.addCallback(self.parser.describe_security_groups)
 
     def create_security_group(self, name, description):
         """Create security group.
@@ -277,11 +120,7 @@
             action="CreateSecurityGroup", creds=self.creds,
             endpoint=self.endpoint, other_params=parameters)
         d = query.submit()
-        return d.addCallback(self._parse_truth_return)
-
-    def _parse_truth_return(self, xml_bytes):
-        root = XML(xml_bytes)
-        return root.findtext("return") == "true"
+        return d.addCallback(self.parser.truth_return)
 
     def delete_security_group(self, name):
         """
@@ -294,7 +133,7 @@
             action="DeleteSecurityGroup", creds=self.creds,
             endpoint=self.endpoint, other_params=parameter)
         d = query.submit()
-        return d.addCallback(self._parse_truth_return)
+        return d.addCallback(self.parser.truth_return)
 
     def authorize_security_group(
         self, group_name, source_group_name="", source_group_owner_id="",
@@ -351,7 +190,7 @@
             action="AuthorizeSecurityGroupIngress", creds=self.creds,
             endpoint=self.endpoint, other_params=parameters)
         d = query.submit()
-        return d.addCallback(self._parse_truth_return)
+        return d.addCallback(self.parser.truth_return)
 
     def authorize_group_permission(
         self, group_name, source_group_name, source_group_owner_id):
@@ -436,7 +275,7 @@
             action="RevokeSecurityGroupIngress", creds=self.creds,
             endpoint=self.endpoint, other_params=parameters)
         d = query.submit()
-        return d.addCallback(self._parse_truth_return)
+        return d.addCallback(self.parser.truth_return)
 
     def revoke_group_permission(
         self, group_name, source_group_name, source_group_owner_id):
@@ -475,35 +314,7 @@
             action="DescribeVolumes", creds=self.creds, endpoint=self.endpoint,
             other_params=volumeset)
         d = query.submit()
-        return d.addCallback(self._parse_describe_volumes)
-
-    def _parse_describe_volumes(self, xml_bytes):
-        root = XML(xml_bytes)
-        result = []
-        for volume_data in root.find("volumeSet"):
-            volume_id = volume_data.findtext("volumeId")
-            size = int(volume_data.findtext("size"))
-            status = volume_data.findtext("status")
-            availability_zone = volume_data.findtext("availabilityZone")
-            snapshot_id = volume_data.findtext("snapshotId")
-            create_time = volume_data.findtext("createTime")
-            create_time = datetime.strptime(
-                create_time[:19], "%Y-%m-%dT%H:%M:%S")
-            volume = model.Volume(
-                volume_id, size, status, create_time, availability_zone,
-                snapshot_id)
-            result.append(volume)
-            for attachment_data in volume_data.find("attachmentSet"):
-                instance_id = attachment_data.findtext("instanceId")
-                status = attachment_data.findtext("status")
-                device = attachment_data.findtext("device")
-                attach_time = attachment_data.findtext("attachTime")
-                attach_time = datetime.strptime(
-                    attach_time[:19], "%Y-%m-%dT%H:%M:%S")
-                attachment = model.Attachment(
-                    instance_id, device, status, attach_time)
-                volume.attachments.append(attachment)
-        return result
+        return d.addCallback(self.parser.describe_volumes)
 
     def create_volume(self, availability_zone, size=None, snapshot_id=None):
         """Create a new volume."""
@@ -519,29 +330,14 @@
             action="CreateVolume", creds=self.creds, endpoint=self.endpoint,
             other_params=params)
         d = query.submit()
-        return d.addCallback(self._parse_create_volume)
-
-    def _parse_create_volume(self, xml_bytes):
-        root = XML(xml_bytes)
-        volume_id = root.findtext("volumeId")
-        size = int(root.findtext("size"))
-        status = root.findtext("status")
-        create_time = root.findtext("createTime")
-        availability_zone = root.findtext("availabilityZone")
-        snapshot_id = root.findtext("snapshotId")
-        create_time = datetime.strptime(
-            create_time[:19], "%Y-%m-%dT%H:%M:%S")
-        volume = model.Volume(
-            volume_id, size, status, create_time, availability_zone,
-            snapshot_id)
-        return volume
+        return d.addCallback(self.parser.create_volume)
 
     def delete_volume(self, volume_id):
         query = self.query_factory(
             action="DeleteVolume", creds=self.creds, endpoint=self.endpoint,
             other_params={"VolumeId": volume_id})
         d = query.submit()
-        return d.addCallback(self._parse_truth_return)
+        return d.addCallback(self.parser.truth_return)
 
     def describe_snapshots(self, *snapshot_ids):
         """Describe available snapshots."""
@@ -552,24 +348,7 @@
             action="DescribeSnapshots", creds=self.creds,
             endpoint=self.endpoint, other_params=snapshot_set)
         d = query.submit()
-        return d.addCallback(self._parse_snapshots)
-
-    def _parse_snapshots(self, xml_bytes):
-        root = XML(xml_bytes)
-        result = []
-        for snapshot_data in root.find("snapshotSet"):
-            snapshot_id = snapshot_data.findtext("snapshotId")
-            volume_id = snapshot_data.findtext("volumeId")
-            status = snapshot_data.findtext("status")
-            start_time = snapshot_data.findtext("startTime")
-            start_time = datetime.strptime(
-                start_time[:19], "%Y-%m-%dT%H:%M:%S")
-            progress = snapshot_data.findtext("progress")[:-1]
-            progress = float(progress or "0") / 100.
-            snapshot = model.Snapshot(
-                snapshot_id, volume_id, status, start_time, progress)
-            result.append(snapshot)
-        return result
+        return d.addCallback(self.parser.snapshots)
 
     def create_snapshot(self, volume_id):
         """Create a new snapshot of an existing volume."""
@@ -577,20 +356,7 @@
             action="CreateSnapshot", creds=self.creds, endpoint=self.endpoint,
             other_params={"VolumeId": volume_id})
         d = query.submit()
-        return d.addCallback(self._parse_create_snapshot)
-
-    def _parse_create_snapshot(self, xml_bytes):
-        root = XML(xml_bytes)
-        snapshot_id = root.findtext("snapshotId")
-        volume_id = root.findtext("volumeId")
-        status = root.findtext("status")
-        start_time = root.findtext("startTime")
-        start_time = datetime.strptime(
-            start_time[:19], "%Y-%m-%dT%H:%M:%S")
-        progress = root.findtext("progress")[:-1]
-        progress = float(progress or "0") / 100.
-        return model.Snapshot(
-            snapshot_id, volume_id, status, start_time, progress)
+        return d.addCallback(self.parser.create_snapshot)
 
     def delete_snapshot(self, snapshot_id):
         """Remove a previously created snapshot."""
@@ -598,7 +364,7 @@
             action="DeleteSnapshot", creds=self.creds, endpoint=self.endpoint,
             other_params={"SnapshotId": snapshot_id})
         d = query.submit()
-        return d.addCallback(self._parse_truth_return)
+        return d.addCallback(self.parser.truth_return)
 
     def attach_volume(self, volume_id, instance_id, device):
         """Attach the given volume to the specified instance at C{device}."""
@@ -607,15 +373,7 @@
             other_params={"VolumeId": volume_id, "InstanceId": instance_id,
                           "Device": device})
         d = query.submit()
-        return d.addCallback(self._parse_attach_volume)
-
-    def _parse_attach_volume(self, xml_bytes):
-        root = XML(xml_bytes)
-        status = root.findtext("status")
-        attach_time = root.findtext("attachTime")
-        attach_time = datetime.strptime(
-            attach_time[:19], "%Y-%m-%dT%H:%M:%S")
-        return {"status": status, "attach_time": attach_time}
+        return d.addCallback(self.parser.attach_volume)
 
     def describe_keypairs(self, *keypair_names):
         """Returns information about key pairs available."""
@@ -626,19 +384,7 @@
             action="DescribeKeyPairs", creds=self.creds,
             endpoint=self.endpoint, other_params=keypairs)
         d = query.submit()
-        return d.addCallback(self._parse_describe_keypairs)
-
-    def _parse_describe_keypairs(self, xml_bytes):
-        results = []
-        root = XML(xml_bytes)
-        keypairs = root.find("keySet")
-        if keypairs is None:
-            return results
-        for keypair_data in keypairs:
-            key_name = keypair_data.findtext("keyName")
-            key_fingerprint = keypair_data.findtext("keyFingerprint")
-            results.append(model.Keypair(key_name, key_fingerprint))
-        return results
+        return d.addCallback(self.parser.describe_keypairs)
 
     def create_keypair(self, keypair_name):
         """
@@ -649,14 +395,7 @@
             action="CreateKeyPair", creds=self.creds, endpoint=self.endpoint,
             other_params={"KeyName": keypair_name})
         d = query.submit()
-        return d.addCallback(self._parse_create_keypair)
-
-    def _parse_create_keypair(self, xml_bytes):
-        keypair_data = XML(xml_bytes)
-        key_name = keypair_data.findtext("keyName")
-        key_fingerprint = keypair_data.findtext("keyFingerprint")
-        key_material = keypair_data.findtext("keyMaterial")
-        return model.Keypair(key_name, key_fingerprint, key_material)
+        return d.addCallback(self.parser.create_keypair)
 
     def delete_keypair(self, keypair_name):
         """Delete a given keypair."""
@@ -664,7 +403,7 @@
             action="DeleteKeyPair", creds=self.creds, endpoint=self.endpoint,
             other_params={"KeyName": keypair_name})
         d = query.submit()
-        return d.addCallback(self._parse_truth_return)
+        return d.addCallback(self.parser.truth_return)
 
     def import_keypair(self, keypair_name, key_material):
         """
@@ -685,14 +424,7 @@
             other_params={"KeyName": keypair_name,
                           "PublicKeyMaterial": b64encode(key_material)})
         d = query.submit()
-        return d.addCallback(self._parse_import_keypair, key_material)
-
-    def _parse_import_keypair(self, xml_bytes, key_material):
-        """Extract the key name and the fingerprint from the result."""
-        keypair_data = XML(xml_bytes)
-        key_name = keypair_data.findtext("keyName")
-        key_fingerprint = keypair_data.findtext("keyFingerprint")
-        return model.Keypair(key_name, key_fingerprint, key_material)
+        return d.addCallback(self.parser.import_keypair, key_material)
 
     def allocate_address(self):
         """
@@ -706,11 +438,7 @@
             action="AllocateAddress", creds=self.creds, endpoint=self.endpoint,
             other_params={})
         d = query.submit()
-        return d.addCallback(self._parse_allocate_address)
-
-    def _parse_allocate_address(self, xml_bytes):
-        address_data = XML(xml_bytes)
-        return address_data.findtext("publicIp")
+        return d.addCallback(self.parser.allocate_address)
 
     def release_address(self, address):
         """
@@ -722,7 +450,7 @@
             action="ReleaseAddress", creds=self.creds, endpoint=self.endpoint,
             other_params={"PublicIp": address})
         d = query.submit()
-        return d.addCallback(self._parse_truth_return)
+        return d.addCallback(self.parser.truth_return)
 
     def associate_address(self, instance_id, address):
         """
@@ -736,7 +464,7 @@
             endpoint=self.endpoint,
             other_params={"InstanceId": instance_id, "PublicIp": address})
         d = query.submit()
-        return d.addCallback(self._parse_truth_return)
+        return d.addCallback(self.parser.truth_return)
 
     def disassociate_address(self, address):
         """
@@ -748,7 +476,7 @@
             action="DisassociateAddress", creds=self.creds,
             endpoint=self.endpoint, other_params={"PublicIp": address})
         d = query.submit()
-        return d.addCallback(self._parse_truth_return)
+        return d.addCallback(self.parser.truth_return)
 
     def describe_addresses(self, *addresses):
         """
@@ -766,16 +494,7 @@
             action="DescribeAddresses", creds=self.creds,
             endpoint=self.endpoint, other_params=address_set)
         d = query.submit()
-        return d.addCallback(self._parse_describe_addresses)
-
-    def _parse_describe_addresses(self, xml_bytes):
-        results = []
-        root = XML(xml_bytes)
-        for address_data in root.find("addressesSet"):
-            address = address_data.findtext("publicIp")
-            instance_id = address_data.findtext("instanceId")
-            results.append((address, instance_id))
-        return results
+        return d.addCallback(self.parser.describe_addresses)
 
     def describe_availability_zones(self, names=None):
         zone_names = None
@@ -786,9 +505,372 @@
             action="DescribeAvailabilityZones", creds=self.creds,
             endpoint=self.endpoint, other_params=zone_names)
         d = query.submit()
-        return d.addCallback(self._parse_describe_availability_zones)
-
-    def _parse_describe_availability_zones(self, xml_bytes):
+        return d.addCallback(self.parser.describe_availability_zones)
+
+
+class Parser(object):
+    """A parser for EC2 responses"""
+
+    def instances_set(self, root, reservation):
+        """Parse instance data out of an XML payload.
+
+        @param root: The root node of the XML payload.
+        @param reservation: The L{Reservation} associated with the instances
+            from the response.
+        @return: A C{list} of L{Instance}s.
+        """
+        instances = []
+        for instance_data in root.find("instancesSet"):
+            instances.append(self.instance(instance_data, reservation))
+        return instances
+
+    def instance(self, instance_data, reservation):
+        """Parse instance data out of an XML payload.
+
+        @param instance_data: An XML node containing instance data.
+        @param reservation: The L{Reservation} associated with the instance.
+        @return: An L{Instance}.
+        """
+        instance_id = instance_data.findtext("instanceId")
+        instance_state = instance_data.find(
+            "instanceState").findtext("name")
+        instance_type = instance_data.findtext("instanceType")
+        image_id = instance_data.findtext("imageId")
+        private_dns_name = instance_data.findtext("privateDnsName")
+        dns_name = instance_data.findtext("dnsName")
+        key_name = instance_data.findtext("keyName")
+        ami_launch_index = instance_data.findtext("amiLaunchIndex")
+        launch_time = instance_data.findtext("launchTime")
+        placement = instance_data.find("placement").findtext(
+            "availabilityZone")
+        products = []
+        product_codes = instance_data.find("productCodes")
+        if product_codes is not None:
+            for product_data in instance_data.find("productCodes"):
+                products.append(product_data.text)
+        kernel_id = instance_data.findtext("kernelId")
+        ramdisk_id = instance_data.findtext("ramdiskId")
+        instance = model.Instance(
+            instance_id, instance_state, instance_type, image_id,
+            private_dns_name, dns_name, key_name, ami_launch_index,
+            launch_time, placement, products, kernel_id, ramdisk_id,
+            reservation=reservation)
+        return instance
+
+    def describe_instances(self, xml_bytes):
+        """
+        Parse the reservations XML payload that is returned from an AWS
+        describeInstances API call.
+
+        Instead of returning the reservations as the "top-most" object, we
+        return the object that most developers and their code will be
+        interested in: the instances. In instances reservation is available on
+        the instance object.
+
+        The following instance attributes are optional:
+            * ami_launch_index
+            * key_name
+            * kernel_id
+            * product_codes
+            * ramdisk_id
+            * reason
+
+        @param xml_bytes: raw XML payload from AWS.
+        """
+        root = XML(xml_bytes)
+        results = []
+        # May be a more elegant way to do this:
+        for reservation_data in root.find("reservationSet"):
+            # Get the security group information.
+            groups = []
+            for group_data in reservation_data.find("groupSet"):
+                group_id = group_data.findtext("groupId")
+                groups.append(group_id)
+            # Create a reservation object with the parsed data.
+            reservation = model.Reservation(
+                reservation_id=reservation_data.findtext("reservationId"),
+                owner_id=reservation_data.findtext("ownerId"),
+                groups=groups)
+            # Get the list of instances.
+            instances = self.instances_set(
+                reservation_data, reservation)
+            results.extend(instances)
+        return results
+
+    def run_instances(self, xml_bytes):
+        """
+        Parse the reservations XML payload that is returned from an AWS
+        RunInstances API call.
+
+        @param xml_bytes: raw XML payload from AWS.
+        """
+        root = XML(xml_bytes)
+        # Get the security group information.
+        groups = []
+        for group_data in root.find("groupSet"):
+            group_id = group_data.findtext("groupId")
+            groups.append(group_id)
+        # Create a reservation object with the parsed data.
+        reservation = model.Reservation(
+            reservation_id=root.findtext("reservationId"),
+            owner_id=root.findtext("ownerId"),
+            groups=groups)
+        # Get the list of instances.
+        instances = self.instances_set(root, reservation)
+        return instances
+
+    def terminate_instances(self, xml_bytes):
+        """Parse the XML returned by the C{TerminateInstances} function.
+
+        @param xml_bytes: XML bytes with a C{TerminateInstancesResponse} root
+            element.
+        @return: An iterable of C{tuple} of (instanceId, previousState,
+            shutdownState) for the ec2 instances that where terminated.
+        """
+        root = XML(xml_bytes)
+        result = []
+        # May be a more elegant way to do this:
+        for instance in root.find("instancesSet"):
+            instanceId = instance.findtext("instanceId")
+            previousState = instance.find("previousState").findtext(
+                "name")
+            shutdownState = instance.find("shutdownState").findtext(
+                "name")
+            result.append((instanceId, previousState, shutdownState))
+        return result
+
+    def describe_security_groups(self, xml_bytes):
+        """Parse the XML returned by the C{DescribeSecurityGroups} function.
+
+        @param xml_bytes: XML bytes with a C{DescribeSecurityGroupsResponse}
+            root element.
+        @return: A list of L{SecurityGroup} instances.
+        """
+        root = XML(xml_bytes)
+        result = []
+        for group_info in root.findall("securityGroupInfo/item"):
+            name = group_info.findtext("groupName")
+            description = group_info.findtext("groupDescription")
+            owner_id = group_info.findtext("ownerId")
+            allowed_groups = []
+            allowed_ips = []
+            ip_permissions = group_info.find("ipPermissions")
+            if ip_permissions is None:
+                ip_permissions = ()
+            for ip_permission in ip_permissions:
+                ip_protocol = ip_permission.findtext("ipProtocol")
+                from_port = int(ip_permission.findtext("fromPort"))
+                to_port = int(ip_permission.findtext("toPort"))
+                for groups in ip_permission.findall("groups/item") or ():
+                    user_id = groups.findtext("userId")
+                    group_name = groups.findtext("groupName")
+                    if user_id and group_name:
+                        if (user_id, group_name) not in allowed_groups:
+                            allowed_groups.append((user_id, group_name))
+                for ip_ranges in ip_permission.findall("ipRanges/item") or ():
+                    cidr_ip = ip_ranges.findtext("cidrIp")
+                    allowed_ips.append(
+                        model.IPPermission(
+                            ip_protocol, from_port, to_port, cidr_ip))
+
+            allowed_groups = [model.UserIDGroupPair(user_id, group_name)
+                              for user_id, group_name in allowed_groups]
+
+            security_group = model.SecurityGroup(
+                name, description, owner_id=owner_id,
+                groups=allowed_groups, ips=allowed_ips)
+            result.append(security_group)
+        return result
+
+    def truth_return(self, xml_bytes):
+        """Parse the XML for a truth value.
+
+        @param xml_bytes: XML bytes.
+        @return: True if the node contains "return" otherwise False.
+        """
+        root = XML(xml_bytes)
+        return root.findtext("return") == "true"
+
+    def describe_volumes(self, xml_bytes):
+        """Parse the XML returned by the C{DescribeVolumes} function.
+
+        @param xml_bytes: XML bytes with a C{DescribeVolumesResponse} root
+            element.
+        @return: A list of L{Volume} instances.
+        """
+        root = XML(xml_bytes)
+        result = []
+        for volume_data in root.find("volumeSet"):
+            volume_id = volume_data.findtext("volumeId")
+            size = int(volume_data.findtext("size"))
+            status = volume_data.findtext("status")
+            availability_zone = volume_data.findtext("availabilityZone")
+            snapshot_id = volume_data.findtext("snapshotId")
+            create_time = volume_data.findtext("createTime")
+            create_time = datetime.strptime(
+                create_time[:19], "%Y-%m-%dT%H:%M:%S")
+            volume = model.Volume(
+                volume_id, size, status, create_time, availability_zone,
+                snapshot_id)
+            result.append(volume)
+            for attachment_data in volume_data.find("attachmentSet"):
+                instance_id = attachment_data.findtext("instanceId")
+                status = attachment_data.findtext("status")
+                device = attachment_data.findtext("device")
+                attach_time = attachment_data.findtext("attachTime")
+                attach_time = datetime.strptime(
+                    attach_time[:19], "%Y-%m-%dT%H:%M:%S")
+                attachment = model.Attachment(
+                    instance_id, device, status, attach_time)
+                volume.attachments.append(attachment)
+        return result
+
+    def create_volume(self, xml_bytes):
+        """Parse the XML returned by the C{CreateVolume} function.
+
+        @param xml_bytes: XML bytes with a C{CreateVolumeResponse} root
+            element.
+        @return: The L{Volume} instance created.
+        """
+        root = XML(xml_bytes)
+        volume_id = root.findtext("volumeId")
+        size = int(root.findtext("size"))
+        status = root.findtext("status")
+        create_time = root.findtext("createTime")
+        availability_zone = root.findtext("availabilityZone")
+        snapshot_id = root.findtext("snapshotId")
+        create_time = datetime.strptime(
+            create_time[:19], "%Y-%m-%dT%H:%M:%S")
+        volume = model.Volume(
+            volume_id, size, status, create_time, availability_zone,
+            snapshot_id)
+        return volume
+
+    def snapshots(self, xml_bytes):
+        """Parse the XML returned by the C{DescribeSnapshots} function.
+
+        @param xml_bytes: XML bytes with a C{DescribeSnapshotsResponse} root
+            element.
+        @return: A list of L{Snapshot} instances.
+        """
+        root = XML(xml_bytes)
+        result = []
+        for snapshot_data in root.find("snapshotSet"):
+            snapshot_id = snapshot_data.findtext("snapshotId")
+            volume_id = snapshot_data.findtext("volumeId")
+            status = snapshot_data.findtext("status")
+            start_time = snapshot_data.findtext("startTime")
+            start_time = datetime.strptime(
+                start_time[:19], "%Y-%m-%dT%H:%M:%S")
+            progress = snapshot_data.findtext("progress")[:-1]
+            progress = float(progress or "0") / 100.
+            snapshot = model.Snapshot(
+                snapshot_id, volume_id, status, start_time, progress)
+            result.append(snapshot)
+        return result
+
+    def create_snapshot(self, xml_bytes):
+        """Parse the XML returned by the C{CreateSnapshot} function.
+
+        @param xml_bytes: XML bytes with a C{CreateSnapshotResponse} root
+            element.
+        @return: The L{Snapshot} instance created.
+        """
+        root = XML(xml_bytes)
+        snapshot_id = root.findtext("snapshotId")
+        volume_id = root.findtext("volumeId")
+        status = root.findtext("status")
+        start_time = root.findtext("startTime")
+        start_time = datetime.strptime(
+            start_time[:19], "%Y-%m-%dT%H:%M:%S")
+        progress = root.findtext("progress")[:-1]
+        progress = float(progress or "0") / 100.
+        return model.Snapshot(
+            snapshot_id, volume_id, status, start_time, progress)
+
+    def attach_volume(self, xml_bytes):
+        """Parse the XML returned by the C{AttachVolume} function.
+
+        @param xml_bytes: XML bytes with a C{AttachVolumeResponse} root
+            element.
+        @return: a C{dict} with status and attach_time keys.
+        """
+        root = XML(xml_bytes)
+        status = root.findtext("status")
+        attach_time = root.findtext("attachTime")
+        attach_time = datetime.strptime(
+            attach_time[:19], "%Y-%m-%dT%H:%M:%S")
+        return {"status": status, "attach_time": attach_time}
+
+    def describe_keypairs(self, xml_bytes):
+        """Parse the XML returned by the C{DescribeKeyPairs} function.
+
+        @param xml_bytes: XML bytes with a C{DescribeKeyPairsResponse} root
+            element.
+        @return: a C{list} of L{Keypair}.
+        """
+        results = []
+        root = XML(xml_bytes)
+        keypairs = root.find("keySet")
+        if keypairs is None:
+            return results
+        for keypair_data in keypairs:
+            key_name = keypair_data.findtext("keyName")
+            key_fingerprint = keypair_data.findtext("keyFingerprint")
+            results.append(model.Keypair(key_name, key_fingerprint))
+        return results
+
+    def create_keypair(self, xml_bytes):
+        """Parse the XML returned by the C{CreateKeyPair} function.
+
+        @param xml_bytes: XML bytes with a C{CreateKeyPairResponse} root
+            element.
+        @return: The L{Keypair} instance created.
+        """
+        keypair_data = XML(xml_bytes)
+        key_name = keypair_data.findtext("keyName")
+        key_fingerprint = keypair_data.findtext("keyFingerprint")
+        key_material = keypair_data.findtext("keyMaterial")
+        return model.Keypair(key_name, key_fingerprint, key_material)
+
+    def import_keypair(self, xml_bytes, key_material):
+        """Extract the key name and the fingerprint from the result."""
+        keypair_data = XML(xml_bytes)
+        key_name = keypair_data.findtext("keyName")
+        key_fingerprint = keypair_data.findtext("keyFingerprint")
+        return model.Keypair(key_name, key_fingerprint, key_material)
+
+    def allocate_address(self, xml_bytes):
+        """Parse the XML returned by the C{AllocateAddress} function.
+
+        @param xml_bytes: XML bytes with a C{AllocateAddress} root element.
+        @return: The public ip address as a string.
+        """
+        address_data = XML(xml_bytes)
+        return address_data.findtext("publicIp")
+
+    def describe_addresses(self, xml_bytes):
+        """Parse the XML returned by the C{DescribeAddresses} function.
+
+        @param xml_bytes: XML bytes with a C{DescribeAddressesResponse} root
+            element.
+        @return: a C{list} of L{tuple} of (publicIp, instancId).
+        """
+        results = []
+        root = XML(xml_bytes)
+        for address_data in root.find("addressesSet"):
+            address = address_data.findtext("publicIp")
+            instance_id = address_data.findtext("instanceId")
+            results.append((address, instance_id))
+        return results
+
+    def describe_availability_zones(self, xml_bytes):
+        """Parse the XML returned by the C{DescribeAvailibilityZones} function.
+
+        @param xml_bytes: XML bytes with a C{DescribeAvailibilityZonesResponse}
+            root element.
+        @return: a C{list} of L{AvailabilityZone}.
+        """
         results = []
         root = XML(xml_bytes)
         for zone_data in root.find("availabilityZoneInfo"):
 
=== modified file 'txaws/ec2/tests/test_client.py'
--- txaws/ec2/tests/test_client.py	2011-04-21 16:14:58 +0000
+++ txaws/ec2/tests/test_client.py	2011-04-26 16:30:59 +0000
@@ -217,7 +217,7 @@
     def test_parse_reservation(self):
         creds = AWSCredentials("foo", "bar")
         ec2 = client.EC2Client(creds=creds)
-        results = ec2._parse_describe_instances(
+        results = ec2.parser.describe_instances(
             payload.sample_describe_instances_result)
         self.check_parsed_instances(results)
 
 
=== modified file 'txaws/testing/ec2.py'
--- txaws/testing/ec2.py	2011-04-19 16:26:26 +0000
+++ txaws/testing/ec2.py	2011-04-26 16:30:59 +0000
@@ -16,11 +16,12 @@
     def __init__(self, creds, endpoint, instances=None, keypairs=None,
                  volumes=None, key_material="", security_groups=None,
                  snapshots=None, addresses=None, availability_zones=None,
-                 query_factory=None):
+                 query_factory=None, parser=None):
 
         self.creds = creds
         self.endpoint = endpoint
         self.query_factory = query_factory
+        self.parser = parser
 
         self.instances = instances or []
         self.keypairs = keypairs or []