Code review comment for lp:~rconradharris/nova/backup_schedule_extension

Revision history for this message
Brian Lamar (blamar) wrote :

35 + import tempo2.client

Is it tempo.client or tempo2.client?

34 +try:
35 + import tempo2.client
36 + tempo_available = True
37 +except ImportError:
38 + tempo_available = False
39 + LOG.warn(_("Unable to import tempo, backup schedules API extension will"
40 + " not be available."))

137 + if tempo_available:
138 + self.tempo_client = tempo.client.TempoClient(FLAGS.tempo_url)

Looks good, however if tempo isn't installed, tempo_available = False, and the extension is loaded anyway the error most likely will occur as a AttributeError indicating that self.tempo_client isn't defined.

Might I suggest:

137 + if tempo_available:
138 + self.tempo_client = tempo.client.TempoClient(FLAGS.tempo_url)
139 + else:
140 + raise ImportError("tempo.client unable to be imported.")

Or alternatively:

try:
    import tempo.client as tempo_client
except ImportError:
    tempo_client = None

...

if tempo_client:
    self.tempo_client = tempo_client.TempoClient(FLAGS.tempo_url)
else:
    raise ImportError("tempo.client unable to be imported.")

25 +from nova import compute
134 + self.compute_api = compute.API()

I think these can be removed.

Looks great otherwise, sure we can get this in soon.

« Back to merge proposal