Excerpts from Daniel P. Berrange's message of 2015-07-24 15:11:19 +0100: > On Fri, Jul 24, 2015 at 09:56:41AM -0400, Doug Hellmann wrote: > > Excerpts from Daniel P. Berrange's message of 2015-07-24 09:48:15 +0100: > > > 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. > > > > We've tried to do this in a lot of places throughout Oslo. It mostly > > works, until you hit a driver that uses options that the other drivers > > don't (oslo.messaging has this problem especially). > > > > We had a ConfigFilter class in oslo.config to enforce the isolation (an > > option registered on a filter object is only visible to the code that > > owns the filter). However, that causes challenges for value > > interpolation. A deployer doesn't know which options are visible to each > > other, and has a flat file where they can clearly see all of the > > values together, so they expect %(foo)s to work no matter where "foo" is > > defined. Until we solve those issues, the ConfigFilter isn't really > > supported. > > > > The best solution we've come up with so far is to isolate the options > > into groups based on the driver name, and then not allow code outside of > > the driver to access those options for any reason. > > > > To deal with import ordering issues, we pass ConfigObj instance > > around, and register options at runtime instead of import time, > > usually in an __init__ method. Registering an option more than > > once is fine. > > Yep, I would imagine that the nova/cmd/*.py command line entry > points would trigger loading of the config file and pass the > object into the service they run. The service may pass that > config object on down to some classes, eg I'd expect the nova > ComputeDriver to accept a config object in its constructor. > Likewise the various manager classes like nova/compute/manager.py > These would read the various config option values they care about > and pass those into the various methods / objects that need them > > > > 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". > > > > There are, from time to time, specs proposed to provide ways for > > applications to reload their configuration settings without restarting. > > So far we've said that was an application issue, because oslo.config > > can reload the files, but it doesn't know when (that's an app signal > > handling thing) and it doesn't know where values might have been > > saved like you describe. I don't know if that's something the nova > > team wants to support. > > I don't think you can magically reload stuff on the fly regardless > of how we store or access the configuration settings, and in fact > I think our current global variable approach makes it even harder > to deal with that too. > > It is not uncommon to read a config parameter and use that to > configure some state somewhere. The CONF object is magically > updated to have the new config value when the user editted the > file. Now we have some code reading CONF.foo directly and acting > on it, and other code using the previously setup state based on > the old CONF.foo value. This will certainly crash & burn in a > horrible way. > > To be able to support live updates of config parameters, we need > to make the code using those parameters aware of it and ensure the > change takes effect everywhere that's needed at the same time. > If the config options are merely used to set attributes on classes > or passed in as method parameters, we can clearly understand the > implications of changing the config value on the fly, and call a > suitable method to change it. This is really all just standard > good design practice for object oriented programming and why mixing > in use of global variables with OO design is a horrible idea in > general.
Yep, that's why we consider it an application concern, and not a library concern. I only raise the point in case you hadn't seen those requests, since it would mean doing some work either way. Doug __________________________________________________________________________ OpenStack Development Mailing List (not for usage questions) Unsubscribe: [email protected]?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
