Code review comment for lp:~allenap/launchpad/ec2-test-race-bug-422433

Revision history for this message
Gavin Panella (allenap) wrote :

Thanks for the review!

> Hi Gavin,
>
> a very nice branch! Just one suggestion:
>
...
>
> The for loops in both methods are nearly identical; the only differences I see
> are that the log messages contain "key pair" or "secuity group", and that
> try_delete_key_pair() or try_delete_security_group() is called. Would it make
> sense to move the code starting at "if session_name" and ending at "else:
> self.log(..)" into a common method?

Good point. I've done this, and also ditched a lot of the noisy
logging; it now says something only if a key pair or security group is
deleted or if the deletion fails.

Incremental diff: http://pastebin.ubuntu.com/283801/

Thanks, Gavin.

« Back to merge proposal