Open Stack

Hi Henry,

On Tue, 2013-08-06 at 22:10 +0100, Henry Nash wrote:
> Hi Mark,
> 
> Of particular interest are your views on the changes to keystone/common/config.py.  The requirement is that we need to be able to instantiate multiple conf objects (built from different sets of config files).  We tried two approaches to this:
> 
> https://review.openstack.org/#/c/39530/11 which attempts to keep the current keystone config helper apps (register_bool() etc.) by passing on the conf instance, and
> https://review.openstack.org/#/c/39530/12 which removes these helper apps and just calls the methods on the conf itself (conf.register_opt())
> 
> Both functionally work, but interested in your views on both approaches.

Definitely prefer more of the latter, since I've proposed it myself
previously :)

  https://review.openstack.org/4547

There are some common patterns of cfg usage which keystone is unusual in
not adopting:

  - declare options as a list, or multiple lists at the top of modules:

      foo_opts = [
          cfg.StrOpt('bar'),
          cfg.ListOpt('foo'),
      ]

  - declare options in the modules in which they're used, rather than 
    having a single module which declares all options for the project. 
    See this blueprint:
 
      https://blueprints.launchpad.net/nova/+spec/scope-config-opts

    for where I moved all of the option declarations out of the 
    nova.config module.

    I recall this being a problem for keystone recently - I think it 
    may have been a keystone.middleware module imported keystone.config 
    which defined logging options which may have been defined 
    elsewhere. This kind of thing is easier to avoid if the config 
    options are scoped to the module which uses them.

  - iff the code in this module needs to only work with cfg.CONF, then 
    register the options with cfg.CONF at the top of the module:

      CONF = cfg.CONF
      CONF.register_opts(foo_opts)

    otherwise register the options before they're used:

      def bar(conf, ..):
          ...
          conf.register_opts(foo_opts)
          if conf.foo:
              ...

      def blaa(conf, ..):
          ...
          conf.register_opts(foo_opts)
          if conf.bar:
              ...

Hope that helps,
Mark.