diff --git a/CHANGELOG.md b/CHANGELOG.md index 1e644bf..665db90 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,8 @@ +## v1.?.? - 2024-??-?? - ??? + +* Add YamlProvider.order_mode to allow picking between natural (human) + the default when enforce_order=True and simple `sort`. + ## v1.10.0 - 2024-10-06 - Lots of little stuff * Zone name validation checking for double dots, and throwing InvalidNameError diff --git a/octodns/cmds/args.py b/octodns/cmds/args.py index 1aefbea..7e0ae7d 100644 --- a/octodns/cmds/args.py +++ b/octodns/cmds/args.py @@ -8,9 +8,8 @@ from logging.config import dictConfig from logging.handlers import SysLogHandler from sys import stderr, stdout -from yaml import safe_load - from octodns import __version__ +from octodns.yaml import safe_load class ArgumentParser(_Base): @@ -68,7 +67,7 @@ class ArgumentParser(_Base): def _setup_logging(self, args, default_log_level): if args.logging_config: with open(args.logging_config) as fh: - config = safe_load(fh.read()) + config = safe_load(fh.read(), enforce_order=False) dictConfig(config) # if we're provided a logging_config we won't do any of our normal # configuration diff --git a/octodns/provider/yaml.py b/octodns/provider/yaml.py index 3fc6102..b5fd3d7 100644 --- a/octodns/provider/yaml.py +++ b/octodns/provider/yaml.py @@ -34,6 +34,11 @@ class YamlProvider(BaseProvider): # Whether or not to enforce sorting order when loading yaml # (optional, default True) enforce_order: true + # What sort mode to employ when enforcing order + # - simple: `sort` + # - natural: https://pypi.org/project/natsort/ + # (optional, default natural) + order_mode: natural # Whether duplicate records should replace rather than error # (optional, default False) @@ -174,6 +179,7 @@ class YamlProvider(BaseProvider): directory, default_ttl=3600, enforce_order=True, + order_mode='natural', populate_should_replace=False, supports_root_ns=True, split_extension=False, @@ -186,11 +192,12 @@ class YamlProvider(BaseProvider): klass = self.__class__.__name__ self.log = logging.getLogger(f'{klass}[{id}]') self.log.debug( - '__init__: id=%s, directory=%s, default_ttl=%d, enforce_order=%d, populate_should_replace=%s, supports_root_ns=%s, split_extension=%s, split_catchall=%s, shared_filename=%s, disable_zonefile=%s', + '__init__: id=%s, directory=%s, default_ttl=%d, enforce_order=%d, order_mode=%s, populate_should_replace=%s, supports_root_ns=%s, split_extension=%s, split_catchall=%s, shared_filename=%s, disable_zonefile=%s', id, directory, default_ttl, enforce_order, + order_mode, populate_should_replace, supports_root_ns, split_extension, @@ -202,6 +209,7 @@ class YamlProvider(BaseProvider): self.directory = directory self.default_ttl = default_ttl self.enforce_order = enforce_order + self.order_mode = order_mode self.populate_should_replace = populate_should_replace self.supports_root_ns = supports_root_ns self.split_extension = split_extension @@ -304,7 +312,9 @@ class YamlProvider(BaseProvider): def _populate_from_file(self, filename, zone, lenient): with open(filename, 'r') as fh: - yaml_data = safe_load(fh, enforce_order=self.enforce_order) + yaml_data = safe_load( + fh, enforce_order=self.enforce_order, order_mode=self.order_mode + ) if yaml_data: for name, data in yaml_data.items(): if not isinstance(data, list): @@ -420,7 +430,7 @@ class YamlProvider(BaseProvider): with open(filename, 'w') as fh: record_data = {record: config} - safe_dump(record_data, fh) + safe_dump(record_data, fh, order_mode=self.order_mode) if catchall: # Scrub the trailing . to make filenames more sane. @@ -429,14 +439,19 @@ class YamlProvider(BaseProvider): '_apply: writing catchall filename=%s', filename ) with open(filename, 'w') as fh: - safe_dump(catchall, fh) + safe_dump(catchall, fh, order_mode=self.order_mode) else: # single large file filename = join(self.directory, f'{desired.decoded_name}yaml') self.log.debug('_apply: writing filename=%s', filename) with open(filename, 'w') as fh: - safe_dump(dict(data), fh, allow_unicode=True) + safe_dump( + dict(data), + fh, + allow_unicode=True, + order_mode=self.order_mode, + ) class SplitYamlProvider(YamlProvider): diff --git a/octodns/yaml.py b/octodns/yaml.py index 433486a..71e7d05 100644 --- a/octodns/yaml.py +++ b/octodns/yaml.py @@ -11,7 +11,9 @@ from yaml.representer import SafeRepresenter from .context import ContextDict -_natsort_key = natsort_keygen() +# as of python 3.13 functools.partial is a method descriptor and must be wrapped +# in staticmethod() to preserve the behavior natsort is expecting it to have +_natsort_key = staticmethod(natsort_keygen()) class ContextLoader(SafeLoader): @@ -44,11 +46,12 @@ ContextLoader.add_constructor( # Found http://stackoverflow.com/a/21912744 which guided me on how to hook in # here class SortEnforcingLoader(ContextLoader): + def _construct(self, node): ret, pairs, context = self._pairs(node) keys = [d[0] for d in pairs] - keys_sorted = sorted(keys, key=_natsort_key) + keys_sorted = sorted(keys, key=self.KEYGEN) for key in keys: expected = keys_sorted.pop(0) if key != expected: @@ -62,13 +65,51 @@ class SortEnforcingLoader(ContextLoader): return ret -SortEnforcingLoader.add_constructor( - SortEnforcingLoader.DEFAULT_MAPPING_TAG, SortEnforcingLoader._construct +class NaturalSortEnforcingLoader(SortEnforcingLoader): + KEYGEN = _natsort_key + + +NaturalSortEnforcingLoader.add_constructor( + SortEnforcingLoader.DEFAULT_MAPPING_TAG, + NaturalSortEnforcingLoader._construct, +) + + +class SimpleSortEnforcingLoader(SortEnforcingLoader): + KEYGEN = lambda _, s: s + + +SimpleSortEnforcingLoader.add_constructor( + SortEnforcingLoader.DEFAULT_MAPPING_TAG, + SimpleSortEnforcingLoader._construct, ) -def safe_load(stream, enforce_order=True): - return load(stream, SortEnforcingLoader if enforce_order else ContextLoader) +_loaders = { + 'natural': NaturalSortEnforcingLoader, + 'simple': SimpleSortEnforcingLoader, +} + + +class InvalidOrder(Exception): + + def __init__(self, order_mode): + options = '", "'.join(_loaders.keys()) + super().__init__( + f'Invalid order_mode, "{order_mode}", options are "{options}"' + ) + + +def safe_load(stream, enforce_order=True, order_mode='natural'): + if enforce_order: + try: + loader = _loaders[order_mode] + except KeyError as e: + raise InvalidOrder(order_mode) from e + else: + loader = ContextLoader + + return load(stream, loader) class SortingDumper(SafeDumper): @@ -81,7 +122,7 @@ class SortingDumper(SafeDumper): ''' def _representer(self, data): - data = sorted(data.items(), key=lambda d: _natsort_key(d[0])) + data = sorted(data.items(), key=self.KEYGEN) return self.represent_mapping(self.DEFAULT_MAPPING_TAG, data) @@ -92,7 +133,18 @@ SortingDumper.add_multi_representer(str, SafeRepresenter.represent_str) SortingDumper.add_multi_representer(dict, SortingDumper._representer) -def safe_dump(data, fh, **options): +class NaturalSortingDumper(SortingDumper): + KEYGEN = _natsort_key + + +class SimpleSortingDumper(SortingDumper): + KEYGEN = lambda _, s: s + + +_dumpers = {'natural': NaturalSortingDumper, 'simple': SimpleSortingDumper} + + +def safe_dump(data, fh, order_mode='natural', **options): kwargs = { 'canonical': False, 'indent': 2, @@ -101,4 +153,8 @@ def safe_dump(data, fh, **options): 'explicit_start': True, } kwargs.update(options) - dump(data, fh, SortingDumper, **kwargs) + try: + dumper = _dumpers[order_mode] + except KeyError as e: + raise InvalidOrder(order_mode) from e + dump(data, fh, dumper, **kwargs) diff --git a/tests/test_octodns_yaml.py b/tests/test_octodns_yaml.py index 396d59c..a05d28d 100644 --- a/tests/test_octodns_yaml.py +++ b/tests/test_octodns_yaml.py @@ -7,7 +7,7 @@ from unittest import TestCase from yaml.constructor import ConstructorError -from octodns.yaml import safe_dump, safe_load +from octodns.yaml import InvalidOrder, safe_dump, safe_load class TestYaml(TestCase): @@ -86,3 +86,60 @@ class TestYaml(TestCase): "[Errno 2] No such file or directory: 'tests/config/include/does-not-exist.yaml'", str(ctx.exception), ) + + def test_order_mode(self): + data = {'*.1.2': 'a', '*.10.1': 'c', '*.11.2': 'd', '*.2.2': 'b'} + natural = '''--- +'*.1.2': a +'*.2.2': b +'*.10.1': c +'*.11.2': d +''' + simple = '''--- +'*.1.2': a +'*.10.1': c +'*.11.2': d +'*.2.2': b +''' + + ## natural + # correct order + self.assertEqual(data, safe_load(natural)) + # wrong order + with self.assertRaises(ConstructorError) as ctx: + safe_load(simple) + problem = ctx.exception.problem.split(' at')[0] + self.assertEqual( + 'keys out of order: expected *.2.2 got *.10.1', problem + ) + # dump + buf = StringIO() + safe_dump(data, buf) + self.assertEqual(natural, buf.getvalue()) + + ## simple + # correct order + self.assertEqual(data, safe_load(simple, order_mode='simple')) + # wrong order + with self.assertRaises(ConstructorError) as ctx: + safe_load(natural, order_mode='simple') + problem = ctx.exception.problem.split(' at')[0] + self.assertEqual( + 'keys out of order: expected *.10.1 got *.2.2', problem + ) + buf = StringIO() + safe_dump(data, buf, order_mode='simple') + self.assertEqual(simple, buf.getvalue()) + + with self.assertRaises(InvalidOrder) as ctx: + safe_load(None, order_mode='bad') + self.assertEqual( + 'Invalid order_mode, "bad", options are "natural", "simple"', + str(ctx.exception), + ) + with self.assertRaises(InvalidOrder) as ctx: + safe_dump(None, None, order_mode='bad2') + self.assertEqual( + 'Invalid order_mode, "bad2", options are "natural", "simple"', + str(ctx.exception), + )