← Back to team overview

txawsteam team mailing list archive

Re: [Merge] lp:~oubiwann/txaws/415486-reservation-object into lp:txaws

 

Review: Approve
[1]

+    def __init__(self, reservation_id, owner_id, groups=[], instances=[]):

The default empty list values for groups and instances will be
inadvertently shared by all Reservation instances that don't provide
explicit values during instantiation.  I recommend you use None and
set the value in the body of the constructor as necessary.


[2]

+    def test_parse_reservation(self):
+        ec2 = client.EC2Client(creds='foo')
+        results = ec2._parse_reservation(sample_describe_instances_result)
+        self.check_parsed_reservations(results)

It would be nice to avoid accessing the private _parse_reservation
method by passing a query_factory to EC2Client that returns a
FakeQuery.  FakeQuery.submit could return a succeeded deferred and
you could call ec2.describe_instances to get a Deferred with the
result you want to make assertions about.


+1 considering #1.


-- 
https://code.edge.launchpad.net/~oubiwann/txaws/415486-reservation-object/+merge/10338
Your team txAWS Team is subscribed to branch lp:txaws.



Follow ups

References