Open Stack

On 07/24/2015 01:48 AM, 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.
>
> 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.
>
> 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.

Amen to everything you say above, Daniel. I've long hated the global 
CONF variables. In fact, in pulling the os_vif code out of Nova [1], I 
made a point of removing all use of the global CONF variables.

-jay

[1] http://github.com/jaypipes/os_vif