From db35ffe72e1ad0e28b3779f53d2da45d561a1d6c Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Mon, 19 Jun 2017 22:17:48 -0700 Subject: [PATCH 1/3] Replace my custom natrual sorting with natsort module Better to use something real/tested and less likely buggy/limited. --- octodns/yaml.py | 21 ++++----------------- requirements.txt | 1 + setup.py | 1 + 3 files changed, 6 insertions(+), 17 deletions(-) diff --git a/octodns/yaml.py b/octodns/yaml.py index 2cab58c..d4ab541 100644 --- a/octodns/yaml.py +++ b/octodns/yaml.py @@ -5,25 +5,12 @@ from __future__ import absolute_import, division, print_function, \ unicode_literals +from natsort import natsort_keygen from yaml import SafeDumper, SafeLoader, load, dump from yaml.constructor import ConstructorError -import re -# zero-padded sort, simplified version of -# https://www.xormedia.com/natural-sort-order-with-zero-padding/ -_pad_re = re.compile('\d+') - - -def _zero_pad(match): - return '{:04d}'.format(int(match.group(0))) - - -def _zero_padded_numbers(s): - try: - int(s) - except ValueError: - return _pad_re.sub(lambda d: _zero_pad(d), s) +_natsort_key = natsort_keygen() # Found http://stackoverflow.com/a/21912744 which guided me on how to hook in @@ -34,7 +21,7 @@ class SortEnforcingLoader(SafeLoader): self.flatten_mapping(node) ret = self.construct_pairs(node) keys = [d[0] for d in ret] - if keys != sorted(keys, key=_zero_padded_numbers): + if keys != sorted(keys, key=_natsort_key): raise ConstructorError(None, None, "keys out of order: {}" .format(', '.join(keys)), node.start_mark) return dict(ret) @@ -59,7 +46,7 @@ class SortingDumper(SafeDumper): def _representer(self, data): data = data.items() - data.sort(key=lambda d: _zero_padded_numbers(d[0])) + data.sort(key=lambda d: _natsort_key(d[0])) return self.represent_mapping(self.DEFAULT_MAPPING_TAG, data) diff --git a/requirements.txt b/requirements.txt index efd7577..b10ca4c 100644 --- a/requirements.txt +++ b/requirements.txt @@ -10,6 +10,7 @@ futures==3.0.5 incf.countryutils==1.0 ipaddress==1.0.18 jmespath==0.9.0 +natsort==5.0.3 nsone==0.9.10 python-dateutil==2.6.0 requests==2.13.0 diff --git a/setup.py b/setup.py index ebb4092..f2b901d 100644 --- a/setup.py +++ b/setup.py @@ -34,6 +34,7 @@ setup( 'futures>=3.0.5', 'incf.countryutils>=1.0', 'ipaddress>=1.0.18', + 'natsort>=5.0.3', 'python-dateutil>=2.6.0', 'requests>=2.13.0' ], From 4a7ce9e833cbb6344837491fdc98cd1752e7baf7 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Mon, 19 Jun 2017 22:36:08 -0700 Subject: [PATCH 2/3] Bake in the existing, but less than great hex sorting behavior --- tests/test_octodns_yaml.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/tests/test_octodns_yaml.py b/tests/test_octodns_yaml.py index 9c3cec5..0f454b3 100644 --- a/tests/test_octodns_yaml.py +++ b/tests/test_octodns_yaml.py @@ -59,3 +59,12 @@ class TestYaml(TestCase): }, buf) self.assertEquals("---\n'*.1.1': 42\n'*.2.1': 44\n'*.11.1': 43\n", buf.getvalue()) + + # hex sorting isn't ideal, not treated as hex, this make sure we don't + # change the behavior + buf = StringIO() + safe_dump({ + '45a03129': 42, + '45a0392a': 43, + }, buf) + self.assertEquals("---\n45a0392a: 43\n45a03129: 42\n", buf.getvalue()) From 046cde43b20871072ef57528b33654d44ee5dbe6 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Mon, 19 Jun 2017 22:44:34 -0700 Subject: [PATCH 3/3] Make sorting enforcement optional with YamlProvider --- octodns/provider/yaml.py | 14 ++++++++++---- tests/test_octodns_provider_yaml.py | 6 ++++++ 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/octodns/provider/yaml.py b/octodns/provider/yaml.py index 1089f02..7b2d209 100644 --- a/octodns/provider/yaml.py +++ b/octodns/provider/yaml.py @@ -26,16 +26,22 @@ class YamlProvider(BaseProvider): # The ttl to use for records when not specified in the data # (optional, default 3600) default_ttl: 3600 + # Whether or not to enforce sorting order on the yaml config + # (optional, default True) + enforce_order: True ''' SUPPORTS_GEO = True - def __init__(self, id, directory, default_ttl=3600, *args, **kwargs): + def __init__(self, id, directory, default_ttl=3600, enforce_order=True, + *args, **kwargs): self.log = logging.getLogger('YamlProvider[{}]'.format(id)) - self.log.debug('__init__: id=%s, directory=%s, default_ttl=%d', id, - directory, default_ttl) + self.log.debug('__init__: id=%s, directory=%s, default_ttl=%d, ' + 'enforce_order=%d', id, directory, default_ttl, + enforce_order) super(YamlProvider, self).__init__(id, *args, **kwargs) self.directory = directory self.default_ttl = default_ttl + self.enforce_order = enforce_order def populate(self, zone, target=False): self.log.debug('populate: zone=%s, target=%s', zone.name, target) @@ -47,7 +53,7 @@ class YamlProvider(BaseProvider): before = len(zone.records) filename = join(self.directory, '{}yaml'.format(zone.name)) with open(filename, 'r') as fh: - yaml_data = safe_load(fh) + yaml_data = safe_load(fh, enforce_order=self.enforce_order) if yaml_data: for name, data in yaml_data.items(): if not isinstance(data, list): diff --git a/tests/test_octodns_provider_yaml.py b/tests/test_octodns_provider_yaml.py index 05c5248..9438f01 100644 --- a/tests/test_octodns_provider_yaml.py +++ b/tests/test_octodns_provider_yaml.py @@ -100,6 +100,12 @@ class TestYamlProvider(TestCase): with self.assertRaises(ConstructorError): source.populate(zone) + source = YamlProvider('test', join(dirname(__file__), 'config'), + enforce_order=False) + # no exception + source.populate(zone) + self.assertEqual(2, len(zone.records)) + def test_subzone_handling(self): source = YamlProvider('test', join(dirname(__file__), 'config'))