Code review comment for lp:~sandy-walsh/nova/zones2

Revision history for this message
Ed Leafe (ed-leafe) wrote :

235 +def _call_novatools(zone):
236 + """Call novatools. Broken out for testing purposes."""
237 + os = novatools.OpenStack(zone.username, zone.password, zone.api_url)
238 + return
   In this code, I would prefer a name that isn't likely to conflict with common Python module names. While 'os' isn't imported into the namespace for that code, it's a good practice not to use common names, especially when you can pick any name, as they can introduce hard-to-detect bugs.

272 + # Cleanup zones removed from db ...
273 + for zone_id in self.zone_states.keys():
   Simpler: for zone_id in self.zone_states:
(.keys() is redundant)

299 +def zone_get_all_scheduler(x, y, z):
 - and -
308 +def zone_get_all_scheduler_empty(x, y, z):
   Is there a reason for three args named x,y,z?

393 +def exploding_novatools(zone):
   Love it!

454 + zone_state.update_credentials(FakeZone(id=1, api_url='',
455 + username='user1', password='pass1'))
   Fake URLs for testing should use ''. '' is an actual domain name.

« Back to merge proposal