openstack team mailing list archive

[Joseph Heck <heckj@xxxxxxx>]

I personally don't have a huge preference one way or the other. I've used a global object for configuration in applications, and find it immensely convenient, and the methods that Mark (and others) have put into the openstack/common/cfg seem pretty amenable to adding on options as needed - top of code or within modules as relevant.

If keystone wants to use it as a global, that doesn't necessarily mean that Glance, Nova, Swift, etc. has to. Is it far to extend the usage patterns so that we can clear state, etc. for working with it as a global object?

Most importantly to me, there are two rather nasty outstanding bugs blocking RC1 in Keystone that are pending the outcome of this conversation:
 * https://bugs.launchpad.net/keystone/+bug/942793
 * https://bugs.launchpad.net/keystone/+bug/949373

with a set of changes from Mark all pending and ready to roll: https://review.openstack.org/#change,4547 (and several dependent on that one)

- joe

On Mar 9, 2012, at 7:57 AM, Mark McLoughlin wrote:
> Hey,
> 
> On Tue, 2012-03-06 at 14:44 -0800, Andy Smith wrote:
>> This is going to be a somewhat verbose reply.
> 
> Positively terse by my standards :)
> 
>> TL;DR:
>> 
>> Summary, I'd like cfg.py to be config.py (pipe dream, I'm sure, but I
>> don't really agree with the abbreviation), to provide a global
>> configuration object, and to have shortcuts for registration of
>> options, that projects should standardize on. And I'm willing to help
>> make those changes.
> 
> AFAICT, the only real issue here is the use of a global object.
> 
> Personally, I think such global objects are in very poor taste.
> 
> But this isn't so much about my taste as agreeing on a pattern which all
> projects are happy to follow. If I thought there was consensus around
> using a global object, I'd happily grit my teeth and make it happen.
> 
> Here's the problem, though - I can't imagine glance-core folks welcoming
> a patch to adopt a global config object. it seems like a completely
> backwards move. Maybe that's just me?
> 
>> On Tue, Mar 6, 2012 at 2:10 AM, Mark McLoughlin <markmc@xxxxxxxxxx> wrote:
>> 
>>> Hey,
>>> 
>>> The original cfg design[1] assumed certain usage patterns that I hoped
>>> would be adopted by all projects using it. In gerrit, we're debating a
>>> set of patch to make keystone use these patterns:
>>> 
>>> https://review.openstack.org/4547
> [..]
>>> 1) We stop using global objects like FLAGS or CONF and instead pass
>>>    the config object to code which uses it.
>>> 
>>>    On an evilness scale, these global objects may be relatively
>>>    benign but they do still harm modularity. Any module that depends
>>>    on these global objects cannot be re-used without ensuring the
>>>    global object exists in the user e.g. doing this:
>>> 
>>>      from nova import flags
>>> 
>>>      FLAGS = flags.FLAGS
>>> 
>>>    is a nice way of ensuring code is nova specific, even if there is
>>>    no other reason for it to be tied to nova.
>>> 
>> 
>> 
>> This is not true if the cfg stuff is providing the global object,
>> as then it will be provided by the library rather than the project.
> 
> Making the library provide a global object is certainly a great way of
> ratcheting up the evilness :)
> 
> Say I've two independent modules in the same process which want to parse
> two separate config files? One of them uses the global object and the
> other does something different? How about if the author of the both
> modules never considered that it would be combined in the same process
> with other cfg using code?
> 
> I could see us wanting to do something like this with the keystone auth
> middleware, for example.
> 
> [..]
>>>    Also, these global objects force us to do a bunch of hacks in unit
>>>    tests. We need to do tricks to ensure the object is initialized as
>>>    we want. We also need to save and restore its state between runs.
>>> 
>> 
>> I don't agree with the statement that they force "a bunch of hacks,"
>> clearing state is a perfectly normal thing to do, it is done for any
>> servers that get started, any mocks that are made, and every test
>> database. Making sure that modifications to a configuration object are
>> cleaned up is no different: there is no "save and restore" just always
>> start from a blank slate and set things as required, same as in the
>> non-global model.
> 
> Creating mocks, DB connection objects or re-starting servers on each
> test run is different from trying to restore a global object to its
> state before a test run.
> 
> The CONF object is initialized the first time the config module is
> imported. Each test run you need to attempt to reset the object back to
> its original pristine state. Compared to just creating a config object
> for each run, I do think that's a hack.
> 
>>>    Subtle mistakes here might mean that a test passes when all the
>>>    tests are run together, but fail when run individually[2].
>>> 
>>> 
>> Subtle mistakes are still subtle mistakes, though the commit you
>> reference was really a pretty large one (and the solution is incorrect
>> as well).
> 
> The issue was subtle enough that it went unnoticed for 3.5 months. And
> previously, before the code was moved out of test_servers.py, there was
> an even hackier attempt to restore the allow_admin_api flag after each
> test. The global object has quite clearly been a source of confusion and
> subtle brokenness.
> 
> [Snip discussion of tearDown() method strategies which I don't disagree
> with]
> 
>> On the benefits of using global configuration:
>> 
>> - simple to use, easy concept
> 
> Simple or not, global objects are probably only second to goto
> statements in "considered harmful"[1] stakes.
> 
> Like goto statements, they can be an elegant solution in certain
> restricted cases - e.g. goto can be good for error handling, globals can
> be good for constants - but I think this case definitely crosses the
> line over into evilness.
> 
>> - different components do not get different configurations
> 
> Different components may want different configurations.
> 
>> - more concise
>> - no additional patterns required besides standard top-of-file boilerplate
> 
> The top-of-file boilerplate may be slightly more concise, but it's
> inelegant because of the dependence on globals.
> 
> [..]
> 
>>> 2) We define options close to where they are used.
>>> 
>>>    Again, this is for modularity. If some code relies on code in
>>>    another module registering an option, it creates a dependence
>>>    between the modules.
>>> 
>> 
>> This has never been an issue in my mind, so if you thought it was I may have
>> miscommunicated.
> 
> Yep, I understood that. I guess I was just raising it because it's the
> combination of this and removing the global object which results in the
> usage patterns I wanted to discuss.
> 
> [..]
>>> Here you have the options used by the class clearly declared at the
>>> beginning of the class definition and a config instance passed to the
>>> constructor.
>>> 
> [..]
>>> Here you have the options declared at the beginning of the module and a
>>> config instance passed to functions which use options. The option schema
>>> is registered by such a function just before the option is used.
>>> 
>> 
>> My preference here is to define all options at the top of the file, largely
>> because one is often looking for a config option, possibly to figure out
>> what its default is, usually to figure out where to change its default, and
>> having them defined at the top of the file makes sure they are always where
>> you expect them to be.
> 
> I think the same is equally true of having them at the top of a class,
> but by putting them in the scope of the class (where possible) makes it
> easier to refactor the class into another module i.e.
> 
>  foo
>  bar
>  blaa
> 
>  class A:
>    ..
>  class B:
>    ...
> 
> vs
> 
>  class A:
>    foo
>    bar
> 
>  class B:
>    blaa
> 
> The latter definitely makes more sense to me.
> 
> [...]
> 
> Cheers,
> Mark.
> 
> [1] - http://en.wikipedia.org/wiki/Considered_harmful
> 
> 
> _______________________________________________
> Mailing list: https://launchpad.net/~openstack
> Post to     : openstack@xxxxxxxxxxxxxxxxxxx
> Unsubscribe : https://launchpad.net/~openstack
> More help   : https://help.launchpad.net/ListHelp