On Fri, Jul 24, 2015 at 12:29:56PM -0400, 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.
Agreed, that's why I suggested the ComputeDriver constructor would probably just accept a config object - each subclass has valid reasons for wanting its own set of config parameters in this case. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| __________________________________________________________________________ OpenStack Development Mailing List (not for usage questions) Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev