Doug Hellmann wrote:
Excerpts from Joshua Harlow's message of 2015-07-24 09:07:13 -0700:
Daniel P. Berrange wrote:
On Thu, Jul 23, 2015 at 05:55:36PM +0300, mhorban wrote:
Hi all,
During development process in nova I faced with an issue related with config
options. Now we have lists of config options and registering options mixed
with source code in regular files.
From one side it can be convenient: to have module-encapsulated config
options. But problems appear when we need to use some config option in
different modules/packages.
If some option is registered in module X and module X imports module Y for
some reasons...
and in one day we need to import this option in module Y we will get
exception
NoSuchOptError on import_opt in module Y.
Because of circular dependency.
To resolve it we can move registering of this option in Y module(in the
inappropriate place) or use other tricks.
I offer to create file options.py in each package and move all package's
config options and registration code there.
Such approach allows us to import any option in any place of nova without
problems.
Implementations of this refactoring can be done piece by piece where piece
is
one package.
What is your opinion about this idea?
I tend to think that focusing on problems with dependancy ordering when
modules import each others config options is merely attacking a symptom
of the real root cause problem.
Amen to that :)
The way we use config options is really entirely wrong. We have gone
to the trouble of creating (or trying to create) structured code with
isolated functional areas, files and object classes, and then we throw
in these config options which are essentially global variables which are
allowed to be accessed by any code anywhere. This destroys the isolation
of the various classes we've created, and means their behaviour often
based on side effects of config options from unrelated pieces of code.
It is total madness in terms of good design practices to have such use
of global variables.
So IMHO, if we want to fix the real big problem with config options, we
need to be looking to solution where we stop using config options as
global variables. We should change our various classes so that the
neccessary configurable options as passed into object constructors
and/or methods as parameters.
As an example in the libvirt driver.
I would set it up so that /only/ the LibvirtDriver class in driver.py
was allowed to access the CONF config options. In its constructor it
would load all the various config options it needs, and either set
class attributes for them, or pass them into other methods it calls.
So in the driver.py, instead of calling CONF.libvirt.libvirt_migration_uri
everywhere in the code, in the constructor we'd save that config param
value to an attribute 'self.mig_uri = CONF.libvirt.libvirt_migration_uri'
and then where needed, we'd just call "self.mig_uri".
Now in the various other libvirt files, imagebackend.py, volume.py
vif.py, etc. None of those files would /ever/ access CONF.*. Any time
they needed a config parameter, it would be passed into their constructor
or method, by the LibvirtDriver or whatever invoked them.
+1 and IMHO if some driver needs some 'funky' new parameter that isn't
getting passed previously, probably the driver may be built wrong, or
it's not conforming to the API that is provided (and therefore either
the API needs adjustments or the driver needs to be fixed); all the
above IMHO is pretty standard OOP practices and I'd like to see more of
it honestly :)
While it's good to share common options as much as possible, there
are a lot of perfectly legitimate reasons for drivers to need
different configuration options. Different backends might use different
authentication mechanisms, or need different ways to specify "where" the
thing the driver is managing actually lives. The job of the driver API
layer is to hide those differences so the application doesn't have to
care about them. Pushing all of the options up to be API parameters
defeats the purpose of that if not all drivers use all of the options.
I get what u are saying, but at that point it does make me wonder what
the API that is being provided really is, if the API is trying to hide
those differences, and all drivers are different (and/or require
different options via config) then I'm not really sure u can call that a
common/shared API, especially if the sole option the API takes is a
config object.
I reckon this to usage of void* pointer in c/c++ (where the void* is
nearly equivalent to the config object we pass around), having an API
that takes a single void* pointer and then lets other functions do
things/anything with it isn't exactly a API (by my definition).
But if the API expects some standard arguments, and then as a final
parameter takes a extra void* pointer, then this starts to become more
acceptable IMHO (the extra void* pointer could be considered equivalent
to **kwargs in python and the implementations of that can use it to
receive other options that they use to specialize themselves). It's a
balance between just sending one void* pointer in (not really an API by
my definition) between expecting say X common API parameters and then a
single void* for extra options that are API derivative specific.
I've found the following to be an interesting read on a pluggable system
that most people have used that I think does a pretty good job of this
(and doesn't just default to passing one void* pointer around):
http://fabiensanglard.net/doom3/index.php (most components of doom3 are
pluggable).
Doug
Getting rid of the global CONF object usage in all these files trivially
now solves the circular dependancy import problem, as well as improving
the overall structure and isolation of our code, freeing all these methods
from unexpected side-effects from global variables.
Regards,
Daniel
__________________________________________________________________________
OpenStack Development Mailing List (not for usage questions)
Unsubscribe: [email protected]?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
__________________________________________________________________________
OpenStack Development Mailing List (not for usage questions)
Unsubscribe: [email protected]?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
