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

Thanks for the comments guys.

> > > Having `import tempo` in the `__init__` means that it won't be imported
> > unless the Backup_schedule
> > > class is actually created.
> >
> > Right, but we're putting Tempo in pip-requires and in the Ubuntu package
> > right? So this should be considered a core extension (I'll attempt to not
> get
> > started on the hilarity of that statement to me).

Good points here. Technically, `tempo` doesn't need to be in the pip-requires since, as mentioned above, the Schedule class isn't actually constructed (yet).

I reflexively added it to the `pip-requires` not for the venv-construction, but really just to indicate a dependency.

Perhaps we should separate out core-dependencies vs. optionals ones (as you allude to): maybe pip-optional-requires (ignore the oxymoron there ;-).

All that said, I see your point and agree.

> Indeed, which is why I think the term "extension" has been a little bit abused
> in Nova ;)
>
> > If it were an 'optional
> > extension' it would be packaged separately to manage dependencies.
>
> Indeed.

Yep, see above.

> > > Since API extensions are eager-loaded--everything in the 'contrib'
> directory
> > is imported in search
> > > of an Extension class--I think it makes sense not to have 3rd party
> imports
> > in the module-scope.
> >
> > This extension is in the base source code checkout, so if I checkout the
> Nova
> > source, and do ./run_tests.sh it's going to require tempo, right? If so it's
> a
> > requirement of the project unless we're up front about it and make the tests
> > skip if tempo isn't installed.
>
> ++
>
> > > If we did, that would mean that users would be required to have every
> > dependency of every
> > > (optional) extension. I don't think that scales.
> >
> > No argument here. I really don't want to have tempo in pip-requires or the
> > Ubuntu package if I'm not going to use it. Since it's there (or going to be
> > there?), shouldn't we just show the imports up front?
> >
> > My main argument though is that imports should always be up top in my
> opinion,
> > even if you have to do something like:
> >
> > try:
> > import tempo
> > except ImportError:
> > LOG.warn("Tempo not installed, backup extension won't work.")
>
> Agree fully.
> -jay

Yeah, I personally can go either way on that; but since HACKING has an opinion on this, you guys are right, this should go up at the top. I'll make that change.