openstack-common

No way to set_default or set_override a config with "None"

Bug #1035478 reported by Vish Ishaya on 2012-08-10

This bug affects 1 person

Affects		Status	Importance	Assigned to	Milestone
	openstack-common	Fix Released	Undecided	Vish Ishaya	openstack-common 2012.2 "folsom"

Bug Description

if you have a config defined like so

cfg.StrConfig('foo', default='bar')

there is no way to override it with a None value:

cfg.set_default('foo', None)
cfg.set_override('foo', None)
cfg.foo == 'bar'

This seems to be because the internal logic for set_default and set_override mean that passing None will clear the existing value.

Vish Ishaya (vishvananda) on 2012-08-10

Changed in openstack-common:
status:	New → In Progress
assignee:	nobody → Vish Ishaya (vishvananda)

Revision history for this message

Vish Ishaya (vishvananda) wrote on 2012-08-10:

Three options for fixing. I'm using set_default in the examples, but it applies to set_override as well.

1) make set_default('foo', None) override the default with the value None and provide a clear_default('foo') method to remove it. Internally we store the NoneValue with a special class.

2) provide a NoneValue class that can be used to override the default value with a None, set_default('foo', NoneValue()) will override the internal value. set_default('foo', None) will clear the default as it does today.

3) allow set_default to take a method. When the value is retrieved, the method is lazily evaluated. You could do something like set_default('foo', lambda: None) to override a value with None. set_default('foo', None) will clear the default as it does today. This definitely is interesting but it seems a little complex and Non-obvious

Option 1 changes the existing api very slightly, but it seems more correct. In order to keep compatibility and keep things simple I've implemented 2). Open to suggestions on the review

Revision history for this message

OpenStack Infra (hudson-openstack) wrote on 2012-08-10: Fix proposed to openstack-common (master)

Fix proposed to branch: master
Review: https://review.openstack.org/11185

Revision history for this message

OpenStack Infra (hudson-openstack) wrote on 2012-08-13: Fix merged to openstack-common (master)

Reviewed: https://review.openstack.org/11185
Committed: http://github.com/openstack/openstack-common/commit/513bd3a917207099125cc044705aee438fee7143
Submitter: Jenkins
Branch: master

commit 513bd3a917207099125cc044705aee438fee7143
Author: Vishvananda Ishaya <email address hidden>
Date: Fri Aug 10 14:28:59 2012 -0700

Allow set_default and set_override to use None

    The current implementation interprets set_default('foo', None) and
    set_override('foo', None) as 'clear the existing default or override',
    which makes it impossible to override a value with None.

    This patch adds support for overriding with a None value by adding
    a special internal class. set_override('foo', None) will now override
    the existing value with None. This is a slight change to the existing
    behavior, so this patch adds two calls for the old functionality of
    clearing defaults and overrides. Example syntax for the new calls
    are shown below:

conf.clear_default('foo')
conf.clear_override('foo')

The patch also updates the tests to reflect the change in functionality
and adds new tests to verify the new functionality.

Fixes bug 1035478

Change-Id: Iee5e20e44da9bef6b86e0483ab0b48b625fe503c