make registering a cli opt on a filter object work.

Change the behaviour of ConfigFilter.register_cli_opt to:

1. If the opt is already registered before parsing, then registering
just import it.
2. if the opt is not registered before, then raise an exception named

CliOptRegisteredError.
By adding this modification, the behaviour of
ConfigFilter.register_cli_opt looks more consistent to
ConfigOpts.register_cli_opt. The solution to the question mentioned in
Bug#1366946 should be like this:

from oslo_config import cfg
from oslo_config import cfgfilter

import sys

c = cfg.CONF
c.register_cli_opt(
cfg.BoolOpt('myflag',
default=False,
help='turn on myflag',
)
)
c(sys.argv[1:])

f = cfgfilter.ConfigFilter(c)
f.register_cli_opt(
cfg.BoolOpt('myflag',
default=False,
help='turn on myflag',
)
)

print f.myflag

Closes-Bug: #1366946
Change-Id: I94df9409f72807461370b4aaf8eb2543c52a89bb
This commit is contained in:
Tong Damon Da 2015-04-09 13:19:28 +08:00
parent a8db68bd43
commit f1f972aeb7
3 changed files with 79 additions and 31 deletions

@ -131,8 +131,19 @@ import itertools
from oslo_config import cfg
class ConfigFilter(collections.Mapping):
class CliOptRegisteredError(cfg.Error):
"""Raised when registering cli opt not in original ConfigOpts."""
def __str__(self):
ret = "Cannot register a cli option that was not present in the" \
" original ConfigOpts."
if self.msg:
ret += ": " + self.msg
return ret
class ConfigFilter(collections.Mapping):
"""A helper class which wraps a ConfigOpts object.
ConfigFilter enforces the explicit declaration of dependencies on external
@ -235,11 +246,13 @@ class ConfigFilter(collections.Mapping):
"""
if self._already_registered(self._conf, opt, group):
# Raises DuplicateError if there is another opt with the same name
ret = self._conf.register_cli_opt(opt, group)
ret = self._conf.register_opt(
opt, group, cli=True, clear_cache=False)
self._import_opt(opt.dest, group)
return ret
else:
return self._fconf.register_cli_opt(opt, group)
raise CliOptRegisteredError(
"Opt '{0}' cannot be registered.".format(opt.dest))
def register_cli_opts(self, opts, group=None):
"""Register multiple CLI option schemas at once."""

@ -13,6 +13,7 @@
# under the License.
from oslotest import base as test_base
import testtools
from oslo_config import cfg
from oslo_config import cfgfilter
@ -116,17 +117,31 @@ class RegisterTestCase(BaseTestCase):
self.assertNotIn('foo', self.conf)
self.assertNotIn('bar', self.conf)
def test_register_cli_opt(self):
def test_register_known_cli_opt(self):
self.conf.register_opt(cfg.StrOpt('foo'))
self.fconf.register_cli_opt(cfg.StrOpt('foo'))
self.assertIn('foo', self.fconf)
self.assertNotIn('foo', self.conf)
self.assertIn('foo', self.conf)
def test_register_cli_opts(self):
def test_register_unknown_cli_opt(self):
with testtools.ExpectedException(cfgfilter.CliOptRegisteredError):
self.fconf.register_cli_opt(cfg.StrOpt('foo'))
def test_register_known_cli_opts(self):
self.conf.register_cli_opts([cfg.StrOpt('foo'), cfg.StrOpt('bar')])
self.fconf.register_cli_opts([cfg.StrOpt('foo'), cfg.StrOpt('bar')])
self.assertIn('foo', self.fconf)
self.assertIn('bar', self.fconf)
self.assertNotIn('foo', self.conf)
self.assertNotIn('bar', self.conf)
self.assertIn('foo', self.conf)
self.assertIn('bar', self.conf)
def test_register_unknown_cli_opts(self):
self.conf.register_cli_opt(cfg.StrOpt('foo'))
with testtools.ExpectedException(cfgfilter.CliOptRegisteredError):
self.fconf.register_cli_opts([
cfg.StrOpt('foo'),
cfg.StrOpt('bar')
])
def test_register_opts_grouped(self):
self.fconf.register_opts([cfg.StrOpt('foo'), cfg.StrOpt('bar')],
@ -135,17 +150,34 @@ class RegisterTestCase(BaseTestCase):
self.assertIn('bar', self.fconf.blaa)
self.assertNotIn('blaa', self.conf)
def test_register_cli_opt_grouped(self):
def test_register_known_cli_opt_grouped(self):
self.conf.register_cli_opt(cfg.StrOpt('foo'), group='blaa')
self.fconf.register_cli_opt(cfg.StrOpt('foo'), group='blaa')
self.assertIn('foo', self.fconf.blaa)
self.assertNotIn('blaa', self.conf)
self.assertIn('blaa', self.fconf)
self.assertIn('blaa', self.conf)
def test_register_cli_opts_grouped(self):
def test_register_unknown_cli_opt_grouped(self):
with testtools.ExpectedException(cfgfilter.CliOptRegisteredError):
self.fconf.register_cli_opt(cfg.StrOpt('foo'), group='blaa')
def test_register_known_cli_opts_grouped(self):
self.conf.register_cli_opts([cfg.StrOpt('foo'), cfg.StrOpt('bar')],
group='blaa')
self.fconf.register_cli_opts([cfg.StrOpt('foo'), cfg.StrOpt('bar')],
group='blaa')
self.assertIn('foo', self.fconf.blaa)
self.assertIn('bar', self.fconf.blaa)
self.assertNotIn('blaa', self.conf)
self.assertIn('blaa', self.fconf)
self.assertIn('blaa', self.conf)
def test_register_unknown_opts_grouped(self):
self.conf.register_cli_opts([cfg.StrOpt('bar')], group='blaa')
with testtools.ExpectedException(cfgfilter.CliOptRegisteredError):
self.fconf.register_cli_opts([
cfg.StrOpt('foo'),
cfg.StrOpt('bar')
], group='blaa')
def test_unknown_opt(self):
self.assertNotIn('foo', self.fconf)
@ -223,20 +255,18 @@ class RegisterTestCase(BaseTestCase):
self.conf.register_cli_opts([cfg.StrOpt('foo'),
cfg.StrOpt('fu')])
self.fconf.register_cli_opts([cfg.StrOpt('foo'),
cfg.StrOpt('bu')])
cfg.StrOpt('fu')])
self.assertIn('foo', self.conf)
self.assertIn('fu', self.conf)
self.assertNotIn('bu', self.conf)
self.assertEqual(2, len(self.conf))
self.assertIsNone(self.conf.foo)
self.assertIsNone(self.conf.fu)
self.assertIn('foo', self.fconf)
self.assertIn('bu', self.fconf)
self.assertNotIn('fu', self.fconf)
self.assertIn('fu', self.fconf)
self.assertEqual(2, len(self.fconf))
self.assertIsNone(self.fconf.foo)
self.assertIsNone(self.fconf.bu)
self.assertIsNone(self.fconf.fu)
self.conf.set_override('foo', 'bar')

@ -116,17 +116,19 @@ class RegisterTestCase(BaseTestCase):
self.assertNotIn('foo', self.conf)
self.assertNotIn('bar', self.conf)
def test_register_cli_opt(self):
def test_register_known_cli_opt(self):
self.conf.register_opt(cfg.StrOpt('foo'))
self.fconf.register_cli_opt(cfg.StrOpt('foo'))
self.assertIn('foo', self.fconf)
self.assertNotIn('foo', self.conf)
self.assertIn('foo', self.conf)
def test_register_cli_opts(self):
def test_register_known_cli_opts(self):
self.conf.register_cli_opts([cfg.StrOpt('foo'), cfg.StrOpt('bar')])
self.fconf.register_cli_opts([cfg.StrOpt('foo'), cfg.StrOpt('bar')])
self.assertIn('foo', self.fconf)
self.assertIn('bar', self.fconf)
self.assertNotIn('foo', self.conf)
self.assertNotIn('bar', self.conf)
self.assertIn('foo', self.conf)
self.assertIn('bar', self.conf)
def test_register_opts_grouped(self):
self.fconf.register_opts([cfg.StrOpt('foo'), cfg.StrOpt('bar')],
@ -135,17 +137,22 @@ class RegisterTestCase(BaseTestCase):
self.assertIn('bar', self.fconf.blaa)
self.assertNotIn('blaa', self.conf)
def test_register_cli_opt_grouped(self):
def test_register_known_cli_opt_grouped(self):
self.conf.register_cli_opt(cfg.StrOpt('foo'), group='blaa')
self.fconf.register_cli_opt(cfg.StrOpt('foo'), group='blaa')
self.assertIn('foo', self.fconf.blaa)
self.assertNotIn('blaa', self.conf)
self.assertIn('blaa', self.fconf)
self.assertIn('blaa', self.conf)
def test_register_cli_opts_grouped(self):
def test_register_known_cli_opts_grouped(self):
self.conf.register_cli_opts([cfg.StrOpt('foo'), cfg.StrOpt('bar')],
group='blaa')
self.fconf.register_cli_opts([cfg.StrOpt('foo'), cfg.StrOpt('bar')],
group='blaa')
self.assertIn('foo', self.fconf.blaa)
self.assertIn('bar', self.fconf.blaa)
self.assertNotIn('blaa', self.conf)
self.assertIn('blaa', self.fconf)
self.assertIn('blaa', self.conf)
def test_unknown_opt(self):
self.assertNotIn('foo', self.fconf)
@ -223,20 +230,18 @@ class RegisterTestCase(BaseTestCase):
self.conf.register_cli_opts([cfg.StrOpt('foo'),
cfg.StrOpt('fu')])
self.fconf.register_cli_opts([cfg.StrOpt('foo'),
cfg.StrOpt('bu')])
cfg.StrOpt('fu')])
self.assertIn('foo', self.conf)
self.assertIn('fu', self.conf)
self.assertNotIn('bu', self.conf)
self.assertEqual(2, len(self.conf))
self.assertIsNone(self.conf.foo)
self.assertIsNone(self.conf.fu)
self.assertIn('foo', self.fconf)
self.assertIn('bu', self.fconf)
self.assertNotIn('fu', self.fconf)
self.assertIn('fu', self.fconf)
self.assertEqual(2, len(self.fconf))
self.assertIsNone(self.fconf.foo)
self.assertIsNone(self.fconf.bu)
self.assertIsNone(self.fconf.fu)
self.conf.set_override('foo', 'bar')