Code review comment for lp:~bcwaldon/nova/lp713144
Revision history for this message
| Brian Waldon (bcwaldon) wrote | # |
© 2004
Canonical Ltd.
•
Terms of use
•
Data privacy
•
Contact Launchpad Support
•
Blog
•
Careers
•
System status
•
a60fb26
(Get the code!)
Thanks for the feedback. Here's what I did:
1) I removed the pass statements. I guess I had them there before I added the log lines, and didn't think about it when I committed. Does it make sense to log on KeyError in the private ip section of my code? I know an instance won't have a private ip immediately when the api returns successfully, and I'm just not sure what the state of that dict will be coming up from the db api code.
2) The list comprehension has been replaced with a for loop. The code is essentially just reversed, but now it doesn't build an unnecessary list.
3) I reordered the datetime import. I hadn't realized they were supposed to be in alphabetical order.
Brian Waldon
-----Original Message-----
From: "Matthew Dietz" <email address hidden>
Sent: Wednesday, February 16, 2011 3:08pm
To: "Dan Prince" <email address hidden>
Subject: Re: [Merge] lp:~bcwaldon/nova/lp713144 into lp:nova
Review: Needs Fixing
Hi Brian!
The passes on lines 28 and 36 seem superfluous.
The list comprehension on 32-33 is pretty odd and a bit unreadable IMO. Perhaps even ruby-ish ;-) Additionally, you're effectively building a list of stuff that you're immediately throwing away, in addition to the list you're actually appending to. I'd like to see this refactored to something a bit cleaner.
Also datetime should be alphabetically listed before json in test_servers.py
--
https:/
You are the owner of lp:~bcwaldon/nova/lp713144.