Merge pull request #1207 from octodns/order-mode

Add YamlProvider.order_mode natural is default, adds simple

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

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

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):

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)

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),
+        )