openstack/oslo.config
Code Issues Proposed changes

Fix MultiConfigParser API breakage

Browse Source 
Commit c952491 made an incompatible change to MultiConfigParser's
behaviour. The code in Quantum which was relying on the behaviour did
the following:

    p = MultiConfigParser()
    multi_parser.read(conf_file)

    for parsed_file in multi_parser.parsed:
        for section in parsed_file.keys():
            if section.startswith("CLUSTER:"):
                ...

However, we made a change such that the keys of the 'parsed' dict were
normalized to lowercase. Restore the previous behaviour and add another
dict for tracking the values with normalized section names. We also
add a normalized=False kwarg to the get() method so that we also restore
the previous behaviour of the method by default.

This shows the need to aggressively make implementation details as not
part of the public API of our libraries. In this case, I considered
MultiConfigParser as an implementation detail and that no projects would
be using it. But, sure enough, if it's public then *someone* will use
it.

Also, add a bunch of test cases to actually test the behaviour of this
class in isolation.

Change-Id: I0344e39222b3aecad5e30f820014f4d9f672e90c
This commit is contained in:
Mark McLoughlin 2013-05-23 11:08:10 +01:00
parent ea4a642587
commit 2e1c685b57
2 changed files with 125 additions and 11 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

38
oslo/config/cfg.py
View File

@ -588,7 +588,7 @@ class Opt(object):
names.append((dgroup if dgroup else section,
dname if dname else self.dest))
return cparser.get(names, multi)
return cparser.get(names, multi, normalized=True)
def _add_to_cli(self, parser, group=None):
"""Makes the option available in the command line interface.
@ -1069,10 +1069,11 @@ class ParseError(iniparser.ParseError):
class ConfigParser(iniparser.BaseParser):
def __init__(self, filename, sections):
def __init__(self, filename, sections, normalized=None):
super(ConfigParser, self).__init__()
self.filename = filename
self.sections = sections
self.normalized = normalized
self.section = None
def parse(self):
@ -1080,15 +1081,25 @@ class ConfigParser(iniparser.BaseParser):
return super(ConfigParser, self).parse(f)
def new_section(self, section):
self.section = _normalize_group_name(section)
self.section = section
self.sections.setdefault(self.section, {})
if self.normalized is not None:
self.normalized.setdefault(_normalize_group_name(self.section), {})
def assignment(self, key, value):
if not self.section:
raise self.error_no_section()
self.sections[self.section].setdefault(key, [])
self.sections[self.section][key].append('\n'.join(value))
value = '\n'.join(value)
def append(sections, section):
sections[section].setdefault(key, [])
sections[section][key].append(value)
append(self.sections, self.section)
if self.normalized is not None:
append(self.normalized, _normalize_group_name(self.section))
def parse_exc(self, msg, lineno, line=None):
return ParseError(msg, lineno, line, self.filename)
@ -1101,28 +1112,35 @@ class ConfigParser(iniparser.BaseParser):
class MultiConfigParser(object):
def __init__(self):
self.parsed = []
self._normalized = []
def read(self, config_files):
read_ok = []
for filename in config_files:
sections = {}
parser = ConfigParser(filename, sections)
normalized = {}
parser = ConfigParser(filename, sections, normalized)
try:
parser.parse()
except IOError:
continue
self.parsed.insert(0, sections)
self._normalized.insert(0, normalized)
read_ok.append(filename)
return read_ok
def get(self, names, multi=False):
def get(self, names, multi=False, normalized=False):
rvalue = []
names = [(_normalize_group_name(section), name)
for section, name in names]
for sections in self.parsed:
def normalize(name):
return _normalize_group_name(section) if normalized else name
names = [(normalize(section), name) for section, name in names]
for sections in (self._normalized if normalized else self.parsed):
for section, name in names:
if section not in sections:
continue

98
tests/test_cfg.py
View File

@ -2260,7 +2260,29 @@ class OptDumpingTestCase(BaseTestCase):
self._do_test_log_opt_values(None)
class ConfigParserTestCase(utils.BaseTestCase):
class ConfigParserTestCase(BaseTestCase):
def test_parse_file(self):
paths = self.create_tempfiles([('test',
'[DEFAULT]\n'
'foo = bar\n'
'[BLAA]\n'
'bar = foo\n')])
sections = {}
normalized = {}
parser = cfg.ConfigParser(paths[0], sections, normalized)
parser.parse()
self.assertTrue('DEFAULT' in sections)
self.assertTrue('DEFAULT' in normalized)
self.assertTrue('BLAA' in sections)
self.assertTrue('blaa' in normalized)
self.assertEquals(sections['DEFAULT']['foo'], ['bar'])
self.assertEquals(normalized['DEFAULT']['foo'], ['bar'])
self.assertEquals(sections['BLAA']['bar'], ['foo'])
self.assertEquals(normalized['blaa']['bar'], ['foo'])
def test_no_section(self):
with tempfile.NamedTemporaryFile() as tmpfile:
tmpfile.write('foo = bar')
@ -2270,6 +2292,80 @@ class ConfigParserTestCase(utils.BaseTestCase):
self.assertRaises(cfg.ParseError, parser.parse)
class MultiConfigParserTestCase(BaseTestCase):
def test_parse_single_file(self):
paths = self.create_tempfiles([('test',
'[DEFAULT]\n'
'foo = bar\n'
'[BLAA]\n'
'bar = foo\n')])
parser = cfg.MultiConfigParser()
read_ok = parser.read(paths)
self.assertEquals(read_ok, paths)
self.assertTrue('DEFAULT' in parser.parsed[0])
self.assertEquals(parser.parsed[0]['DEFAULT']['foo'], ['bar'])
self.assertEquals(parser.get([('DEFAULT', 'foo')]), ['bar'])
self.assertEquals(parser.get([('DEFAULT', 'foo')], multi=True),
['bar'])
self.assertEquals(parser.get([('DEFAULT', 'foo')],
multi=True, normalized=True),
['bar'])
self.assertTrue('BLAA' in parser.parsed[0])
self.assertEquals(parser.parsed[0]['BLAA']['bar'], ['foo'])
self.assertEquals(parser.get([('BLAA', 'bar')]), ['foo'])
self.assertEquals(parser.get([('BLAA', 'bar')], multi=True),
['foo'])
self.assertEquals(parser.get([('blaa', 'bar')],
multi=True, normalized=True),
['foo'])
def test_parse_multiple_files(self):
paths = self.create_tempfiles([('test1',
'[DEFAULT]\n'
'foo = bar\n'
'[BLAA]\n'
'bar = foo'),
('test2',
'[DEFAULT]\n'
'foo = barbar\n'
'[BLAA]\n'
'bar = foofoo\n'
'[bLAa]\n'
'bar = foofoofoo\n')])
parser = cfg.MultiConfigParser()
read_ok = parser.read(paths)
self.assertEquals(read_ok, paths)
self.assertTrue('DEFAULT' in parser.parsed[0])
self.assertEquals(parser.parsed[0]['DEFAULT']['foo'], ['barbar'])
self.assertTrue('DEFAULT' in parser.parsed[1])
self.assertEquals(parser.parsed[1]['DEFAULT']['foo'], ['bar'])
self.assertEquals(parser.get([('DEFAULT', 'foo')]), ['barbar'])
self.assertEquals(parser.get([('DEFAULT', 'foo')], multi=True),
['bar', 'barbar'])
self.assertTrue('BLAA' in parser.parsed[0])
self.assertTrue('bLAa' in parser.parsed[0])
self.assertEquals(parser.parsed[0]['BLAA']['bar'], ['foofoo'])
self.assertEquals(parser.parsed[0]['bLAa']['bar'], ['foofoofoo'])
self.assertTrue('BLAA' in parser.parsed[1])
self.assertEquals(parser.parsed[1]['BLAA']['bar'], ['foo'])
self.assertEquals(parser.get([('BLAA', 'bar')]), ['foofoo'])
self.assertEquals(parser.get([('bLAa', 'bar')]), ['foofoofoo'])
self.assertEquals(parser.get([('BLAA', 'bar')], multi=True),
['foo', 'foofoo'])
self.assertEquals(parser.get([('BLAA', 'bar')],
multi=True, normalized=True),
['foo', 'foofoo', 'foofoofoo'])
class TildeExpansionTestCase(BaseTestCase):
def test_config_file_tilde(self):