openstack/reno
Code Issues Proposed changes

Refactor Section config to use NamedTuple

Browse Source 
Users configure the sections as a List[List[str]]. That continues
to be the case.

But now we parse that raw list into a list of a NamedTuple
called Section. This makes our consuming code, `formatter.py`
and `linter.py` more clear because we can access the values
by name, rather than index.

But more importantly, this is a "prefactor" to add support
for nested headers. In a followup, we will add the field
'top_level: bool' so that `formatter.py` can know how to
render the section. This factoring simplifies that
change.

Change-Id: Ie80a525af61e879dd872079b2b9d0513db40e82d
This commit is contained in:
Eric Arellano 2023-02-07 13:49:35 -07:00
parent 04233e0eae
commit 919210c386
5 changed files with 60 additions and 53 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

33
reno/config.py
View File

@ -10,10 +10,12 @@
# License for the specific language governing permissions and limitations
# under the License.
import collections
import logging
import os.path
import textwrap
from typing import Any
from typing import List
from typing import NamedTuple
import yaml
@ -22,7 +24,20 @@ from reno import defaults
LOG = logging.getLogger(__name__)
Opt = collections.namedtuple('Opt', 'name default help')
class Opt(NamedTuple):
name: str
default: Any
help: str
class Section(NamedTuple):
name: str
title: str
@classmethod
def from_raw_yaml(cls, val: List[List[str]]) -> List["Section"]:
return [Section(*entry) for entry in val]
_OPTIONS = [
Opt('notesdir', defaults.NOTES_SUBDIR,
@ -219,13 +234,13 @@ _OPTIONS = [
]
class Config(object):
class Config:
_OPTS = {o.name: o for o in _OPTIONS}
@classmethod
def get_default(cls, opt):
"Return the default for an option."
"""Return the default for an option."""
try:
return cls._OPTS[opt].default
except KeyError:
@ -301,12 +316,14 @@ class Config(object):
# Replace prelude section name if it has been changed.
self._rename_prelude_section(**kwds)
for n, v in kwds.items():
if n not in self._OPTS:
for name, val in kwds.items():
if name not in self._OPTS:
LOG.warning('ignoring unknown configuration value %r = %r',
n, v)
name, val)
else:
setattr(self, n, v)
if name == "sections":
val = Section.from_raw_yaml(val)
setattr(self, name, val)
def override_from_parsed_args(self, parsed_args):
"""Set the values of the configuration options from parsed CLI args.

12
reno/formatter.py
View File

@ -94,19 +94,19 @@ def format_report(loader, config, versions_to_include, title=None,
report.append('')
# Add other sections.
for section_name, section_title in config.sections:
for section in config.sections:
notes = [
(n, fn, sha)
for fn, sha in notefiles
if file_contents[fn].get(section_name)
for n in file_contents[fn].get(section_name, [])
if file_contents[fn].get(section.name)
for n in file_contents[fn].get(section.name, [])
]
if notes:
report.append(_section_anchor(
section_title, version_title, title, branch))
section.title, version_title, title, branch))
report.append('')
report.append(section_title)
report.append('-' * len(section_title))
report.append(section.title)
report.append('-' * len(section.title))
report.append('')
for n, fn, sha in notes:
if show_source:

4
reno/linter.py
View File

@ -21,14 +21,14 @@ LOG = logging.getLogger(__name__)
def lint_cmd(args, conf):
"Check some common mistakes"
"""Check some common mistakes"""
LOG.debug('starting lint')
notesdir = os.path.join(conf.reporoot, conf.notespath)
notes = glob.glob(os.path.join(notesdir, '*.yaml'))
error = 0
allowed_section_names = [conf.prelude_section_name] + \
[s[0] for s in conf.sections]
[s.name for s in conf.sections]
uids = {}
with loader.Loader(conf, ignore_cache=True) as ldr:

7
reno/loader.py
View File

@ -143,6 +143,9 @@ class Loader(object):
f'mapping. Did you forget a top-level key?'
)
valid_section_names = {
section.name for section in self._config.sections
}
for section_name, section_content in content.items():
if section_name == self._config.prelude_section_name:
if not isinstance(section_content, str):
@ -152,14 +155,14 @@ class Loader(object):
section_name, filename,
)
else:
if section_name not in dict(self._config.sections):
if section_name not in valid_section_names:
# TODO(stephenfin): Make this an error in a future release
LOG.warning(
'The %s section of %s is not a recognized section. '
'It should be one of: %s. '
'This will be an error in a future release.',
section_name, filename,
', '.join(dict(self._config.sections)),
', '.join(valid_section_names),
)
if isinstance(section_content, str):
# A single string is OK, but wrap it with a list

57
reno/tests/test_config.py
View File

@ -18,11 +18,26 @@ from unittest import mock
import fixtures
from reno import config
from reno.config import Section
from reno import defaults
from reno import main
from reno.tests import base
def expected_options(**overrides):
"""The default config options, along with any overrides set via kwargs."""
result = {
o.name: (
Section.from_raw_yaml(o.default)
if o.name == "sections"
else o.default
)
for o in config._OPTIONS
}
result.update(**overrides)
return result
class TestConfig(base.TestCase):
EXAMPLE_CONFIG = """
collapse_pre_releases: false
@ -36,11 +51,7 @@ collapse_pre_releases: false
def test_defaults(self):
c = config.Config(self.tempdir.path)
actual = c.options
expected = {
o.name: o.default
for o in config._OPTIONS
}
self.assertEqual(expected, actual)
self.assertEqual(expected_options(), actual)
def test_override(self):
c = config.Config(self.tempdir.path)
@ -48,11 +59,7 @@ collapse_pre_releases: false
collapse_pre_releases=False,
)
actual = c.options
expected = {
o.name: o.default
for o in config._OPTIONS
}
expected['collapse_pre_releases'] = False
expected = expected_options(collapse_pre_releases=False)
self.assertEqual(expected, actual)
def test_override_multiple(self):
@ -64,11 +71,7 @@ collapse_pre_releases: false
notesdir='value2',
)
actual = c.options
expected = {
o.name: o.default
for o in config._OPTIONS
}
expected['notesdir'] = 'value2'
expected = expected_options(notesdir='value2')
self.assertEqual(expected, actual)
def test_load_file_not_present(self):
@ -127,22 +130,14 @@ collapse_pre_releases: false
o.name: getattr(c, o.name)
for o in config._OPTIONS
}
expected = {
o.name: o.default
for o in config._OPTIONS
}
self.assertEqual(expected, actual)
self.assertEqual(expected_options(), actual)
def test_override_from_parsed_args_boolean_false(self):
c = self._run_override_from_parsed_args([
'--no-collapse-pre-releases',
])
actual = c.options
expected = {
o.name: o.default
for o in config._OPTIONS
}
expected['collapse_pre_releases'] = False
expected = expected_options(collapse_pre_releases=False)
self.assertEqual(expected, actual)
def test_override_from_parsed_args_boolean_true(self):
@ -150,11 +145,7 @@ collapse_pre_releases: false
'--collapse-pre-releases',
])
actual = c.options
expected = {
o.name: o.default
for o in config._OPTIONS
}
expected['collapse_pre_releases'] = True
expected = expected_options(collapse_pre_releases=True)
self.assertEqual(expected, actual)
def test_override_from_parsed_args_string(self):
@ -162,11 +153,7 @@ collapse_pre_releases: false
'--earliest-version', '1.2.3',
])
actual = c.options
expected = {
o.name: o.default
for o in config._OPTIONS
}
expected['earliest_version'] = '1.2.3'
expected = expected_options(earliest_version='1.2.3')
self.assertEqual(expected, actual)
def test_override_from_parsed_args_ignore_non_options(self):