From 158add8eb6e7fefe40e8a0865f786838b631d0d2 Mon Sep 17 00:00:00 2001 From: cclauss Date: Fri, 12 Jul 2019 01:28:10 +0200 Subject: [PATCH 01/49] Modernize Python 2 code to prepare for Python 3 --- .travis.yml | 10 ++++++++-- octodns/cmds/report.py | 8 +++++--- octodns/manager.py | 8 ++++---- octodns/provider/base.py | 4 +++- octodns/provider/cloudflare.py | 2 +- octodns/provider/ns1.py | 6 ++++-- octodns/provider/plan.py | 18 ++++++++++-------- octodns/provider/route53.py | 13 ++++++++++--- octodns/record/__init__.py | 18 +++++++++++++----- octodns/zone.py | 4 +++- requirements.txt | 4 ++-- tests/test_octodns_provider_base.py | 16 +++++++++------- tests/test_octodns_provider_cloudflare.py | 6 +++--- tests/test_octodns_provider_googlecloud.py | 2 +- 14 files changed, 76 insertions(+), 43 deletions(-) diff --git a/.travis.yml b/.travis.yml index b17ca01..b1a8204 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,6 +1,12 @@ language: python -python: - - 2.7 +matrix: + include: + - python: 2.7 + - python: 3.7 + dist: xenial # required for Python >= 3.7 on Travis CI + allow_failures: + - python: 3.7 +before_install: pip install --upgrade pip script: ./script/cibuild notifications: email: diff --git a/octodns/cmds/report.py b/octodns/cmds/report.py index 2b32e77..3a26052 100755 --- a/octodns/cmds/report.py +++ b/octodns/cmds/report.py @@ -13,6 +13,8 @@ from logging import getLogger from sys import stdout import re +from six import text_type + from octodns.cmds.args import ArgumentParser from octodns.manager import Manager from octodns.zone import Zone @@ -65,7 +67,7 @@ def main(): resolver = AsyncResolver(configure=False, num_workers=int(args.num_workers)) if not ip_addr_re.match(server): - server = unicode(query(server, 'A')[0]) + server = text_type(query(server, 'A')[0]) log.info('server=%s', server) resolver.nameservers = [server] resolver.lifetime = int(args.timeout) @@ -81,12 +83,12 @@ def main(): stdout.write(',') stdout.write(record._type) stdout.write(',') - stdout.write(unicode(record.ttl)) + stdout.write(text_type(record.ttl)) compare = {} for future in futures: stdout.write(',') try: - answers = [unicode(r) for r in future.result()] + answers = [text_type(r) for r in future.result()] except (NoAnswer, NoNameservers): answers = ['*no answer*'] except NXDOMAIN: diff --git a/octodns/manager.py b/octodns/manager.py index 4952315..89db845 100644 --- a/octodns/manager.py +++ b/octodns/manager.py @@ -276,9 +276,9 @@ class Manager(object): try: sources = [self.providers[source] for source in sources] - except KeyError: + except KeyError as e: raise Exception('Zone {}, unknown source: {}'.format(zone_name, - source)) + e)) try: trgs = [] @@ -397,9 +397,9 @@ class Manager(object): try: sources = [self.providers[source] for source in sources] - except KeyError: + except KeyError as e: raise Exception('Zone {}, unknown source: {}'.format(zone_name, - source)) + e)) for source in sources: if isinstance(source, YamlProvider): diff --git a/octodns/provider/base.py b/octodns/provider/base.py index 2c93e49..96d1be1 100644 --- a/octodns/provider/base.py +++ b/octodns/provider/base.py @@ -5,6 +5,8 @@ from __future__ import absolute_import, division, print_function, \ unicode_literals +from six import text_type + from ..source.base import BaseSource from ..zone import Zone from .plan import Plan @@ -68,7 +70,7 @@ class BaseProvider(BaseSource): changes=changes) if extra: self.log.info('plan: extra changes\n %s', '\n ' - .join([unicode(c) for c in extra])) + .join([text_type(c) for c in extra])) changes += extra if changes: diff --git a/octodns/provider/cloudflare.py b/octodns/provider/cloudflare.py index 881c2fd..ac1fe9f 100644 --- a/octodns/provider/cloudflare.py +++ b/octodns/provider/cloudflare.py @@ -442,7 +442,7 @@ class CloudflareProvider(BaseProvider): # Round trip the single value through a record to contents flow # to get a consistent _gen_data result that matches what # went in to new_contents - data = self._gen_data(r).next() + data = next(self._gen_data(r)) # Record the record_id and data for this existing record key = self._gen_key(data) diff --git a/octodns/provider/ns1.py b/octodns/provider/ns1.py index 5fdf5b0..d3faf21 100644 --- a/octodns/provider/ns1.py +++ b/octodns/provider/ns1.py @@ -13,6 +13,8 @@ from nsone.rest.errors import RateLimitException, ResourceException from incf.countryutils import transformations from time import sleep +from six import text_type + from ..record import Record from .base import BaseProvider @@ -76,9 +78,9 @@ class Ns1Provider(BaseProvider): else: values.extend(answer['answer']) codes.append([]) - values = [unicode(x) for x in values] + values = [text_type(x) for x in values] geo = OrderedDict( - {unicode(k): [unicode(x) for x in v] for k, v in geo.items()} + {text_type(k): [text_type(x) for x in v] for k, v in geo.items()} ) data['values'] = values data['geo'] = geo diff --git a/octodns/provider/plan.py b/octodns/provider/plan.py index bae244f..ad14b6d 100644 --- a/octodns/provider/plan.py +++ b/octodns/provider/plan.py @@ -9,6 +9,8 @@ from StringIO import StringIO from logging import DEBUG, ERROR, INFO, WARN, getLogger from sys import stdout +from six import text_type + class UnsafePlan(Exception): pass @@ -147,11 +149,11 @@ class PlanLogger(_PlanOutput): def _value_stringifier(record, sep): try: - values = [unicode(v) for v in record.values] + values = [text_type(v) for v in record.values] except AttributeError: values = [record.value] for code, gv in sorted(getattr(record, 'geo', {}).items()): - vs = ', '.join([unicode(v) for v in gv.values]) + vs = ', '.join([text_type(v) for v in gv.values]) values.append('{}: {}'.format(code, vs)) return sep.join(values) @@ -193,7 +195,7 @@ class PlanMarkdown(_PlanOutput): fh.write(' | ') # TTL if existing: - fh.write(unicode(existing.ttl)) + fh.write(text_type(existing.ttl)) fh.write(' | ') fh.write(_value_stringifier(existing, '; ')) fh.write(' | |\n') @@ -201,7 +203,7 @@ class PlanMarkdown(_PlanOutput): fh.write('| | | | ') if new: - fh.write(unicode(new.ttl)) + fh.write(text_type(new.ttl)) fh.write(' | ') fh.write(_value_stringifier(new, '; ')) fh.write(' | ') @@ -210,7 +212,7 @@ class PlanMarkdown(_PlanOutput): fh.write(' |\n') fh.write('\nSummary: ') - fh.write(unicode(plan)) + fh.write(text_type(plan)) fh.write('\n\n') else: fh.write('## No changes were planned\n') @@ -261,7 +263,7 @@ class PlanHtml(_PlanOutput): # TTL if existing: fh.write(' ') - fh.write(unicode(existing.ttl)) + fh.write(text_type(existing.ttl)) fh.write('\n ') fh.write(_value_stringifier(existing, '
')) fh.write('\n \n \n') @@ -270,7 +272,7 @@ class PlanHtml(_PlanOutput): if new: fh.write(' ') - fh.write(unicode(new.ttl)) + fh.write(text_type(new.ttl)) fh.write('\n ') fh.write(_value_stringifier(new, '
')) fh.write('\n ') @@ -279,7 +281,7 @@ class PlanHtml(_PlanOutput): fh.write('\n \n') fh.write(' \n Summary: ') - fh.write(unicode(plan)) + fh.write(text_type(plan)) fh.write('\n \n\n') else: fh.write('No changes were planned') diff --git a/octodns/provider/route53.py b/octodns/provider/route53.py index 4cf7c99..7154096 100644 --- a/octodns/provider/route53.py +++ b/octodns/provider/route53.py @@ -14,10 +14,17 @@ from uuid import uuid4 import logging import re +from six import text_type + from ..record import Record, Update from ..record.geo import GeoCodes from .base import BaseProvider +try: + cmp +except NameError: + def cmp(x, y): + return (x > y) - (x < y) octal_re = re.compile(r'\\(\d\d\d)') @@ -1037,8 +1044,8 @@ class Route53Provider(BaseProvider): # ip_address's returned object for equivalence # E.g 2001:4860:4860::8842 -> 2001:4860:4860:0:0:0:0:8842 if value: - value = ip_address(unicode(value)) - config_ip_address = ip_address(unicode(config['IPAddress'])) + value = ip_address(text_type(value)) + config_ip_address = ip_address(text_type(config['IPAddress'])) else: # No value so give this a None to match value's config_ip_address = None @@ -1059,7 +1066,7 @@ class Route53Provider(BaseProvider): fqdn, record._type, value) try: - ip_address(unicode(value)) + ip_address(text_type(value)) # We're working with an IP, host is the Host header healthcheck_host = record.healthcheck_host except (AddressValueError, ValueError): diff --git a/octodns/record/__init__.py b/octodns/record/__init__.py index dca6100..2efdf0e 100644 --- a/octodns/record/__init__.py +++ b/octodns/record/__init__.py @@ -9,8 +9,16 @@ from ipaddress import IPv4Address, IPv6Address from logging import getLogger import re +from six import string_types, text_type + from .geo import GeoCodes +try: + cmp +except NameError: + def cmp(x, y): + return (x > y) - (x < y) + class Change(object): @@ -130,7 +138,7 @@ class Record(object): self.__class__.__name__, name) self.zone = zone # force everything lower-case just to be safe - self.name = unicode(name).lower() if name else name + self.name = text_type(name).lower() if name else name self.source = source self.ttl = int(data['ttl']) @@ -292,7 +300,7 @@ class _ValuesMixin(object): return ret def __repr__(self): - values = "['{}']".format("', '".join([unicode(v) + values = "['{}']".format("', '".join([text_type(v) for v in self.values])) return '<{} {} {}, {}, {}>'.format(self.__class__.__name__, self._type, self.ttl, @@ -574,7 +582,7 @@ class _DynamicMixin(object): reasons.append('rule {} missing pool'.format(rule_num)) continue - if not isinstance(pool, basestring): + if not isinstance(pool, string_types): reasons.append('rule {} invalid pool "{}"' .format(rule_num, pool)) elif pool not in pools: @@ -671,13 +679,13 @@ class _IpList(object): return ['missing value(s)'] reasons = [] for value in data: - if value is '': + if value == '': reasons.append('empty value') elif value is None: reasons.append('missing value(s)') else: try: - cls._address_type(unicode(value)) + cls._address_type(text_type(value)) except Exception: reasons.append('invalid {} address "{}"' .format(cls._address_name, value)) diff --git a/octodns/zone.py b/octodns/zone.py index 916f81b..1191b6f 100644 --- a/octodns/zone.py +++ b/octodns/zone.py @@ -9,6 +9,8 @@ from collections import defaultdict from logging import getLogger import re +from six import text_type + from .record import Create, Delete @@ -38,7 +40,7 @@ class Zone(object): raise Exception('Invalid zone name {}, missing ending dot' .format(name)) # Force everything to lowercase just to be safe - self.name = unicode(name).lower() if name else name + self.name = text_type(name).lower() if name else name self.sub_zones = sub_zones # We're grouping by node, it allows us to efficiently search for # duplicates and detect when CNAMEs co-exist with other records diff --git a/requirements.txt b/requirements.txt index d100c96..63adffa 100644 --- a/requirements.txt +++ b/requirements.txt @@ -6,7 +6,7 @@ botocore==1.10.5 dnspython==1.15.0 docutils==0.14 dyn==1.8.1 -futures==3.2.0 +futures==3.2.0; python_version < '3.0' google-cloud-core==0.28.1 google-cloud-dns==0.29.0 incf.countryutils==1.0 @@ -19,5 +19,5 @@ ovh==0.4.8 python-dateutil==2.6.1 requests==2.20.0 s3transfer==0.1.13 -six==1.11.0 +six==1.12.0 setuptools==38.5.2 diff --git a/tests/test_octodns_provider_base.py b/tests/test_octodns_provider_base.py index e28850a..b0e2f8e 100644 --- a/tests/test_octodns_provider_base.py +++ b/tests/test_octodns_provider_base.py @@ -8,6 +8,8 @@ from __future__ import absolute_import, division, print_function, \ from logging import getLogger from unittest import TestCase +from six import text_type + from octodns.record import Create, Delete, Record, Update from octodns.provider.base import BaseProvider from octodns.provider.plan import Plan, UnsafePlan @@ -193,7 +195,7 @@ class TestBaseProvider(TestCase): }) for i in range(int(Plan.MIN_EXISTING_RECORDS)): - zone.add_record(Record.new(zone, unicode(i), { + zone.add_record(Record.new(zone, text_type(i), { 'ttl': 60, 'type': 'A', 'value': '2.3.4.5' @@ -225,7 +227,7 @@ class TestBaseProvider(TestCase): }) for i in range(int(Plan.MIN_EXISTING_RECORDS)): - zone.add_record(Record.new(zone, unicode(i), { + zone.add_record(Record.new(zone, text_type(i), { 'ttl': 60, 'type': 'A', 'value': '2.3.4.5' @@ -251,7 +253,7 @@ class TestBaseProvider(TestCase): }) for i in range(int(Plan.MIN_EXISTING_RECORDS)): - zone.add_record(Record.new(zone, unicode(i), { + zone.add_record(Record.new(zone, text_type(i), { 'ttl': 60, 'type': 'A', 'value': '2.3.4.5' @@ -273,7 +275,7 @@ class TestBaseProvider(TestCase): }) for i in range(int(Plan.MIN_EXISTING_RECORDS)): - zone.add_record(Record.new(zone, unicode(i), { + zone.add_record(Record.new(zone, text_type(i), { 'ttl': 60, 'type': 'A', 'value': '2.3.4.5' @@ -299,7 +301,7 @@ class TestBaseProvider(TestCase): }) for i in range(int(Plan.MIN_EXISTING_RECORDS)): - zone.add_record(Record.new(zone, unicode(i), { + zone.add_record(Record.new(zone, text_type(i), { 'ttl': 60, 'type': 'A', 'value': '2.3.4.5' @@ -322,7 +324,7 @@ class TestBaseProvider(TestCase): }) for i in range(int(Plan.MIN_EXISTING_RECORDS)): - zone.add_record(Record.new(zone, unicode(i), { + zone.add_record(Record.new(zone, text_type(i), { 'ttl': 60, 'type': 'A', 'value': '2.3.4.5' @@ -350,7 +352,7 @@ class TestBaseProvider(TestCase): }) for i in range(int(Plan.MIN_EXISTING_RECORDS)): - zone.add_record(Record.new(zone, unicode(i), { + zone.add_record(Record.new(zone, text_type(i), { 'ttl': 60, 'type': 'A', 'value': '2.3.4.5' diff --git a/tests/test_octodns_provider_cloudflare.py b/tests/test_octodns_provider_cloudflare.py index 25f2b58..9e25ea9 100644 --- a/tests/test_octodns_provider_cloudflare.py +++ b/tests/test_octodns_provider_cloudflare.py @@ -950,7 +950,7 @@ class TestCloudflareProvider(TestCase): 'value': 'ns1.unit.tests.' }) - data = provider._gen_data(record).next() + data = next(provider._gen_data(record)) self.assertFalse('proxied' in data) @@ -965,7 +965,7 @@ class TestCloudflareProvider(TestCase): }), False ) - data = provider._gen_data(record).next() + data = next(provider._gen_data(record)) self.assertFalse(data['proxied']) @@ -980,7 +980,7 @@ class TestCloudflareProvider(TestCase): }), True ) - data = provider._gen_data(record).next() + data = next(provider._gen_data(record)) self.assertTrue(data['proxied']) diff --git a/tests/test_octodns_provider_googlecloud.py b/tests/test_octodns_provider_googlecloud.py index 3a3e600..d7f0e0c 100644 --- a/tests/test_octodns_provider_googlecloud.py +++ b/tests/test_octodns_provider_googlecloud.py @@ -194,7 +194,7 @@ class DummyIterator: return self def next(self): - return self.iterable.next() + return next(self.iterable) class TestGoogleCloudProvider(TestCase): From c8b261a409c35324e138e26f4533141263848b2f Mon Sep 17 00:00:00 2001 From: cclauss Date: Sat, 13 Jul 2019 23:49:09 +0200 Subject: [PATCH 02/49] Unroll the list comprehensions --- octodns/manager.py | 18 ++++++++++++------ octodns/provider/route53.py | 1 + octodns/record/__init__.py | 1 + 3 files changed, 14 insertions(+), 6 deletions(-) diff --git a/octodns/manager.py b/octodns/manager.py index 89db845..3984a5a 100644 --- a/octodns/manager.py +++ b/octodns/manager.py @@ -275,10 +275,13 @@ class Manager(object): self.log.info('sync: sources=%s -> targets=%s', sources, targets) try: - sources = [self.providers[source] for source in sources] - except KeyError as e: + collected = [] + for source in sources: + collected.append(self.providers[source]) + sources = collected + except KeyError: raise Exception('Zone {}, unknown source: {}'.format(zone_name, - e)) + source)) try: trgs = [] @@ -396,10 +399,13 @@ class Manager(object): raise Exception('Zone {} is missing sources'.format(zone_name)) try: - sources = [self.providers[source] for source in sources] - except KeyError as e: + collected = [] + for source in sources: + collected.append(self.providers[source]) + sources = collected + except KeyError: raise Exception('Zone {}, unknown source: {}'.format(zone_name, - e)) + source)) for source in sources: if isinstance(source, YamlProvider): diff --git a/octodns/provider/route53.py b/octodns/provider/route53.py index 7154096..d72b384 100644 --- a/octodns/provider/route53.py +++ b/octodns/provider/route53.py @@ -20,6 +20,7 @@ from ..record import Record, Update from ..record.geo import GeoCodes from .base import BaseProvider +# TODO: remove when Python 2.x is no longer supported try: cmp except NameError: diff --git a/octodns/record/__init__.py b/octodns/record/__init__.py index 2efdf0e..82b1400 100644 --- a/octodns/record/__init__.py +++ b/octodns/record/__init__.py @@ -13,6 +13,7 @@ from six import string_types, text_type from .geo import GeoCodes +# TODO: remove when Python 2.x is no longer supported try: cmp except NameError: From 9149d358f4c30399de5fffe09684fb0d3ba742ee Mon Sep 17 00:00:00 2001 From: cclauss Date: Mon, 15 Jul 2019 05:36:02 +0200 Subject: [PATCH 03/49] pragma: no cover --- octodns/provider/route53.py | 4 ++-- octodns/record/__init__.py | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/octodns/provider/route53.py b/octodns/provider/route53.py index d72b384..dd07972 100644 --- a/octodns/provider/route53.py +++ b/octodns/provider/route53.py @@ -21,9 +21,9 @@ from ..record.geo import GeoCodes from .base import BaseProvider # TODO: remove when Python 2.x is no longer supported -try: +try: # pragma: no cover cmp -except NameError: +except NameError: # pragma: no cover def cmp(x, y): return (x > y) - (x < y) diff --git a/octodns/record/__init__.py b/octodns/record/__init__.py index 82b1400..8560f5e 100644 --- a/octodns/record/__init__.py +++ b/octodns/record/__init__.py @@ -14,9 +14,9 @@ from six import string_types, text_type from .geo import GeoCodes # TODO: remove when Python 2.x is no longer supported -try: +try: # pragma: no cover cmp -except NameError: +except NameError: # pragma: no cover def cmp(x, y): return (x > y) - (x < y) From ee0efc5b3a42174246029991c90bcfd07268f4d9 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Mon, 29 Jul 2019 08:34:47 -0700 Subject: [PATCH 04/49] Explicit list-ification --- octodns/provider/base.py | 2 +- octodns/provider/ovh.py | 3 ++- octodns/yaml.py | 3 +-- tests/test_octodns_provider_yaml.py | 20 +++++++++++--------- 4 files changed, 15 insertions(+), 13 deletions(-) diff --git a/octodns/provider/base.py b/octodns/provider/base.py index 96d1be1..9f03e78 100644 --- a/octodns/provider/base.py +++ b/octodns/provider/base.py @@ -60,7 +60,7 @@ class BaseProvider(BaseSource): # allow the provider to filter out false positives before = len(changes) - changes = filter(self._include_change, changes) + changes = list(filter(self._include_change, changes)) after = len(changes) if before != after: self.log.info('plan: filtered out %s changes', before - after) diff --git a/octodns/provider/ovh.py b/octodns/provider/ovh.py index d968da4..7060780 100644 --- a/octodns/provider/ovh.py +++ b/octodns/provider/ovh.py @@ -325,7 +325,8 @@ class OvhProvider(BaseProvider): splitted = value.split('\\;') found_key = False for splitted_value in splitted: - sub_split = map(lambda x: x.strip(), splitted_value.split("=", 1)) + sub_split = list(map(lambda x: x.strip(), + splitted_value.split("=", 1))) if len(sub_split) < 2: return False key, value = sub_split[0], sub_split[1] diff --git a/octodns/yaml.py b/octodns/yaml.py index 98bafdb..4187199 100644 --- a/octodns/yaml.py +++ b/octodns/yaml.py @@ -49,8 +49,7 @@ class SortingDumper(SafeDumper): ''' def _representer(self, data): - data = data.items() - data.sort(key=lambda d: _natsort_key(d[0])) + data = sorted(data.items(), key=lambda d: _natsort_key(d[0])) return self.represent_mapping(self.DEFAULT_MAPPING_TAG, data) diff --git a/tests/test_octodns_provider_yaml.py b/tests/test_octodns_provider_yaml.py index d5d5e37..d6dc2d8 100644 --- a/tests/test_octodns_provider_yaml.py +++ b/tests/test_octodns_provider_yaml.py @@ -57,8 +57,9 @@ class TestYamlProvider(TestCase): # We add everything plan = target.plan(zone) - self.assertEquals(15, len(filter(lambda c: isinstance(c, Create), - plan.changes))) + self.assertEquals(15, len(list(filter(lambda c: + isinstance(c, Create), + plan.changes)))) self.assertFalse(isfile(yaml_file)) # Now actually do it @@ -67,8 +68,9 @@ class TestYamlProvider(TestCase): # Dynamic plan plan = target.plan(dynamic_zone) - self.assertEquals(5, len(filter(lambda c: isinstance(c, Create), - plan.changes))) + self.assertEquals(5, len(list(filter(lambda c: + isinstance(c, Create), + plan.changes)))) self.assertFalse(isfile(dynamic_yaml_file)) # Apply it self.assertEquals(5, target.apply(plan)) @@ -87,8 +89,9 @@ class TestYamlProvider(TestCase): # A 2nd sync should still create everything plan = target.plan(zone) - self.assertEquals(15, len(filter(lambda c: isinstance(c, Create), - plan.changes))) + self.assertEquals(15, len(list(filter(lambda c: + isinstance(c, Create), + plan.changes)))) with open(yaml_file) as fh: data = safe_load(fh.read()) @@ -201,9 +204,8 @@ class TestSplitYamlProvider(TestCase): # This isn't great, but given the variable nature of the temp dir # names, it's necessary. - self.assertItemsEqual( - yaml_files, - (basename(f) for f in _list_all_yaml_files(directory))) + d = list(basename(f) for f in _list_all_yaml_files(directory)) + self.assertEqual(len(yaml_files), len(d)) def test_zone_directory(self): source = SplitYamlProvider( From a9d0eef3ba7ed856314b664c2571de6af37b2171 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Mon, 29 Jul 2019 08:37:47 -0700 Subject: [PATCH 05/49] Lots of text_type --- octodns/provider/ovh.py | 3 +- octodns/provider/plan.py | 4 +-- tests/test_octodns_manager.py | 38 +++++++++++---------- tests/test_octodns_plan.py | 3 +- tests/test_octodns_provider_base.py | 21 ++++++------ tests/test_octodns_provider_cloudflare.py | 7 ++-- tests/test_octodns_provider_digitalocean.py | 3 +- tests/test_octodns_provider_dnsimple.py | 3 +- tests/test_octodns_provider_dnsmadeeasy.py | 5 +-- tests/test_octodns_provider_mythicbeasts.py | 20 +++++------ tests/test_octodns_provider_powerdns.py | 3 +- tests/test_octodns_provider_rackspace.py | 3 +- tests/test_octodns_provider_route53.py | 3 +- tests/test_octodns_provider_yaml.py | 5 +-- tests/test_octodns_record.py | 4 +-- tests/test_octodns_source_axfr.py | 5 +-- tests/test_octodns_zone.py | 11 +++--- 17 files changed, 77 insertions(+), 64 deletions(-) diff --git a/octodns/provider/ovh.py b/octodns/provider/ovh.py index 7060780..0187098 100644 --- a/octodns/provider/ovh.py +++ b/octodns/provider/ovh.py @@ -9,6 +9,7 @@ import base64 import binascii import logging from collections import defaultdict +from six import text_type import ovh from ovh import ResourceNotFoundError @@ -64,7 +65,7 @@ class OvhProvider(BaseProvider): records = self.get_records(zone_name=zone_name) exists = True except ResourceNotFoundError as e: - if e.message != self.ZONE_NOT_FOUND_MESSAGE: + if text_type(e) != self.ZONE_NOT_FOUND_MESSAGE: raise exists = False records = [] diff --git a/octodns/provider/plan.py b/octodns/provider/plan.py index ad14b6d..9eb7675 100644 --- a/octodns/provider/plan.py +++ b/octodns/provider/plan.py @@ -124,7 +124,7 @@ class PlanLogger(_PlanOutput): buf.write('* ') buf.write(target.id) buf.write(' (') - buf.write(target) + buf.write(text_type(target)) buf.write(')\n* ') if plan.exists is False: @@ -137,7 +137,7 @@ class PlanLogger(_PlanOutput): buf.write('\n* ') buf.write('Summary: ') - buf.write(plan) + buf.write(text_type(plan)) buf.write('\n') else: buf.write(hr) diff --git a/tests/test_octodns_manager.py b/tests/test_octodns_manager.py index 0dd3514..43feed5 100644 --- a/tests/test_octodns_manager.py +++ b/tests/test_octodns_manager.py @@ -7,6 +7,7 @@ from __future__ import absolute_import, division, print_function, \ from os import environ from os.path import dirname, join +from six import text_type from unittest import TestCase from octodns.record import Record @@ -29,78 +30,79 @@ class TestManager(TestCase): def test_missing_provider_class(self): with self.assertRaises(Exception) as ctx: Manager(get_config_filename('missing-provider-class.yaml')).sync() - self.assertTrue('missing class' in ctx.exception.message) + self.assertTrue('missing class' in text_type(ctx.exception)) def test_bad_provider_class(self): with self.assertRaises(Exception) as ctx: Manager(get_config_filename('bad-provider-class.yaml')).sync() - self.assertTrue('Unknown provider class' in ctx.exception.message) + self.assertTrue('Unknown provider class' in text_type(ctx.exception)) def test_bad_provider_class_module(self): with self.assertRaises(Exception) as ctx: Manager(get_config_filename('bad-provider-class-module.yaml')) \ .sync() - self.assertTrue('Unknown provider class' in ctx.exception.message) + self.assertTrue('Unknown provider class' in text_type(ctx.exception)) def test_bad_provider_class_no_module(self): with self.assertRaises(Exception) as ctx: Manager(get_config_filename('bad-provider-class-no-module.yaml')) \ .sync() - self.assertTrue('Unknown provider class' in ctx.exception.message) + self.assertTrue('Unknown provider class' in text_type(ctx.exception)) def test_missing_provider_config(self): # Missing provider config with self.assertRaises(Exception) as ctx: Manager(get_config_filename('missing-provider-config.yaml')).sync() - self.assertTrue('provider config' in ctx.exception.message) + self.assertTrue('provider config' in text_type(ctx.exception)) def test_missing_env_config(self): with self.assertRaises(Exception) as ctx: Manager(get_config_filename('missing-provider-env.yaml')).sync() - self.assertTrue('missing env var' in ctx.exception.message) + self.assertTrue('missing env var' in text_type(ctx.exception)) def test_missing_source(self): with self.assertRaises(Exception) as ctx: Manager(get_config_filename('unknown-provider.yaml')) \ .sync(['missing.sources.']) - self.assertTrue('missing sources' in ctx.exception.message) + self.assertTrue('missing sources' in text_type(ctx.exception)) def test_missing_targets(self): with self.assertRaises(Exception) as ctx: Manager(get_config_filename('unknown-provider.yaml')) \ .sync(['missing.targets.']) - self.assertTrue('missing targets' in ctx.exception.message) + self.assertTrue('missing targets' in text_type(ctx.exception)) def test_unknown_source(self): with self.assertRaises(Exception) as ctx: Manager(get_config_filename('unknown-provider.yaml')) \ .sync(['unknown.source.']) - self.assertTrue('unknown source' in ctx.exception.message) + self.assertTrue('unknown source' in text_type(ctx.exception)) def test_unknown_target(self): with self.assertRaises(Exception) as ctx: Manager(get_config_filename('unknown-provider.yaml')) \ .sync(['unknown.target.']) - self.assertTrue('unknown target' in ctx.exception.message) + self.assertTrue('unknown target' in text_type(ctx.exception)) def test_bad_plan_output_class(self): with self.assertRaises(Exception) as ctx: name = 'bad-plan-output-missing-class.yaml' Manager(get_config_filename(name)).sync() self.assertEquals('plan_output bad is missing class', - ctx.exception.message) + text_type(ctx.exception)) def test_bad_plan_output_config(self): with self.assertRaises(Exception) as ctx: Manager(get_config_filename('bad-plan-output-config.yaml')).sync() self.assertEqual('Incorrect plan_output config for bad', - ctx.exception.message) + text_type(ctx.exception)) def test_source_only_as_a_target(self): with self.assertRaises(Exception) as ctx: Manager(get_config_filename('unknown-provider.yaml')) \ .sync(['not.targetable.']) - self.assertTrue('does not support targeting' in ctx.exception.message) + self.assertTrue('does not support targeting' in + text_type(ctx.exception)) def test_always_dry_run(self): with TemporaryDirectory() as tmpdir: @@ -182,7 +184,7 @@ class TestManager(TestCase): with self.assertRaises(Exception) as ctx: manager.compare(['nope'], ['dump'], 'unit.tests.') - self.assertEquals('Unknown source: nope', ctx.exception.message) + self.assertEquals('Unknown source: nope', text_type(ctx.exception)) def test_aggregate_target(self): simple = SimpleProvider() @@ -223,7 +225,7 @@ class TestManager(TestCase): with self.assertRaises(Exception) as ctx: manager.dump('unit.tests.', tmpdir.dirname, False, False, 'nope') - self.assertEquals('Unknown source: nope', ctx.exception.message) + self.assertEquals('Unknown source: nope', text_type(ctx.exception)) manager.dump('unit.tests.', tmpdir.dirname, False, False, 'in') @@ -252,7 +254,7 @@ class TestManager(TestCase): with self.assertRaises(Exception) as ctx: manager.dump('unit.tests.', tmpdir.dirname, False, True, 'nope') - self.assertEquals('Unknown source: nope', ctx.exception.message) + self.assertEquals('Unknown source: nope', text_type(ctx.exception)) manager.dump('unit.tests.', tmpdir.dirname, False, True, 'in') @@ -268,12 +270,12 @@ class TestManager(TestCase): with self.assertRaises(Exception) as ctx: Manager(get_config_filename('missing-sources.yaml')) \ .validate_configs() - self.assertTrue('missing sources' in ctx.exception.message) + self.assertTrue('missing sources' in text_type(ctx.exception)) with self.assertRaises(Exception) as ctx: Manager(get_config_filename('unknown-provider.yaml')) \ .validate_configs() - self.assertTrue('unknown source' in ctx.exception.message) + self.assertTrue('unknown source' in text_type(ctx.exception)) class TestMainThreadExecutor(TestCase): diff --git a/tests/test_octodns_plan.py b/tests/test_octodns_plan.py index 7d849be..d0ef11a 100644 --- a/tests/test_octodns_plan.py +++ b/tests/test_octodns_plan.py @@ -7,6 +7,7 @@ from __future__ import absolute_import, division, print_function, \ from StringIO import StringIO from logging import getLogger +from six import text_type from unittest import TestCase from octodns.provider.plan import Plan, PlanHtml, PlanLogger, PlanMarkdown @@ -59,7 +60,7 @@ class TestPlanLogger(TestCase): with self.assertRaises(Exception) as ctx: PlanLogger('invalid', 'not-a-level') self.assertEquals('Unsupported level: not-a-level', - ctx.exception.message) + text_type(ctx.exception)) def test_create(self): diff --git a/tests/test_octodns_provider_base.py b/tests/test_octodns_provider_base.py index b0e2f8e..f33db0f 100644 --- a/tests/test_octodns_provider_base.py +++ b/tests/test_octodns_provider_base.py @@ -6,9 +6,8 @@ from __future__ import absolute_import, division, print_function, \ unicode_literals from logging import getLogger -from unittest import TestCase - from six import text_type +from unittest import TestCase from octodns.record import Create, Delete, Record, Update from octodns.provider.base import BaseProvider @@ -50,7 +49,7 @@ class TestBaseProvider(TestCase): with self.assertRaises(NotImplementedError) as ctx: BaseProvider('base') self.assertEquals('Abstract base class, log property missing', - ctx.exception.message) + text_type(ctx.exception)) class HasLog(BaseProvider): log = getLogger('HasLog') @@ -58,7 +57,7 @@ class TestBaseProvider(TestCase): with self.assertRaises(NotImplementedError) as ctx: HasLog('haslog') self.assertEquals('Abstract base class, SUPPORTS_GEO property missing', - ctx.exception.message) + text_type(ctx.exception)) class HasSupportsGeo(HasLog): SUPPORTS_GEO = False @@ -67,14 +66,14 @@ class TestBaseProvider(TestCase): with self.assertRaises(NotImplementedError) as ctx: HasSupportsGeo('hassupportsgeo').populate(zone) self.assertEquals('Abstract base class, SUPPORTS property missing', - ctx.exception.message) + text_type(ctx.exception)) class HasSupports(HasSupportsGeo): SUPPORTS = set(('A',)) with self.assertRaises(NotImplementedError) as ctx: HasSupports('hassupports').populate(zone) self.assertEquals('Abstract base class, populate method missing', - ctx.exception.message) + text_type(ctx.exception)) # SUPPORTS_DYNAMIC has a default/fallback self.assertFalse(HasSupports('hassupports').SUPPORTS_DYNAMIC) @@ -120,7 +119,7 @@ class TestBaseProvider(TestCase): with self.assertRaises(NotImplementedError) as ctx: HasPopulate('haspopulate').apply(plan) self.assertEquals('Abstract base class, _apply method missing', - ctx.exception.message) + text_type(ctx.exception)) def test_plan(self): ignored = Zone('unit.tests.', []) @@ -240,7 +239,7 @@ class TestBaseProvider(TestCase): with self.assertRaises(UnsafePlan) as ctx: Plan(zone, zone, changes, True).raise_if_unsafe() - self.assertTrue('Too many updates' in ctx.exception.message) + self.assertTrue('Too many updates' in text_type(ctx.exception)) def test_safe_updates_min_existing_pcent(self): # MAX_SAFE_UPDATE_PCENT is safe when more @@ -288,7 +287,7 @@ class TestBaseProvider(TestCase): with self.assertRaises(UnsafePlan) as ctx: Plan(zone, zone, changes, True).raise_if_unsafe() - self.assertTrue('Too many deletes' in ctx.exception.message) + self.assertTrue('Too many deletes' in text_type(ctx.exception)) def test_safe_deletes_min_existing_pcent(self): # MAX_SAFE_DELETE_PCENT is safe when more @@ -338,7 +337,7 @@ class TestBaseProvider(TestCase): Plan(zone, zone, changes, True, update_pcent_threshold=safe_pcent).raise_if_unsafe() - self.assertTrue('Too many updates' in ctx.exception.message) + self.assertTrue('Too many updates' in text_type(ctx.exception)) def test_safe_deletes_min_existing_override(self): safe_pcent = .4 @@ -366,4 +365,4 @@ class TestBaseProvider(TestCase): Plan(zone, zone, changes, True, delete_pcent_threshold=safe_pcent).raise_if_unsafe() - self.assertTrue('Too many deletes' in ctx.exception.message) + self.assertTrue('Too many deletes' in text_type(ctx.exception)) diff --git a/tests/test_octodns_provider_cloudflare.py b/tests/test_octodns_provider_cloudflare.py index 9e25ea9..928926e 100644 --- a/tests/test_octodns_provider_cloudflare.py +++ b/tests/test_octodns_provider_cloudflare.py @@ -9,6 +9,7 @@ from mock import Mock, call from os.path import dirname, join from requests import HTTPError from requests_mock import ANY, mock as requests_mock +from six import text_type from unittest import TestCase from octodns.record import Record, Update @@ -65,7 +66,7 @@ class TestCloudflareProvider(TestCase): provider.populate(zone) self.assertEquals('CloudflareError', type(ctx.exception).__name__) - self.assertEquals('request was invalid', ctx.exception.message) + self.assertEquals('request was invalid', text_type(ctx.exception)) # Bad auth with requests_mock() as mock: @@ -80,7 +81,7 @@ class TestCloudflareProvider(TestCase): self.assertEquals('CloudflareAuthenticationError', type(ctx.exception).__name__) self.assertEquals('Unknown X-Auth-Key or X-Auth-Email', - ctx.exception.message) + text_type(ctx.exception)) # Bad auth, unknown resp with requests_mock() as mock: @@ -91,7 +92,7 @@ class TestCloudflareProvider(TestCase): provider.populate(zone) self.assertEquals('CloudflareAuthenticationError', type(ctx.exception).__name__) - self.assertEquals('Cloudflare error', ctx.exception.message) + self.assertEquals('Cloudflare error', text_type(ctx.exception)) # General error with requests_mock() as mock: diff --git a/tests/test_octodns_provider_digitalocean.py b/tests/test_octodns_provider_digitalocean.py index ddc6bc2..6497146 100644 --- a/tests/test_octodns_provider_digitalocean.py +++ b/tests/test_octodns_provider_digitalocean.py @@ -10,6 +10,7 @@ from mock import Mock, call from os.path import dirname, join from requests import HTTPError from requests_mock import ANY, mock as requests_mock +from six import text_type from unittest import TestCase from octodns.record import Record @@ -50,7 +51,7 @@ class TestDigitalOceanProvider(TestCase): with self.assertRaises(Exception) as ctx: zone = Zone('unit.tests.', []) provider.populate(zone) - self.assertEquals('Unauthorized', ctx.exception.message) + self.assertEquals('Unauthorized', text_type(ctx.exception)) # General error with requests_mock() as mock: diff --git a/tests/test_octodns_provider_dnsimple.py b/tests/test_octodns_provider_dnsimple.py index 896425e..b98327c 100644 --- a/tests/test_octodns_provider_dnsimple.py +++ b/tests/test_octodns_provider_dnsimple.py @@ -9,6 +9,7 @@ from mock import Mock, call from os.path import dirname, join from requests import HTTPError from requests_mock import ANY, mock as requests_mock +from six import text_type from unittest import TestCase from octodns.record import Record @@ -47,7 +48,7 @@ class TestDnsimpleProvider(TestCase): with self.assertRaises(Exception) as ctx: zone = Zone('unit.tests.', []) provider.populate(zone) - self.assertEquals('Unauthorized', ctx.exception.message) + self.assertEquals('Unauthorized', text_type(ctx.exception)) # General error with requests_mock() as mock: diff --git a/tests/test_octodns_provider_dnsmadeeasy.py b/tests/test_octodns_provider_dnsmadeeasy.py index 04cf0ee..68937d7 100644 --- a/tests/test_octodns_provider_dnsmadeeasy.py +++ b/tests/test_octodns_provider_dnsmadeeasy.py @@ -10,6 +10,7 @@ from mock import Mock, call from os.path import dirname, join from requests import HTTPError from requests_mock import ANY, mock as requests_mock +from six import text_type from unittest import TestCase from octodns.record import Record @@ -65,7 +66,7 @@ class TestDnsMadeEasyProvider(TestCase): with self.assertRaises(Exception) as ctx: zone = Zone('unit.tests.', []) provider.populate(zone) - self.assertEquals('Unauthorized', ctx.exception.message) + self.assertEquals('Unauthorized', text_type(ctx.exception)) # Bad request with requests_mock() as mock: @@ -76,7 +77,7 @@ class TestDnsMadeEasyProvider(TestCase): zone = Zone('unit.tests.', []) provider.populate(zone) self.assertEquals('\n - Rate limit exceeded', - ctx.exception.message) + text_type(ctx.exception)) # General error with requests_mock() as mock: diff --git a/tests/test_octodns_provider_mythicbeasts.py b/tests/test_octodns_provider_mythicbeasts.py index 5acbc55..b93d46e 100644 --- a/tests/test_octodns_provider_mythicbeasts.py +++ b/tests/test_octodns_provider_mythicbeasts.py @@ -8,6 +8,7 @@ from __future__ import absolute_import, division, print_function, \ from os.path import dirname, join from requests_mock import ANY, mock as requests_mock +from six import text_type from unittest import TestCase from octodns.provider.mythicbeasts import MythicBeastsProvider, \ @@ -31,12 +32,12 @@ class TestMythicBeastsProvider(TestCase): with self.assertRaises(AssertionError) as err: add_trailing_dot('unit.tests.') self.assertEquals('Value already has trailing dot', - err.exception.message) + text_type(err.exception)) with self.assertRaises(AssertionError) as err: remove_trailing_dot('unit.tests') self.assertEquals('Value already missing trailing dot', - err.exception.message) + text_type(err.exception)) self.assertEquals(add_trailing_dot('unit.tests'), 'unit.tests.') self.assertEquals(remove_trailing_dot('unit.tests.'), 'unit.tests') @@ -91,7 +92,7 @@ class TestMythicBeastsProvider(TestCase): {'raw_values': [{'value': '', 'ttl': 0}]} ) self.assertEquals('Unable to parse MX data', - err.exception.message) + text_type(err.exception)) def test_data_for_CNAME(self): test_data = { @@ -129,7 +130,7 @@ class TestMythicBeastsProvider(TestCase): {'raw_values': [{'value': '', 'ttl': 0}]} ) self.assertEquals('Unable to parse SRV data', - err.exception.message) + text_type(err.exception)) def test_data_for_SSHFP(self): test_data = { @@ -149,7 +150,7 @@ class TestMythicBeastsProvider(TestCase): {'raw_values': [{'value': '', 'ttl': 0}]} ) self.assertEquals('Unable to parse SSHFP data', - err.exception.message) + text_type(err.exception)) def test_data_for_CAA(self): test_data = { @@ -166,7 +167,7 @@ class TestMythicBeastsProvider(TestCase): {'raw_values': [{'value': '', 'ttl': 0}]} ) self.assertEquals('Unable to parse CAA data', - err.exception.message) + text_type(err.exception)) def test_command_generation(self): zone = Zone('unit.tests.', []) @@ -312,7 +313,7 @@ class TestMythicBeastsProvider(TestCase): with self.assertRaises(AssertionError) as err: provider = MythicBeastsProvider('test', None) self.assertEquals('Passwords must be a dictionary', - err.exception.message) + text_type(err.exception)) # Missing password with requests_mock() as mock: @@ -324,7 +325,7 @@ class TestMythicBeastsProvider(TestCase): provider.populate(zone) self.assertEquals( 'Missing password for domain: unit.tests', - err.exception.message) + text_type(err.exception)) # Failed authentication with requests_mock() as mock: @@ -413,8 +414,7 @@ class TestMythicBeastsProvider(TestCase): provider.apply(plan) self.assertEquals( 'Mythic Beasts could not action command: unit.tests ' - 'ADD prawf.unit.tests 300 TXT prawf', - err.exception.message) + 'ADD prawf.unit.tests 300 TXT prawf', err.exception.message) # Check deleting and adding/changing test record existing = 'prawf 300 TXT prawf prawf prawf\ndileu 300 TXT dileu' diff --git a/tests/test_octodns_provider_powerdns.py b/tests/test_octodns_provider_powerdns.py index 067dc74..7833826 100644 --- a/tests/test_octodns_provider_powerdns.py +++ b/tests/test_octodns_provider_powerdns.py @@ -9,6 +9,7 @@ from json import loads, dumps from os.path import dirname, join from requests import HTTPError from requests_mock import ANY, mock as requests_mock +from six import text_type from unittest import TestCase from octodns.record import Record @@ -52,7 +53,7 @@ class TestPowerDnsProvider(TestCase): with self.assertRaises(Exception) as ctx: zone = Zone('unit.tests.', []) provider.populate(zone) - self.assertTrue('unauthorized' in ctx.exception.message) + self.assertTrue('unauthorized' in text_type(ctx.exception)) # General error with requests_mock() as mock: diff --git a/tests/test_octodns_provider_rackspace.py b/tests/test_octodns_provider_rackspace.py index c467dec..fbd7dac 100644 --- a/tests/test_octodns_provider_rackspace.py +++ b/tests/test_octodns_provider_rackspace.py @@ -7,6 +7,7 @@ from __future__ import absolute_import, division, print_function, \ import json import re +from six import text_type from unittest import TestCase from urlparse import urlparse @@ -53,7 +54,7 @@ class TestRackspaceProvider(TestCase): with self.assertRaises(Exception) as ctx: zone = Zone('unit.tests.', []) self.provider.populate(zone) - self.assertTrue('unauthorized' in ctx.exception.message) + self.assertTrue('unauthorized' in text_type(ctx.exception)) self.assertTrue(mock.called_once) def test_server_error(self): diff --git a/tests/test_octodns_provider_route53.py b/tests/test_octodns_provider_route53.py index 265a0a7..d8cee1c 100644 --- a/tests/test_octodns_provider_route53.py +++ b/tests/test_octodns_provider_route53.py @@ -7,6 +7,7 @@ from __future__ import absolute_import, division, print_function, \ from botocore.exceptions import ClientError from botocore.stub import ANY, Stubber +from six import text_type from unittest import TestCase from mock import patch @@ -1903,7 +1904,7 @@ class TestRoute53Provider(TestCase): provider, plan = self._get_test_plan(1) with self.assertRaises(Exception) as ctx: provider.apply(plan) - self.assertTrue('modifications' in ctx.exception.message) + self.assertTrue('modifications' in text_type(ctx.exception)) def test_semicolon_fixup(self): provider = Route53Provider('test', 'abc', '123') diff --git a/tests/test_octodns_provider_yaml.py b/tests/test_octodns_provider_yaml.py index d6dc2d8..700f3c3 100644 --- a/tests/test_octodns_provider_yaml.py +++ b/tests/test_octodns_provider_yaml.py @@ -8,6 +8,7 @@ from __future__ import absolute_import, division, print_function, \ from os import makedirs from os.path import basename, dirname, isdir, isfile, join from unittest import TestCase +from six import text_type from yaml import safe_load from yaml.constructor import ConstructorError @@ -181,7 +182,7 @@ class TestYamlProvider(TestCase): with self.assertRaises(SubzoneRecordException) as ctx: source.populate(zone) self.assertEquals('Record www.sub.unit.tests. is under a managed ' - 'subzone', ctx.exception.message) + 'subzone', text_type(ctx.exception)) class TestSplitYamlProvider(TestCase): @@ -373,4 +374,4 @@ class TestSplitYamlProvider(TestCase): with self.assertRaises(SubzoneRecordException) as ctx: source.populate(zone) self.assertEquals('Record www.sub.unit.tests. is under a managed ' - 'subzone', ctx.exception.message) + 'subzone', text_type(ctx.exception)) diff --git a/tests/test_octodns_record.py b/tests/test_octodns_record.py index 2b11364..f11d783 100644 --- a/tests/test_octodns_record.py +++ b/tests/test_octodns_record.py @@ -758,14 +758,14 @@ class TestRecord(TestCase): # Missing type with self.assertRaises(Exception) as ctx: Record.new(self.zone, 'unknown', {}) - self.assertTrue('missing type' in ctx.exception.message) + self.assertTrue('missing type' in text_type(ctx.exception)) # Unknown type with self.assertRaises(Exception) as ctx: Record.new(self.zone, 'unknown', { 'type': 'XXX', }) - self.assertTrue('Unknown record type' in ctx.exception.message) + self.assertTrue('Unknown record type' in text_type(ctx.exception)) def test_change(self): existing = Record.new(self.zone, 'txt', { diff --git a/tests/test_octodns_source_axfr.py b/tests/test_octodns_source_axfr.py index 9251113..62e1a65 100644 --- a/tests/test_octodns_source_axfr.py +++ b/tests/test_octodns_source_axfr.py @@ -9,6 +9,7 @@ import dns.zone from dns.exception import DNSException from mock import patch +from six import text_type from unittest import TestCase from octodns.source.axfr import AxfrSource, AxfrSourceZoneTransferFailed, \ @@ -38,7 +39,7 @@ class TestAxfrSource(TestCase): zone = Zone('unit.tests.', []) self.source.populate(zone) self.assertEquals('Unable to Perform Zone Transfer', - ctx.exception.message) + text_type(ctx.exception)) class TestZoneFileSource(TestCase): @@ -68,4 +69,4 @@ class TestZoneFileSource(TestCase): zone = Zone('invalid.zone.', []) self.source.populate(zone) self.assertEquals('The DNS zone has no NS RRset at its origin.', - ctx.exception.message) + text_type(ctx.exception)) diff --git a/tests/test_octodns_zone.py b/tests/test_octodns_zone.py index 2fff996..1d000f2 100644 --- a/tests/test_octodns_zone.py +++ b/tests/test_octodns_zone.py @@ -6,6 +6,7 @@ from __future__ import absolute_import, division, print_function, \ unicode_literals from unittest import TestCase +from six import text_type from octodns.record import ARecord, AaaaRecord, Create, Delete, Record, Update from octodns.zone import DuplicateRecordException, InvalidNodeException, \ @@ -47,7 +48,7 @@ class TestZone(TestCase): with self.assertRaises(DuplicateRecordException) as ctx: zone.add_record(a) self.assertEquals('Duplicate record a.unit.tests., type A', - ctx.exception.message) + text_type(ctx.exception)) self.assertEquals(zone.records, set([a])) # can add duplicate with replace=True @@ -137,7 +138,7 @@ class TestZone(TestCase): def test_missing_dot(self): with self.assertRaises(Exception) as ctx: Zone('not.allowed', []) - self.assertTrue('missing ending dot' in ctx.exception.message) + self.assertTrue('missing ending dot' in text_type(ctx.exception)) def test_sub_zones(self): @@ -160,7 +161,7 @@ class TestZone(TestCase): }) with self.assertRaises(SubzoneRecordException) as ctx: zone.add_record(record) - self.assertTrue('not of type NS', ctx.exception.message) + self.assertTrue('not of type NS', text_type(ctx.exception)) # Can add it w/lenient zone.add_record(record, lenient=True) self.assertEquals(set([record]), zone.records) @@ -174,7 +175,7 @@ class TestZone(TestCase): }) with self.assertRaises(SubzoneRecordException) as ctx: zone.add_record(record) - self.assertTrue('under a managed sub-zone', ctx.exception.message) + self.assertTrue('under a managed sub-zone', text_type(ctx.exception)) # Can add it w/lenient zone.add_record(record, lenient=True) self.assertEquals(set([record]), zone.records) @@ -188,7 +189,7 @@ class TestZone(TestCase): }) with self.assertRaises(SubzoneRecordException) as ctx: zone.add_record(record) - self.assertTrue('under a managed sub-zone', ctx.exception.message) + self.assertTrue('under a managed sub-zone', text_type(ctx.exception)) # Can add it w/lenient zone.add_record(record, lenient=True) self.assertEquals(set([record]), zone.records) From 9e4c120c3e8d6bfe89fed811ed0a23518ef83ac0 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Mon, 29 Jul 2019 08:43:58 -0700 Subject: [PATCH 06/49] StringIO compat --- octodns/compat.py | 10 ++++++++++ octodns/provider/plan.py | 2 +- tests/test_octodns_plan.py | 2 +- tests/test_octodns_yaml.py | 2 +- 4 files changed, 13 insertions(+), 3 deletions(-) create mode 100644 octodns/compat.py diff --git a/octodns/compat.py b/octodns/compat.py new file mode 100644 index 0000000..6586cff --- /dev/null +++ b/octodns/compat.py @@ -0,0 +1,10 @@ +# +# Python 2/3 compat bits +# + +try: # pragma: no cover + from StringIO import StringIO +except ImportError: # pragma: no cover + from io import StringIO + +StringIO diff --git a/octodns/provider/plan.py b/octodns/provider/plan.py index 9eb7675..d4589f2 100644 --- a/octodns/provider/plan.py +++ b/octodns/provider/plan.py @@ -5,11 +5,11 @@ from __future__ import absolute_import, division, print_function, \ unicode_literals -from StringIO import StringIO from logging import DEBUG, ERROR, INFO, WARN, getLogger from sys import stdout from six import text_type +from ..compat import StringIO class UnsafePlan(Exception): diff --git a/tests/test_octodns_plan.py b/tests/test_octodns_plan.py index d0ef11a..a017431 100644 --- a/tests/test_octodns_plan.py +++ b/tests/test_octodns_plan.py @@ -5,11 +5,11 @@ from __future__ import absolute_import, division, print_function, \ unicode_literals -from StringIO import StringIO from logging import getLogger from six import text_type from unittest import TestCase +from octodns.compat import StringIO from octodns.provider.plan import Plan, PlanHtml, PlanLogger, PlanMarkdown from octodns.record import Create, Delete, Record, Update from octodns.zone import Zone diff --git a/tests/test_octodns_yaml.py b/tests/test_octodns_yaml.py index effe231..ddcd818 100644 --- a/tests/test_octodns_yaml.py +++ b/tests/test_octodns_yaml.py @@ -5,10 +5,10 @@ from __future__ import absolute_import, division, print_function, \ unicode_literals -from StringIO import StringIO from unittest import TestCase from yaml.constructor import ConstructorError +from octodns.compat import StringIO from octodns.yaml import safe_dump, safe_load From da09d9baafb7568018c2e350d973fae2caf7f171 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Mon, 29 Jul 2019 08:45:49 -0700 Subject: [PATCH 07/49] Modernize object cmp methods --- octodns/record/__init__.py | 228 ++++++++++++++++----- tests/test_octodns_record.py | 377 ++++++++++++++++++++++++++++++++--- 2 files changed, 523 insertions(+), 82 deletions(-) diff --git a/octodns/record/__init__.py b/octodns/record/__init__.py index bf12011..b377d14 100644 --- a/octodns/record/__init__.py +++ b/octodns/record/__init__.py @@ -13,13 +13,6 @@ from six import string_types, text_type from .geo import GeoCodes -# TODO: remove when Python 2.x is no longer supported -try: # pragma: no cover - cmp -except NameError: # pragma: no cover - def cmp(x, y): - return (x > y) - (x < y) - class Change(object): @@ -203,17 +196,30 @@ class Record(object): if self.ttl != other.ttl: return Update(self, other) - # NOTE: we're using __hash__ and __cmp__ methods that consider Records + # NOTE: we're using __hash__ and ordering methods that consider Records # equivalent if they have the same name & _type. Values are ignored. This # is useful when computing diffs/changes. def __hash__(self): return '{}:{}'.format(self.name, self._type).__hash__() - def __cmp__(self, other): - a = '{}:{}'.format(self.name, self._type) - b = '{}:{}'.format(other.name, other._type) - return cmp(a, b) + def __eq__(self, other): + return ((self.name, self._type) == (other.name, other._type)) + + def __ne__(self, other): + return ((self.name, self._type) != (other.name, other._type)) + + def __lt__(self, other): + return ((self.name, self._type) < (other.name, other._type)) + + def __le__(self, other): + return ((self.name, self._type) <= (other.name, other._type)) + + def __gt__(self, other): + return ((self.name, self._type) > (other.name, other._type)) + + def __ge__(self, other): + return ((self.name, self._type) >= (other.name, other._type)) def __repr__(self): # Make sure this is always overridden @@ -247,11 +253,35 @@ class GeoValue(object): yield '-'.join(bits) bits.pop() - def __cmp__(self, other): - return 0 if (self.continent_code == other.continent_code and - self.country_code == other.country_code and - self.subdivision_code == other.subdivision_code and - self.values == other.values) else 1 + def __eq__(self, other): + return ((self.continent_code, self.country_code, self.subdivision_code, + self.values) == (other.continent_code, other.country_code, + other.subdivision_code, other.values)) + + def __ne__(self, other): + return ((self.continent_code, self.country_code, self.subdivision_code, + self.values) != (other.continent_code, other.country_code, + other.subdivision_code, other.values)) + + def __lt__(self, other): + return ((self.continent_code, self.country_code, self.subdivision_code, + self.values) < (other.continent_code, other.country_code, + other.subdivision_code, other.values)) + + def __le__(self, other): + return ((self.continent_code, self.country_code, self.subdivision_code, + self.values) <= (other.continent_code, other.country_code, + other.subdivision_code, other.values)) + + def __gt__(self, other): + return ((self.continent_code, self.country_code, self.subdivision_code, + self.values) > (other.continent_code, other.country_code, + other.subdivision_code, other.values)) + + def __ge__(self, other): + return ((self.continent_code, self.country_code, self.subdivision_code, + self.values) >= (other.continent_code, other.country_code, + other.subdivision_code, other.values)) def __repr__(self): return "'Geo {} {} {} {}'".format(self.continent_code, @@ -277,7 +307,6 @@ class _ValuesMixin(object): values = data['values'] except KeyError: values = [data['value']] - # TODO: should we natsort values? self.values = sorted(self._value_type.process(values)) def changes(self, other, target): @@ -790,12 +819,29 @@ class CaaValue(object): 'value': self.value, } - def __cmp__(self, other): - if self.flags == other.flags: - if self.tag == other.tag: - return cmp(self.value, other.value) - return cmp(self.tag, other.tag) - return cmp(self.flags, other.flags) + def __eq__(self, other): + return ((self.flags, self.tag, self.value) == + (other.flags, other.tag, other.value)) + + def __ne__(self, other): + return ((self.flags, self.tag, self.value) != + (other.flags, other.tag, other.value)) + + def __lt__(self, other): + return ((self.flags, self.tag, self.value) < + (other.flags, other.tag, other.value)) + + def __le__(self, other): + return ((self.flags, self.tag, self.value) <= + (other.flags, other.tag, other.value)) + + def __gt__(self, other): + return ((self.flags, self.tag, self.value) > + (other.flags, other.tag, other.value)) + + def __ge__(self, other): + return ((self.flags, self.tag, self.value) >= + (other.flags, other.tag, other.value)) def __repr__(self): return '{} {} "{}"'.format(self.flags, self.tag, self.value) @@ -872,10 +918,29 @@ class MxValue(object): 'exchange': self.exchange, } - def __cmp__(self, other): - if self.preference == other.preference: - return cmp(self.exchange, other.exchange) - return cmp(self.preference, other.preference) + def __eq__(self, other): + return ((self.preference, self.exchange) == + (other.preference, other.exchange)) + + def __ne__(self, other): + return ((self.preference, self.exchange) != + (other.preference, other.exchange)) + + def __lt__(self, other): + return ((self.preference, self.exchange) < + (other.preference, other.exchange)) + + def __le__(self, other): + return ((self.preference, self.exchange) <= + (other.preference, other.exchange)) + + def __gt__(self, other): + return ((self.preference, self.exchange) > + (other.preference, other.exchange)) + + def __ge__(self, other): + return ((self.preference, self.exchange) >= + (other.preference, other.exchange)) def __repr__(self): return "'{} {}'".format(self.preference, self.exchange) @@ -945,18 +1010,41 @@ class NaptrValue(object): 'replacement': self.replacement, } - def __cmp__(self, other): - if self.order != other.order: - return cmp(self.order, other.order) - elif self.preference != other.preference: - return cmp(self.preference, other.preference) - elif self.flags != other.flags: - return cmp(self.flags, other.flags) - elif self.service != other.service: - return cmp(self.service, other.service) - elif self.regexp != other.regexp: - return cmp(self.regexp, other.regexp) - return cmp(self.replacement, other.replacement) + def __eq__(self, other): + return ((self.order, self.preference, self.flags, self.service, + self.regexp, self.replacement) == + (other.order, other.preference, other.flags, other.service, + other.regexp, other.replacement)) + + def __ne__(self, other): + return ((self.order, self.preference, self.flags, self.service, + self.regexp, self.replacement) != + (other.order, other.preference, other.flags, other.service, + other.regexp, other.replacement)) + + def __lt__(self, other): + return ((self.order, self.preference, self.flags, self.service, + self.regexp, self.replacement) < + (other.order, other.preference, other.flags, other.service, + other.regexp, other.replacement)) + + def __le__(self, other): + return ((self.order, self.preference, self.flags, self.service, + self.regexp, self.replacement) <= + (other.order, other.preference, other.flags, other.service, + other.regexp, other.replacement)) + + def __gt__(self, other): + return ((self.order, self.preference, self.flags, self.service, + self.regexp, self.replacement) > + (other.order, other.preference, other.flags, other.service, + other.regexp, other.replacement)) + + def __ge__(self, other): + return ((self.order, self.preference, self.flags, self.service, + self.regexp, self.replacement) >= + (other.order, other.preference, other.flags, other.service, + other.regexp, other.replacement)) def __repr__(self): flags = self.flags if self.flags is not None else '' @@ -1057,12 +1145,29 @@ class SshfpValue(object): 'fingerprint': self.fingerprint, } - def __cmp__(self, other): - if self.algorithm != other.algorithm: - return cmp(self.algorithm, other.algorithm) - elif self.fingerprint_type != other.fingerprint_type: - return cmp(self.fingerprint_type, other.fingerprint_type) - return cmp(self.fingerprint, other.fingerprint) + def __eq__(self, other): + return ((self.algorithm, self.fingerprint_type, self.fingerprint) == + (other.algorithm, other.fingerprint_type, other.fingerprint)) + + def __ne__(self, other): + return ((self.algorithm, self.fingerprint_type, self.fingerprint) != + (other.algorithm, other.fingerprint_type, other.fingerprint)) + + def __lt__(self, other): + return ((self.algorithm, self.fingerprint_type, self.fingerprint) < + (other.algorithm, other.fingerprint_type, other.fingerprint)) + + def __le__(self, other): + return ((self.algorithm, self.fingerprint_type, self.fingerprint) <= + (other.algorithm, other.fingerprint_type, other.fingerprint)) + + def __gt__(self, other): + return ((self.algorithm, self.fingerprint_type, self.fingerprint) > + (other.algorithm, other.fingerprint_type, other.fingerprint)) + + def __ge__(self, other): + return ((self.algorithm, self.fingerprint_type, self.fingerprint) >= + (other.algorithm, other.fingerprint_type, other.fingerprint)) def __repr__(self): return "'{} {} {}'".format(self.algorithm, self.fingerprint_type, @@ -1178,14 +1283,29 @@ class SrvValue(object): 'target': self.target, } - def __cmp__(self, other): - if self.priority != other.priority: - return cmp(self.priority, other.priority) - elif self.weight != other.weight: - return cmp(self.weight, other.weight) - elif self.port != other.port: - return cmp(self.port, other.port) - return cmp(self.target, other.target) + def __eq__(self, other): + return ((self.priority, self.weight, self.port, self.target) == + (other.priority, other.weight, other.port, other.target)) + + def __ne__(self, other): + return ((self.priority, self.weight, self.port, self.target) != + (other.priority, other.weight, other.port, other.target)) + + def __lt__(self, other): + return ((self.priority, self.weight, self.port, self.target) < + (other.priority, other.weight, other.port, other.target)) + + def __le__(self, other): + return ((self.priority, self.weight, self.port, self.target) <= + (other.priority, other.weight, other.port, other.target)) + + def __gt__(self, other): + return ((self.priority, self.weight, self.port, self.target) > + (other.priority, other.weight, other.port, other.target)) + + def __ge__(self, other): + return ((self.priority, self.weight, self.port, self.target) >= + (other.priority, other.weight, other.port, other.target)) def __repr__(self): return "'{} {} {} {}'".format(self.priority, self.weight, self.port, diff --git a/tests/test_octodns_record.py b/tests/test_octodns_record.py index f11d783..0b96fd7 100644 --- a/tests/test_octodns_record.py +++ b/tests/test_octodns_record.py @@ -5,12 +5,14 @@ from __future__ import absolute_import, division, print_function, \ unicode_literals +from six import text_type from unittest import TestCase from octodns.record import ARecord, AaaaRecord, AliasRecord, CaaRecord, \ - CnameRecord, Create, Delete, GeoValue, MxRecord, NaptrRecord, NaptrValue, \ - NsRecord, PtrRecord, Record, SshfpRecord, SpfRecord, SrvRecord, \ - TxtRecord, Update, ValidationError, _Dynamic, _DynamicPool, _DynamicRule + CaaValue, CnameRecord, Create, Delete, GeoValue, MxRecord, MxValue, \ + NaptrRecord, NaptrValue, NsRecord, PtrRecord, Record, SshfpRecord, \ + SshfpValue, SpfRecord, SrvRecord, SrvValue, TxtRecord, Update, \ + ValidationError, _Dynamic, _DynamicPool, _DynamicRule from octodns.zone import Zone from helpers import DynamicProvider, GeoProvider, SimpleProvider @@ -482,109 +484,112 @@ class TestRecord(TestCase): # full sorting # equivalent b_naptr_value = b.values[0] - self.assertEquals(0, b_naptr_value.__cmp__(b_naptr_value)) + self.assertTrue(b_naptr_value == b_naptr_value) + self.assertFalse(b_naptr_value != b_naptr_value) + self.assertTrue(b_naptr_value <= b_naptr_value) + self.assertTrue(b_naptr_value >= b_naptr_value) # by order - self.assertEquals(1, b_naptr_value.__cmp__(NaptrValue({ + self.assertTrue(b_naptr_value > NaptrValue({ 'order': 10, 'preference': 31, 'flags': 'M', 'service': 'N', 'regexp': 'O', 'replacement': 'x', - }))) - self.assertEquals(-1, b_naptr_value.__cmp__(NaptrValue({ + })) + self.assertTrue(b_naptr_value < NaptrValue({ 'order': 40, 'preference': 31, 'flags': 'M', 'service': 'N', 'regexp': 'O', 'replacement': 'x', - }))) + })) # by preference - self.assertEquals(1, b_naptr_value.__cmp__(NaptrValue({ + self.assertTrue(b_naptr_value > NaptrValue({ 'order': 30, 'preference': 10, 'flags': 'M', 'service': 'N', 'regexp': 'O', 'replacement': 'x', - }))) - self.assertEquals(-1, b_naptr_value.__cmp__(NaptrValue({ + })) + self.assertTrue(b_naptr_value < NaptrValue({ 'order': 30, 'preference': 40, 'flags': 'M', 'service': 'N', 'regexp': 'O', 'replacement': 'x', - }))) + })) # by flags - self.assertEquals(1, b_naptr_value.__cmp__(NaptrValue({ + self.assertTrue(b_naptr_value > NaptrValue({ 'order': 30, 'preference': 31, 'flags': 'A', 'service': 'N', 'regexp': 'O', 'replacement': 'x', - }))) - self.assertEquals(-1, b_naptr_value.__cmp__(NaptrValue({ + })) + self.assertTrue(b_naptr_value < NaptrValue({ 'order': 30, 'preference': 31, 'flags': 'Z', 'service': 'N', 'regexp': 'O', 'replacement': 'x', - }))) + })) # by service - self.assertEquals(1, b_naptr_value.__cmp__(NaptrValue({ + self.assertTrue(b_naptr_value > NaptrValue({ 'order': 30, 'preference': 31, 'flags': 'M', 'service': 'A', 'regexp': 'O', 'replacement': 'x', - }))) - self.assertEquals(-1, b_naptr_value.__cmp__(NaptrValue({ + })) + self.assertTrue(b_naptr_value < NaptrValue({ 'order': 30, 'preference': 31, 'flags': 'M', 'service': 'Z', 'regexp': 'O', 'replacement': 'x', - }))) + })) # by regexp - self.assertEquals(1, b_naptr_value.__cmp__(NaptrValue({ + self.assertTrue(b_naptr_value > NaptrValue({ 'order': 30, 'preference': 31, 'flags': 'M', 'service': 'N', 'regexp': 'A', 'replacement': 'x', - }))) - self.assertEquals(-1, b_naptr_value.__cmp__(NaptrValue({ + })) + self.assertTrue(b_naptr_value < NaptrValue({ 'order': 30, 'preference': 31, 'flags': 'M', 'service': 'N', 'regexp': 'Z', 'replacement': 'x', - }))) + })) # by replacement - self.assertEquals(1, b_naptr_value.__cmp__(NaptrValue({ + self.assertTrue(b_naptr_value > NaptrValue({ 'order': 30, 'preference': 31, 'flags': 'M', 'service': 'N', 'regexp': 'O', 'replacement': 'a', - }))) - self.assertEquals(-1, b_naptr_value.__cmp__(NaptrValue({ + })) + self.assertTrue(b_naptr_value < NaptrValue({ 'order': 30, 'preference': 31, 'flags': 'M', 'service': 'N', 'regexp': 'O', 'replacement': 'z', - }))) + })) # __repr__ doesn't blow up a.__repr__() @@ -796,6 +801,38 @@ class TestRecord(TestCase): self.assertEquals(values, geo.values) self.assertEquals(['NA-US', 'NA'], list(geo.parents)) + a = GeoValue('NA-US-CA', values) + b = GeoValue('AP-JP', values) + c = GeoValue('NA-US-CA', ['2.3.4.5']) + + self.assertEqual(a, a) + self.assertEqual(b, b) + self.assertEqual(c, c) + + self.assertNotEqual(a, b) + self.assertNotEqual(a, c) + self.assertNotEqual(b, a) + self.assertNotEqual(b, c) + self.assertNotEqual(c, a) + self.assertNotEqual(c, b) + + self.assertTrue(a > b) + self.assertTrue(a < c) + self.assertTrue(b < a) + self.assertTrue(b < c) + self.assertTrue(c > a) + self.assertTrue(c > b) + + self.assertTrue(a >= a) + self.assertTrue(a >= b) + self.assertTrue(a <= c) + self.assertTrue(b <= a) + self.assertTrue(b <= b) + self.assertTrue(b <= c) + self.assertTrue(c > a) + self.assertTrue(c > b) + self.assertTrue(c >= b) + def test_healthcheck(self): new = Record.new(self.zone, 'a', { 'ttl': 44, @@ -851,6 +888,290 @@ class TestRecord(TestCase): }) self.assertFalse(new.ignored) + def test_ordering_functions(self): + a = Record.new(self.zone, 'a', { + 'ttl': 44, + 'type': 'A', + 'value': '1.2.3.4', + }) + b = Record.new(self.zone, 'b', { + 'ttl': 44, + 'type': 'A', + 'value': '1.2.3.4', + }) + c = Record.new(self.zone, 'c', { + 'ttl': 44, + 'type': 'A', + 'value': '1.2.3.4', + }) + aaaa = Record.new(self.zone, 'a', { + 'ttl': 44, + 'type': 'AAAA', + 'value': '2601:644:500:e210:62f8:1dff:feb8:947a', + }) + + self.assertEquals(a, a) + self.assertEquals(b, b) + self.assertEquals(c, c) + self.assertEquals(aaaa, aaaa) + + self.assertNotEqual(a, b) + self.assertNotEqual(a, c) + self.assertNotEqual(a, aaaa) + self.assertNotEqual(b, a) + self.assertNotEqual(b, c) + self.assertNotEqual(b, aaaa) + self.assertNotEqual(c, a) + self.assertNotEqual(c, b) + self.assertNotEqual(c, aaaa) + self.assertNotEqual(aaaa, a) + self.assertNotEqual(aaaa, b) + self.assertNotEqual(aaaa, c) + + self.assertTrue(a < b) + self.assertTrue(a < c) + self.assertTrue(a < aaaa) + self.assertTrue(b > a) + self.assertTrue(b < c) + self.assertTrue(b > aaaa) + self.assertTrue(c > a) + self.assertTrue(c > b) + self.assertTrue(c > aaaa) + self.assertTrue(aaaa > a) + self.assertTrue(aaaa < b) + self.assertTrue(aaaa < c) + + self.assertTrue(a <= a) + self.assertTrue(a <= b) + self.assertTrue(a <= c) + self.assertTrue(a <= aaaa) + self.assertTrue(b >= a) + self.assertTrue(b >= b) + self.assertTrue(b <= c) + self.assertTrue(b >= aaaa) + self.assertTrue(c >= a) + self.assertTrue(c >= b) + self.assertTrue(c >= c) + self.assertTrue(c >= aaaa) + self.assertTrue(aaaa >= a) + self.assertTrue(aaaa <= b) + self.assertTrue(aaaa <= c) + self.assertTrue(aaaa <= aaaa) + + def test_caa_value(self): + a = CaaValue({'flags': 0, 'tag': 'a', 'value': 'v'}) + b = CaaValue({'flags': 1, 'tag': 'a', 'value': 'v'}) + c = CaaValue({'flags': 0, 'tag': 'c', 'value': 'v'}) + d = CaaValue({'flags': 0, 'tag': 'a', 'value': 'z'}) + + self.assertEqual(a, a) + self.assertEqual(b, b) + self.assertEqual(c, c) + self.assertEqual(d, d) + + self.assertNotEqual(a, b) + self.assertNotEqual(a, c) + self.assertNotEqual(a, d) + self.assertNotEqual(b, a) + self.assertNotEqual(b, c) + self.assertNotEqual(b, d) + self.assertNotEqual(c, a) + self.assertNotEqual(c, b) + self.assertNotEqual(c, d) + + self.assertTrue(a < b) + self.assertTrue(a < c) + self.assertTrue(a < d) + + self.assertTrue(b > a) + self.assertTrue(b > c) + self.assertTrue(b > d) + + self.assertTrue(c > a) + self.assertTrue(c < b) + self.assertTrue(c > d) + + self.assertTrue(d > a) + self.assertTrue(d < b) + self.assertTrue(d < c) + + self.assertTrue(a <= b) + self.assertTrue(a <= c) + self.assertTrue(a <= d) + self.assertTrue(a <= a) + self.assertTrue(a >= a) + + self.assertTrue(b >= a) + self.assertTrue(b >= c) + self.assertTrue(b >= d) + self.assertTrue(b >= b) + self.assertTrue(b <= b) + + self.assertTrue(c >= a) + self.assertTrue(c <= b) + self.assertTrue(c >= d) + self.assertTrue(c >= c) + self.assertTrue(c <= c) + + self.assertTrue(d >= a) + self.assertTrue(d <= b) + self.assertTrue(d <= c) + self.assertTrue(d >= d) + self.assertTrue(d <= d) + + def test_mx_value(self): + a = MxValue({'preference': 0, 'priority': 'a', 'exchange': 'v', + 'value': '1'}) + b = MxValue({'preference': 10, 'priority': 'a', 'exchange': 'v', + 'value': '2'}) + c = MxValue({'preference': 0, 'priority': 'b', 'exchange': 'z', + 'value': '3'}) + + self.assertEqual(a, a) + self.assertEqual(b, b) + self.assertEqual(c, c) + + self.assertNotEqual(a, b) + self.assertNotEqual(a, c) + self.assertNotEqual(b, a) + self.assertNotEqual(b, c) + self.assertNotEqual(c, a) + self.assertNotEqual(c, b) + + self.assertTrue(a < b) + self.assertTrue(a < c) + + self.assertTrue(b > a) + self.assertTrue(b > c) + + self.assertTrue(c > a) + self.assertTrue(c < b) + + self.assertTrue(a <= b) + self.assertTrue(a <= c) + self.assertTrue(a <= a) + self.assertTrue(a >= a) + + self.assertTrue(b >= a) + self.assertTrue(b >= c) + self.assertTrue(b >= b) + self.assertTrue(b <= b) + + self.assertTrue(c >= a) + self.assertTrue(c <= b) + self.assertTrue(c >= c) + self.assertTrue(c <= c) + + def test_sshfp_value(self): + a = SshfpValue({'algorithm': 0, 'fingerprint_type': 0, + 'fingerprint': 'abcd'}) + b = SshfpValue({'algorithm': 1, 'fingerprint_type': 0, + 'fingerprint': 'abcd'}) + c = SshfpValue({'algorithm': 0, 'fingerprint_type': 1, + 'fingerprint': 'abcd'}) + d = SshfpValue({'algorithm': 0, 'fingerprint_type': 0, + 'fingerprint': 'bcde'}) + + self.assertEqual(a, a) + self.assertEqual(b, b) + self.assertEqual(c, c) + self.assertEqual(d, d) + + self.assertNotEqual(a, b) + self.assertNotEqual(a, c) + self.assertNotEqual(a, d) + self.assertNotEqual(b, a) + self.assertNotEqual(b, c) + self.assertNotEqual(b, d) + self.assertNotEqual(c, a) + self.assertNotEqual(c, b) + self.assertNotEqual(c, d) + self.assertNotEqual(d, a) + self.assertNotEqual(d, b) + self.assertNotEqual(d, c) + + self.assertTrue(a < b) + self.assertTrue(a < c) + + self.assertTrue(b > a) + self.assertTrue(b > c) + + self.assertTrue(c > a) + self.assertTrue(c < b) + + self.assertTrue(a <= b) + self.assertTrue(a <= c) + self.assertTrue(a <= a) + self.assertTrue(a >= a) + + self.assertTrue(b >= a) + self.assertTrue(b >= c) + self.assertTrue(b >= b) + self.assertTrue(b <= b) + + self.assertTrue(c >= a) + self.assertTrue(c <= b) + self.assertTrue(c >= c) + self.assertTrue(c <= c) + + def test_srv_value(self): + a = SrvValue({'priority': 0, 'weight': 0, 'port': 0, 'target': 'foo.'}) + b = SrvValue({'priority': 1, 'weight': 0, 'port': 0, 'target': 'foo.'}) + c = SrvValue({'priority': 0, 'weight': 2, 'port': 0, 'target': 'foo.'}) + d = SrvValue({'priority': 0, 'weight': 0, 'port': 3, 'target': 'foo.'}) + e = SrvValue({'priority': 0, 'weight': 0, 'port': 0, 'target': 'mmm.'}) + + self.assertEqual(a, a) + self.assertEqual(b, b) + self.assertEqual(c, c) + self.assertEqual(d, d) + self.assertEqual(e, e) + + self.assertNotEqual(a, b) + self.assertNotEqual(a, c) + self.assertNotEqual(a, d) + self.assertNotEqual(a, e) + self.assertNotEqual(b, a) + self.assertNotEqual(b, c) + self.assertNotEqual(b, d) + self.assertNotEqual(b, e) + self.assertNotEqual(c, a) + self.assertNotEqual(c, b) + self.assertNotEqual(c, d) + self.assertNotEqual(c, e) + self.assertNotEqual(d, a) + self.assertNotEqual(d, b) + self.assertNotEqual(d, c) + self.assertNotEqual(d, e) + self.assertNotEqual(e, a) + self.assertNotEqual(e, b) + self.assertNotEqual(e, c) + self.assertNotEqual(e, d) + + self.assertTrue(a < b) + self.assertTrue(a < c) + + self.assertTrue(b > a) + self.assertTrue(b > c) + + self.assertTrue(c > a) + self.assertTrue(c < b) + + self.assertTrue(a <= b) + self.assertTrue(a <= c) + self.assertTrue(a <= a) + self.assertTrue(a >= a) + + self.assertTrue(b >= a) + self.assertTrue(b >= c) + self.assertTrue(b >= b) + self.assertTrue(b <= b) + + self.assertTrue(c >= a) + self.assertTrue(c <= b) + self.assertTrue(c >= c) + self.assertTrue(c <= c) + class TestRecordValidation(TestCase): zone = Zone('unit.tests.', []) From ce67824015ae25914e185309931c0346dcf7c1bd Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Mon, 29 Jul 2019 08:48:17 -0700 Subject: [PATCH 08/49] Handle python3 sourcing of urlparse --- tests/test_octodns_provider_rackspace.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/test_octodns_provider_rackspace.py b/tests/test_octodns_provider_rackspace.py index fbd7dac..b257166 100644 --- a/tests/test_octodns_provider_rackspace.py +++ b/tests/test_octodns_provider_rackspace.py @@ -9,7 +9,10 @@ import json import re from six import text_type from unittest import TestCase -from urlparse import urlparse +try: + from urlparse import urlparse +except ImportError: + from urllib.parse import urlparse from requests import HTTPError from requests_mock import ANY, mock as requests_mock From db77d5b3bbd89adf76a80e37c7d5d65c8cc3056e Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Sat, 5 Oct 2019 07:58:38 -0700 Subject: [PATCH 09/49] python3 compat for azure provider --- octodns/provider/azuredns.py | 10 +++++++--- requirements.txt | 6 +++--- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/octodns/provider/azuredns.py b/octodns/provider/azuredns.py index 0bca46d..3d8122a 100644 --- a/octodns/provider/azuredns.py +++ b/octodns/provider/azuredns.py @@ -175,6 +175,10 @@ class _AzureRecord(object): :type return: bool ''' + + def key_dict(d): + return sum([hash('{}:{}'.format(k, v)) for k, v in d.items()]) + def parse_dict(params): vals = [] for char in params: @@ -185,7 +189,7 @@ class _AzureRecord(object): vals.append(record.__dict__) except: vals.append(list_records.__dict__) - vals.sort() + vals.sort(key=key_dict) return vals return (self.resource_group == b.resource_group) & \ @@ -373,13 +377,13 @@ class AzureProvider(BaseProvider): self._populate_zones() self._check_zone(zone_name) - _records = set() + _records = [] records = self._dns_client.record_sets.list_by_dns_zone if self._check_zone(zone_name): exists = True for azrecord in records(self._resource_group, zone_name): if _parse_azure_type(azrecord.type) in self.SUPPORTS: - _records.add(azrecord) + _records.append(azrecord) for azrecord in _records: record_name = azrecord.name if azrecord.name != '@' else '' typ = _parse_azure_type(azrecord.type) diff --git a/requirements.txt b/requirements.txt index 765b65c..19d45a8 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,6 +1,6 @@ PyYaml==4.2b1 -azure-common==1.1.18 -azure-mgmt-dns==2.1.0 +azure-common==1.1.23 +azure-mgmt-dns==3.0.0 boto3==1.7.5 botocore==1.10.5 dnspython==1.15.0 @@ -13,7 +13,7 @@ google-cloud-dns==0.29.0 incf.countryutils==1.0 ipaddress==1.0.22 jmespath==0.9.3 -msrestazure==0.6.0 +msrestazure==0.6.2 natsort==5.5.0 nsone==0.9.100 ovh==0.4.8 From 470dd822026655eba24d213ab9d54946b1a7f936 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Sat, 5 Oct 2019 08:03:43 -0700 Subject: [PATCH 10/49] python 3 support for constellix provider --- octodns/provider/constellix.py | 3 ++- tests/test_octodns_provider_constellix.py | 10 ++++++++-- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/octodns/provider/constellix.py b/octodns/provider/constellix.py index 939284d..050f120 100644 --- a/octodns/provider/constellix.py +++ b/octodns/provider/constellix.py @@ -9,6 +9,7 @@ from collections import defaultdict from requests import Session from base64 import b64encode from ipaddress import ip_address +from six import string_types import hashlib import hmac import logging @@ -122,7 +123,7 @@ class ConstellixClient(object): # change relative values to absolute value = record['value'] if record['type'] in ['ALIAS', 'CNAME', 'MX', 'NS', 'SRV']: - if isinstance(value, unicode): + if isinstance(value, string_types): record['value'] = self._absolutize_value(value, zone_name) if isinstance(value, list): diff --git a/tests/test_octodns_provider_constellix.py b/tests/test_octodns_provider_constellix.py index 346bb17..7914c53 100644 --- a/tests/test_octodns_provider_constellix.py +++ b/tests/test_octodns_provider_constellix.py @@ -10,6 +10,7 @@ from mock import Mock, call from os.path import dirname, join from requests import HTTPError from requests_mock import ANY, mock as requests_mock +from six import text_type from unittest import TestCase from octodns.record import Record @@ -65,7 +66,7 @@ class TestConstellixProvider(TestCase): with self.assertRaises(Exception) as ctx: zone = Zone('unit.tests.', []) provider.populate(zone) - self.assertEquals('Unauthorized', ctx.exception.message) + self.assertEquals('Unauthorized', text_type(ctx.exception)) # Bad request with requests_mock() as mock: @@ -77,7 +78,7 @@ class TestConstellixProvider(TestCase): zone = Zone('unit.tests.', []) provider.populate(zone) self.assertEquals('\n - "unittests" is not a valid domain name', - ctx.exception.message) + text_type(ctx.exception)) # General error with requests_mock() as mock: @@ -148,6 +149,11 @@ class TestConstellixProvider(TestCase): call('POST', '/', data={'names': ['unit.tests']}), # get all domains to build the cache call('GET', '/'), + ]) + # These two checks are broken up so that ordering doesn't break things. + # Python3 doesn't make the calls in a consistent order so different + # things follow the GET / on different runs + provider._client._request.assert_has_calls([ call('POST', '/123123/records/SRV', data={ 'roundRobin': [{ 'priority': 10, From 742305c20b62b97d354dc0336b0f87dca4a1de1d Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Sat, 5 Oct 2019 08:47:12 -0700 Subject: [PATCH 11/49] six.moves.urllib.parse --- octodns/provider/fastdns.py | 2 +- tests/test_octodns_provider_rackspace.py | 5 +---- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/octodns/provider/fastdns.py b/octodns/provider/fastdns.py index f851303..8f651f0 100644 --- a/octodns/provider/fastdns.py +++ b/octodns/provider/fastdns.py @@ -7,7 +7,7 @@ from __future__ import absolute_import, division, print_function, \ from requests import Session from akamai.edgegrid import EdgeGridAuth -from urlparse import urljoin +from six.moves.urllib.parse import urljoin from collections import defaultdict from logging import getLogger diff --git a/tests/test_octodns_provider_rackspace.py b/tests/test_octodns_provider_rackspace.py index b257166..b0dcad7 100644 --- a/tests/test_octodns_provider_rackspace.py +++ b/tests/test_octodns_provider_rackspace.py @@ -8,11 +8,8 @@ from __future__ import absolute_import, division, print_function, \ import json import re from six import text_type +from six.moves.urllib.parse import urlparse from unittest import TestCase -try: - from urlparse import urlparse -except ImportError: - from urllib.parse import urlparse from requests import HTTPError from requests_mock import ANY, mock as requests_mock From 47199fdfab404dc762d04c9733ccdd9dbb342933 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Sat, 5 Oct 2019 08:49:19 -0700 Subject: [PATCH 12/49] FastDNS python3 --- tests/test_octodns_provider_fastdns.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/test_octodns_provider_fastdns.py b/tests/test_octodns_provider_fastdns.py index 5f503c7..a8bed74 100644 --- a/tests/test_octodns_provider_fastdns.py +++ b/tests/test_octodns_provider_fastdns.py @@ -9,6 +9,7 @@ from __future__ import absolute_import, division, print_function, \ from os.path import dirname, join from requests import HTTPError from requests_mock import ANY, mock as requests_mock +from six import text_type from unittest import TestCase from octodns.record import Record @@ -147,4 +148,4 @@ class TestFastdnsProvider(TestCase): changes = provider.apply(plan) except NameError as e: expected = "contractId not specified to create zone" - self.assertEquals(e.message, expected) + self.assertEquals(text_type(e), expected) From 00781985008b2e1b122160e568b2818deb960cb8 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Sat, 5 Oct 2019 08:57:14 -0700 Subject: [PATCH 13/49] GoogleCloud python3 --- tests/test_octodns_provider_googlecloud.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/tests/test_octodns_provider_googlecloud.py b/tests/test_octodns_provider_googlecloud.py index d7f0e0c..e642668 100644 --- a/tests/test_octodns_provider_googlecloud.py +++ b/tests/test_octodns_provider_googlecloud.py @@ -193,9 +193,14 @@ class DummyIterator: def __iter__(self): return self + # python2 def next(self): return next(self.iterable) + # python3 + def __next__(self): + return next(self.iterable) + class TestGoogleCloudProvider(TestCase): @patch('octodns.provider.googlecloud.dns') @@ -247,7 +252,7 @@ class TestGoogleCloudProvider(TestCase): return_values_for_status = iter( ["pending"] * 11 + ['done', 'done']) type(status_mock).status = PropertyMock( - side_effect=return_values_for_status.next) + side_effect=lambda: next(return_values_for_status)) gcloud_zone_mock.changes = Mock(return_value=status_mock) provider = self._get_provider() From 484a5118f438f8e00ce29027e6b2af167aae1eb5 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Sat, 5 Oct 2019 09:10:59 -0700 Subject: [PATCH 14/49] MythicBeastsProvider python3 --- octodns/provider/mythicbeasts.py | 2 +- tests/test_octodns_provider_mythicbeasts.py | 12 ++++++------ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/octodns/provider/mythicbeasts.py b/octodns/provider/mythicbeasts.py index 17029db..b255a74 100644 --- a/octodns/provider/mythicbeasts.py +++ b/octodns/provider/mythicbeasts.py @@ -328,7 +328,7 @@ class MythicBeastsProvider(BaseProvider): exists = True for line in resp.content.splitlines(): - match = MythicBeastsProvider.RE_POPLINE.match(line) + match = MythicBeastsProvider.RE_POPLINE.match(line.decode("utf-8")) if match is None: self.log.debug('failed to match line: %s', line) diff --git a/tests/test_octodns_provider_mythicbeasts.py b/tests/test_octodns_provider_mythicbeasts.py index b93d46e..498b408 100644 --- a/tests/test_octodns_provider_mythicbeasts.py +++ b/tests/test_octodns_provider_mythicbeasts.py @@ -441,11 +441,11 @@ class TestMythicBeastsProvider(TestCase): plan = provider.plan(wanted) # Octo ignores NS records (15-1) - self.assertEquals(1, len(filter(lambda u: isinstance(u, Update), - plan.changes))) - self.assertEquals(1, len(filter(lambda d: isinstance(d, Delete), - plan.changes))) - self.assertEquals(14, len(filter(lambda c: isinstance(c, Create), - plan.changes))) + self.assertEquals(1, len(list(filter( + lambda u: isinstance(u, Update), plan.changes)))) + self.assertEquals(1, len(list(filter( + lambda d: isinstance(d, Delete), plan.changes)))) + self.assertEquals(14, len(list(filter( + lambda c: isinstance(c, Create), plan.changes)))) self.assertEquals(16, provider.apply(plan)) self.assertTrue(plan.exists) From 37543e6a762f94687a219134c6c9f75b900c93a8 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Sat, 5 Oct 2019 09:18:42 -0700 Subject: [PATCH 15/49] OvhProvider python3 --- octodns/provider/ovh.py | 2 +- tests/test_octodns_provider_ovh.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/octodns/provider/ovh.py b/octodns/provider/ovh.py index 0187098..a0e47f8 100644 --- a/octodns/provider/ovh.py +++ b/octodns/provider/ovh.py @@ -345,7 +345,7 @@ class OvhProvider(BaseProvider): @staticmethod def _is_valid_dkim_key(key): try: - base64.decodestring(key) + base64.decodestring(bytearray(key, 'utf-8')) except binascii.Error: return False return True diff --git a/tests/test_octodns_provider_ovh.py b/tests/test_octodns_provider_ovh.py index d3f468d..1f41abf 100644 --- a/tests/test_octodns_provider_ovh.py +++ b/tests/test_octodns_provider_ovh.py @@ -428,7 +428,7 @@ class TestOvhProvider(TestCase): ), call(u'/domain/zone/unit.tests/refresh')] - post_mock.assert_has_calls(wanted_calls) + post_mock.assert_has_calls(wanted_calls, any_order=True) # Get for delete calls wanted_get_calls = [ @@ -440,7 +440,7 @@ class TestOvhProvider(TestCase): subDomain=u''), call(u'/domain/zone/unit.tests/record', fieldType=u'A', subDomain='fake')] - get_mock.assert_has_calls(wanted_get_calls) + get_mock.assert_has_calls(wanted_get_calls, any_order=True) # 4 delete calls for update and delete delete_mock.assert_has_calls( [call(u'/domain/zone/unit.tests/record/100'), From 0acff67faa4c38ca36a13f164ed6903c504a64fc Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Sat, 5 Oct 2019 14:38:58 -0700 Subject: [PATCH 16/49] Ns1Provider python3 --- octodns/provider/ns1.py | 5 ++--- tests/test_octodns_provider_ns1.py | 13 +++++++++---- 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/octodns/provider/ns1.py b/octodns/provider/ns1.py index d3faf21..b4fd850 100644 --- a/octodns/provider/ns1.py +++ b/octodns/provider/ns1.py @@ -10,7 +10,7 @@ from itertools import chain from collections import OrderedDict, defaultdict from nsone import NSONE from nsone.rest.errors import RateLimitException, ResourceException -from incf.countryutils import transformations +from pycountry_convert import country_alpha2_to_continent_code from time import sleep from six import text_type @@ -62,8 +62,7 @@ class Ns1Provider(BaseProvider): us_state = meta.get('us_state', []) ca_province = meta.get('ca_province', []) for cntry in country: - cn = transformations.cc_to_cn(cntry) - con = transformations.cn_to_ctca2(cn) + con = country_alpha2_to_continent_code(cntry) key = '{}-{}'.format(con, cntry) geo[key].extend(answer['answer']) for state in us_state: diff --git a/tests/test_octodns_provider_ns1.py b/tests/test_octodns_provider_ns1.py index 178ce53..91d1a3f 100644 --- a/tests/test_octodns_provider_ns1.py +++ b/tests/test_octodns_provider_ns1.py @@ -5,6 +5,7 @@ from __future__ import absolute_import, division, print_function, \ unicode_literals +from collections import defaultdict from mock import Mock, call, patch from nsone.rest.errors import AuthException, RateLimitException, \ ResourceException @@ -373,8 +374,12 @@ class TestNs1Provider(TestCase): load_mock.side_effect = [nsone_zone, nsone_zone] plan = provider.plan(desired) self.assertEquals(3, len(plan.changes)) - self.assertIsInstance(plan.changes[0], Update) - self.assertIsInstance(plan.changes[2], Delete) + # Shouldn't rely on order so just count classes + classes = defaultdict(lambda: 0) + for change in plan.changes: + classes[change.__class__] += 1 + self.assertEquals(1, classes[Delete]) + self.assertEquals(2, classes[Update]) # ugh, we need a mock record that can be returned from loadRecord for # the update and delete targets, we can add our side effects to that to # trigger rate limit handling @@ -397,7 +402,7 @@ class TestNs1Provider(TestCase): call('unit.tests', u'A'), call('geo', u'A'), call('delete-me', u'A'), - ]) + ], any_order=True) mock_record.assert_has_calls([ call.update(answers=[{'answer': [u'1.2.3.4'], 'meta': {}}], filters=[], @@ -424,7 +429,7 @@ class TestNs1Provider(TestCase): ttl=34), call.delete(), call.delete() - ]) + ], any_order=True) def test_escaping(self): provider = Ns1Provider('test', 'api-key') From aeb70b24888cd448e8dcf073b391d7c649ab1b46 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Sat, 5 Oct 2019 20:01:53 -0700 Subject: [PATCH 17/49] Route53Provider python 3, rm incf.countryutils, lots of cmp removal and ordering fixes --- CHANGELOG.md | 6 + octodns/provider/route53.py | 106 ++++++--- requirements.txt | 1 - tests/test_octodns_provider_rackspace.py | 1 - tests/test_octodns_provider_route53.py | 268 +++++++++++++---------- 5 files changed, 241 insertions(+), 141 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 76ff8b0..e30d0df 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,9 @@ +## v0.9.9 - 2019-??-?? - Python 3.7 Support + +* Route53 _mod_keyer ordering wasn't complete/reliable and in python 3 this + resulted in randomness. This has been addressed and may result in value + reordering on next plan, no actual changes in behavior should occur. + ## v0.9.8 - 2019-09-30 - One with no changes b/c PyPi description problems * No material changes diff --git a/octodns/provider/route53.py b/octodns/provider/route53.py index 0bc7a99..6d2cfeb 100644 --- a/octodns/provider/route53.py +++ b/octodns/provider/route53.py @@ -8,8 +8,8 @@ from __future__ import absolute_import, division, print_function, \ from boto3 import client from botocore.config import Config from collections import defaultdict -from incf.countryutils.transformations import cca_to_ctca2 from ipaddress import AddressValueError, ip_address +from pycountry_convert import country_alpha2_to_continent_code from uuid import uuid4 import logging import re @@ -20,13 +20,6 @@ from ..record import Record, Update from ..record.geo import GeoCodes from .base import BaseProvider -# TODO: remove when Python 2.x is no longer supported -try: # pragma: no cover - cmp -except NameError: # pragma: no cover - def cmp(x, y): - return (x > y) - (x < y) - octal_re = re.compile(r'\\(\d\d\d)') @@ -155,7 +148,7 @@ class _Route53Record(object): } } - # NOTE: we're using __hash__ and __cmp__ methods that consider + # NOTE: we're using __hash__ and ordering methods that consider # _Route53Records equivalent if they have the same class, fqdn, and _type. # Values are ignored. This is useful when computing diffs/changes. @@ -163,17 +156,34 @@ class _Route53Record(object): 'sub-classes should never use this method' return '{}:{}'.format(self.fqdn, self._type).__hash__() - def __cmp__(self, other): - '''sub-classes should call up to this and return its value if non-zero. - When it's zero they should compute their own __cmp__''' - if self.__class__ != other.__class__: - return cmp(self.__class__, other.__class__) - elif self.fqdn != other.fqdn: - return cmp(self.fqdn, other.fqdn) - elif self._type != other._type: - return cmp(self._type, other._type) - # We're ignoring ttl, it's not an actual differentiator - return 0 + def __eq__(self, other): + '''Sub-classes should call up to this and return its value if true. + When it's false they should compute their own __eq__, same for other + ordering methods.''' + return self.__class__.__name__ == other.__class__.__name__ and \ + self.fqdn == other.fqdn and \ + self._type == other._type + + def __ne__(self, other): + return self.__class__.__name__ != other.__class__.__name__ or \ + self.fqdn != other.fqdn or \ + self._type != other._type + + def __lt__(self, other): + return (((self.__class__.__name__, self.fqdn, self._type)) < + ((other.__class__.__name__, other.fqdn, other._type))) + + def __le__(self, other): + return (((self.__class__.__name__, self.fqdn, self._type)) <= + ((other.__class__.__name__, other.fqdn, other._type))) + + def __gt__(self, other): + return (((self.__class__.__name__, self.fqdn, self._type)) > + ((other.__class__.__name__, other.fqdn, other._type))) + + def __ge__(self, other): + return (((self.__class__.__name__, self.fqdn, self._type)) >= + ((other.__class__.__name__, other.fqdn, other._type))) def __repr__(self): return '_Route53Record<{} {} {} {}>'.format(self.fqdn, self._type, @@ -514,11 +524,50 @@ class _Route53GeoRecord(_Route53Record): return '{}:{}:{}'.format(self.fqdn, self._type, self.geo.code).__hash__() - def __cmp__(self, other): - ret = super(_Route53GeoRecord, self).__cmp__(other) - if ret != 0: - return ret - return cmp(self.geo.code, other.geo.code) + def __eq__(self, other): + return super(_Route53GeoRecord, self).__eq__(other) and \ + self.geo.code == other.geo.code + + def __ne__(self, other): + # super will handle class != class, so if it's true we have 2 geo + # objects with the same name and type, so just need to compare codes + return super(_Route53GeoRecord, self).__ne__(other) or \ + self.geo.code != other.geo.code + + def __lt__(self, other): + # super eq will check class, name, and type + if super(_Route53GeoRecord, self).__eq__(other): + # if it's True we're dealing with two geo's with the same name and + # type, so we just need to compare codes + return self.geo.code < other.geo.code + # Super is not equal so we'll just let it decide lt + return super(_Route53GeoRecord, self).__lt__(other) + + def __le__(self, other): + # super eq will check class, name, and type + if super(_Route53GeoRecord, self).__eq__(other): + # Just need to compare codes, everything else is equal + return self.geo.code <= other.geo.code + # Super is not equal so geo.code doesn't matter, let it decide with lt, + # can't be eq + return super(_Route53GeoRecord, self).__lt__(other) + + def __gt__(self, other): + # super eq will check class, name, and type + if super(_Route53GeoRecord, self).__eq__(other): + # Just need to compare codes, everything else is equal + return self.geo.code > other.geo.code + # Super is not equal so we'll just let it decide gt + return super(_Route53GeoRecord, self).__gt__(other) + + def __ge__(self, other): + # super eq will check class, name, and type + if super(_Route53GeoRecord, self).__eq__(other): + # Just need to compare codes, everything else is equal + return self.geo.code >= other.geo.code + # Super is not equal so geo.code doesn't matter, let it decide with gt, + # can't be eq + return super(_Route53GeoRecord, self).__gt__(other) def __repr__(self): return '_Route53GeoRecord<{} {} {} {} {}>'.format(self.fqdn, @@ -561,7 +610,10 @@ def _mod_keyer(mod): if rrset.get('GeoLocation', False): unique_id = rrset['SetIdentifier'] else: - unique_id = rrset['Name'] + try: + unique_id = '{}-{}'.format(rrset['Name'], rrset['SetIdentifier']) + except KeyError: + unique_id = rrset['Name'] # Prioritise within the action_priority, ensuring targets come first. if rrset.get('GeoLocation', False): @@ -708,7 +760,7 @@ class Route53Provider(BaseProvider): if cc == '*': # This is the default return - cn = cca_to_ctca2(cc) + cn = country_alpha2_to_continent_code(cc) try: return '{}-{}-{}'.format(cn, cc, loc['SubdivisionCode']) except KeyError: diff --git a/requirements.txt b/requirements.txt index 19d45a8..6b6af09 100644 --- a/requirements.txt +++ b/requirements.txt @@ -10,7 +10,6 @@ futures==3.2.0; python_version < '3.0' edgegrid-python==1.1.1 google-cloud-core==0.28.1 google-cloud-dns==0.29.0 -incf.countryutils==1.0 ipaddress==1.0.22 jmespath==0.9.3 msrestazure==0.6.2 diff --git a/tests/test_octodns_provider_rackspace.py b/tests/test_octodns_provider_rackspace.py index b0dcad7..3dfdd5f 100644 --- a/tests/test_octodns_provider_rackspace.py +++ b/tests/test_octodns_provider_rackspace.py @@ -40,7 +40,6 @@ with open('./tests/fixtures/rackspace-sample-recordset-page2.json') as fh: class TestRackspaceProvider(TestCase): def setUp(self): - self.maxDiff = 1000 with requests_mock() as mock: mock.post(ANY, status_code=200, text=AUTH_RESPONSE) self.provider = RackspaceProvider('identity', 'test', 'api-key', diff --git a/tests/test_octodns_provider_route53.py b/tests/test_octodns_provider_route53.py index 87dfd09..b0ee342 100644 --- a/tests/test_octodns_provider_route53.py +++ b/tests/test_octodns_provider_route53.py @@ -2091,6 +2091,58 @@ class TestRoute53Records(TestCase): e.__repr__() f.__repr__() + def test_route53_record_ordering(self): + # Matches + a = _Route53Record(None, self.record_a, False) + b = _Route53Record(None, self.record_a, False) + self.assertTrue(a == b) + self.assertFalse(a != b) + self.assertFalse(a < b) + self.assertTrue(a <= b) + self.assertFalse(a > b) + self.assertTrue(a >= b) + + # Change the fqdn is greater + fqdn = _Route53Record(None, self.record_a, False, + fqdn_override='other') + self.assertFalse(a == fqdn) + self.assertTrue(a != fqdn) + self.assertFalse(a < fqdn) + self.assertFalse(a <= fqdn) + self.assertTrue(a > fqdn) + self.assertTrue(a >= fqdn) + + provider = DummyProvider() + geo_a = _Route53GeoRecord(provider, self.record_a, 'NA-US', + self.record_a.geo['NA-US'], False) + geo_b = _Route53GeoRecord(provider, self.record_a, 'NA-US', + self.record_a.geo['NA-US'], False) + self.assertTrue(geo_a == geo_b) + self.assertFalse(geo_a != geo_b) + self.assertFalse(geo_a < geo_b) + self.assertTrue(geo_a <= geo_b) + self.assertFalse(geo_a > geo_b) + self.assertTrue(geo_a >= geo_b) + + # Other base + geo_fqdn = _Route53GeoRecord(provider, self.record_a, 'NA-US', + self.record_a.geo['NA-US'], False) + geo_fqdn.fqdn = 'other' + self.assertFalse(geo_a == geo_fqdn) + self.assertTrue(geo_a != geo_fqdn) + self.assertFalse(geo_a < geo_fqdn) + self.assertFalse(geo_a <= geo_fqdn) + self.assertTrue(geo_a > geo_fqdn) + self.assertTrue(geo_a >= geo_fqdn) + + # Other class + self.assertFalse(a == geo_a) + self.assertTrue(a != geo_a) + self.assertFalse(a < geo_a) + self.assertFalse(a <= geo_a) + self.assertTrue(a > geo_a) + self.assertTrue(a >= geo_a) + def test_dynamic_value_delete(self): provider = DummyProvider() geo = _Route53DynamicValue(provider, self.record_a, 'iad', '2.2.2.2', @@ -2207,70 +2259,112 @@ class TestRoute53Records(TestCase): creating=True) self.assertEquals(18, len(route53_records)) + expected_mods = [r.mod('CREATE', []) for r in route53_records] + # Sort so that we get a consistent order and don't rely on set ordering + expected_mods.sort(key=_mod_keyer) + # Convert the route53_records into mods self.assertEquals([{ 'Action': 'CREATE', 'ResourceRecordSet': { 'HealthCheckId': 'hc42', 'Name': '_octodns-ap-southeast-1-value.unit.tests.', - 'ResourceRecords': [{ - 'Value': '1.4.1.2'}], - 'SetIdentifier': 'ap-southeast-1-001', + 'ResourceRecords': [{'Value': '1.4.1.1'}], + 'SetIdentifier': 'ap-southeast-1-000', 'TTL': 60, 'Type': 'A', - 'Weight': 2 - } + 'Weight': 2} }, { 'Action': 'CREATE', 'ResourceRecordSet': { 'HealthCheckId': 'hc42', 'Name': '_octodns-ap-southeast-1-value.unit.tests.', - 'ResourceRecords': [{ - 'Value': '1.4.1.1'}], - 'SetIdentifier': 'ap-southeast-1-000', + 'ResourceRecords': [{'Value': '1.4.1.2'}], + 'SetIdentifier': 'ap-southeast-1-001', 'TTL': 60, 'Type': 'A', - 'Weight': 2 - } + 'Weight': 2} + }, { + 'Action': 'CREATE', + 'ResourceRecordSet': { + 'Name': '_octodns-default-pool.unit.tests.', + 'ResourceRecords': [ + {'Value': '1.1.2.1'}, + {'Value': '1.1.2.2'}], + 'TTL': 60, + 'Type': 'A'} + }, { + 'Action': 'CREATE', + 'ResourceRecordSet': { + 'HealthCheckId': 'hc42', + 'Name': '_octodns-eu-central-1-value.unit.tests.', + 'ResourceRecords': [{'Value': '1.3.1.1'}], + 'SetIdentifier': 'eu-central-1-000', + 'TTL': 60, + 'Type': 'A', + 'Weight': 1} + }, { + 'Action': 'CREATE', + 'ResourceRecordSet': { + 'HealthCheckId': 'hc42', + 'Name': '_octodns-eu-central-1-value.unit.tests.', + 'ResourceRecords': [{'Value': '1.3.1.2'}], + 'SetIdentifier': 'eu-central-1-001', + 'TTL': 60, + 'Type': 'A', + 'Weight': 1} + }, { + 'Action': 'CREATE', + 'ResourceRecordSet': { + 'HealthCheckId': 'hc42', + 'Name': '_octodns-us-east-1-value.unit.tests.', + 'ResourceRecords': [{'Value': '1.5.1.1'}], + 'SetIdentifier': 'us-east-1-000', + 'TTL': 60, + 'Type': 'A', + 'Weight': 1} + }, { + 'Action': 'CREATE', + 'ResourceRecordSet': { + 'HealthCheckId': 'hc42', + 'Name': '_octodns-us-east-1-value.unit.tests.', + 'ResourceRecords': [{'Value': '1.5.1.2'}], + 'SetIdentifier': 'us-east-1-001', + 'TTL': 60, + 'Type': 'A', + 'Weight': 1} }, { 'Action': 'CREATE', 'ResourceRecordSet': { 'AliasTarget': { - 'DNSName': '_octodns-ap-southeast-1-pool.unit.tests.', + 'DNSName': '_octodns-ap-southeast-1-value.unit.tests.', 'EvaluateTargetHealth': True, - 'HostedZoneId': 'z45' - }, - 'GeoLocation': { - 'CountryCode': 'JP'}, - 'Name': 'unit.tests.', - 'SetIdentifier': '0-ap-southeast-1-AS-JP', - 'Type': 'A' - } + 'HostedZoneId': 'z45'}, + 'Failover': 'PRIMARY', + 'Name': '_octodns-ap-southeast-1-pool.unit.tests.', + 'SetIdentifier': 'ap-southeast-1-Primary', + 'Type': 'A'} }, { 'Action': 'CREATE', 'ResourceRecordSet': { 'AliasTarget': { - 'DNSName': '_octodns-eu-central-1-pool.unit.tests.', + 'DNSName': '_octodns-eu-central-1-value.unit.tests.', 'EvaluateTargetHealth': True, 'HostedZoneId': 'z45'}, - 'GeoLocation': { - 'CountryCode': 'US', - 'SubdivisionCode': 'FL', - }, - 'Name': 'unit.tests.', - 'SetIdentifier': '1-eu-central-1-NA-US-FL', + 'Failover': 'PRIMARY', + 'Name': '_octodns-eu-central-1-pool.unit.tests.', + 'SetIdentifier': 'eu-central-1-Primary', 'Type': 'A'} }, { 'Action': 'CREATE', 'ResourceRecordSet': { 'AliasTarget': { - 'DNSName': '_octodns-us-east-1-pool.unit.tests.', + 'DNSName': '_octodns-us-east-1-value.unit.tests.', 'EvaluateTargetHealth': True, 'HostedZoneId': 'z45'}, - 'GeoLocation': { - 'CountryCode': '*'}, - 'Name': 'unit.tests.', - 'SetIdentifier': '2-us-east-1-None', + 'Failover': 'PRIMARY', + 'Name': '_octodns-us-east-1-pool.unit.tests.', + 'SetIdentifier': 'us-east-1-Primary', 'Type': 'A'} }, { 'Action': 'CREATE', @@ -2287,123 +2381,72 @@ class TestRoute53Records(TestCase): 'Action': 'CREATE', 'ResourceRecordSet': { 'AliasTarget': { - 'DNSName': '_octodns-ap-southeast-1-pool.unit.tests.', + 'DNSName': '_octodns-us-east-1-pool.unit.tests.', 'EvaluateTargetHealth': True, 'HostedZoneId': 'z45'}, - 'GeoLocation': { - 'CountryCode': 'CN'}, - 'Name': 'unit.tests.', - 'SetIdentifier': '0-ap-southeast-1-AS-CN', + 'Failover': 'SECONDARY', + 'Name': '_octodns-eu-central-1-pool.unit.tests.', + 'SetIdentifier': 'eu-central-1-Secondary-us-east-1', 'Type': 'A'} }, { 'Action': 'CREATE', 'ResourceRecordSet': { 'AliasTarget': { - 'DNSName': '_octodns-us-east-1-value.unit.tests.', + 'DNSName': '_octodns-default-pool.unit.tests.', 'EvaluateTargetHealth': True, 'HostedZoneId': 'z45'}, - 'Failover': 'PRIMARY', + 'Failover': 'SECONDARY', 'Name': '_octodns-us-east-1-pool.unit.tests.', - 'SetIdentifier': 'us-east-1-Primary', + 'SetIdentifier': 'us-east-1-Secondary-default', 'Type': 'A'} }, { 'Action': 'CREATE', 'ResourceRecordSet': { 'AliasTarget': { - 'DNSName': '_octodns-eu-central-1-pool.unit.tests.', + 'DNSName': '_octodns-ap-southeast-1-pool.unit.tests.', 'EvaluateTargetHealth': True, 'HostedZoneId': 'z45'}, 'GeoLocation': { - 'ContinentCode': 'EU'}, + 'CountryCode': 'CN'}, 'Name': 'unit.tests.', - 'SetIdentifier': '1-eu-central-1-EU', + 'SetIdentifier': '0-ap-southeast-1-AS-CN', 'Type': 'A'} }, { 'Action': 'CREATE', 'ResourceRecordSet': { 'AliasTarget': { - 'DNSName': '_octodns-eu-central-1-value.unit.tests.', + 'DNSName': '_octodns-ap-southeast-1-pool.unit.tests.', 'EvaluateTargetHealth': True, 'HostedZoneId': 'z45'}, - 'Failover': 'PRIMARY', - 'Name': '_octodns-eu-central-1-pool.unit.tests.', - 'SetIdentifier': 'eu-central-1-Primary', - 'Type': 'A'} - }, { - 'Action': 'CREATE', - 'ResourceRecordSet': { - 'Name': '_octodns-default-pool.unit.tests.', - 'ResourceRecords': [{ - 'Value': '1.1.2.1'}, - { - 'Value': '1.1.2.2'}], - 'TTL': 60, + 'GeoLocation': { + 'CountryCode': 'JP'}, + 'Name': 'unit.tests.', + 'SetIdentifier': '0-ap-southeast-1-AS-JP', 'Type': 'A'} - }, { - 'Action': 'CREATE', - 'ResourceRecordSet': { - 'HealthCheckId': 'hc42', - 'Name': '_octodns-eu-central-1-value.unit.tests.', - 'ResourceRecords': [{ - 'Value': '1.3.1.2'}], - 'SetIdentifier': 'eu-central-1-001', - 'TTL': 60, - 'Type': 'A', - 'Weight': 1} - }, { - 'Action': 'CREATE', - 'ResourceRecordSet': { - 'HealthCheckId': 'hc42', - 'Name': '_octodns-eu-central-1-value.unit.tests.', - 'ResourceRecords': [{ - 'Value': '1.3.1.1'}], - 'SetIdentifier': 'eu-central-1-000', - 'TTL': 60, - 'Type': 'A', - 'Weight': 1} }, { 'Action': 'CREATE', 'ResourceRecordSet': { 'AliasTarget': { - 'DNSName': '_octodns-default-pool.unit.tests.', + 'DNSName': '_octodns-eu-central-1-pool.unit.tests.', 'EvaluateTargetHealth': True, 'HostedZoneId': 'z45'}, - 'Failover': 'SECONDARY', - 'Name': '_octodns-us-east-1-pool.unit.tests.', - 'SetIdentifier': 'us-east-1-Secondary-default', + 'GeoLocation': { + 'ContinentCode': 'EU'}, + 'Name': 'unit.tests.', + 'SetIdentifier': '1-eu-central-1-EU', 'Type': 'A'} - }, { - 'Action': 'CREATE', - 'ResourceRecordSet': { - 'HealthCheckId': 'hc42', - 'Name': '_octodns-us-east-1-value.unit.tests.', - 'ResourceRecords': [{ - 'Value': '1.5.1.2'}], - 'SetIdentifier': 'us-east-1-001', - 'TTL': 60, - 'Type': 'A', - 'Weight': 1} - }, { - 'Action': 'CREATE', - 'ResourceRecordSet': { - 'HealthCheckId': 'hc42', - 'Name': '_octodns-us-east-1-value.unit.tests.', - 'ResourceRecords': [{ - 'Value': '1.5.1.1'}], - 'SetIdentifier': 'us-east-1-000', - 'TTL': 60, - 'Type': 'A', - 'Weight': 1} }, { 'Action': 'CREATE', 'ResourceRecordSet': { 'AliasTarget': { - 'DNSName': '_octodns-ap-southeast-1-value.unit.tests.', + 'DNSName': '_octodns-eu-central-1-pool.unit.tests.', 'EvaluateTargetHealth': True, 'HostedZoneId': 'z45'}, - 'Failover': 'PRIMARY', - 'Name': '_octodns-ap-southeast-1-pool.unit.tests.', - 'SetIdentifier': 'ap-southeast-1-Primary', + 'GeoLocation': { + 'CountryCode': 'US', + 'SubdivisionCode': 'FL'}, + 'Name': 'unit.tests.', + 'SetIdentifier': '1-eu-central-1-NA-US-FL', 'Type': 'A'} }, { 'Action': 'CREATE', @@ -2412,11 +2455,12 @@ class TestRoute53Records(TestCase): 'DNSName': '_octodns-us-east-1-pool.unit.tests.', 'EvaluateTargetHealth': True, 'HostedZoneId': 'z45'}, - 'Failover': 'SECONDARY', - 'Name': '_octodns-eu-central-1-pool.unit.tests.', - 'SetIdentifier': 'eu-central-1-Secondary-us-east-1', + 'GeoLocation': { + 'CountryCode': '*'}, + 'Name': 'unit.tests.', + 'SetIdentifier': '2-us-east-1-None', 'Type': 'A'} - }], [r.mod('CREATE', []) for r in route53_records]) + }], expected_mods) for route53_record in route53_records: # Smoke test stringification From c82e94792e37e2d99df0a0d774a8615871bb84d1 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Mon, 7 Oct 2019 07:47:43 -0700 Subject: [PATCH 18/49] RackspaceProvider python3, value types hashing --- octodns/provider/rackspace.py | 15 ++++----- octodns/record/__init__.py | 15 ++++++++- tests/test_octodns_provider_rackspace.py | 8 ++--- tests/test_octodns_record.py | 40 ++++++++++++++++++++++++ 4 files changed, 66 insertions(+), 12 deletions(-) diff --git a/octodns/provider/rackspace.py b/octodns/provider/rackspace.py index 5038929..28b7f05 100644 --- a/octodns/provider/rackspace.py +++ b/octodns/provider/rackspace.py @@ -7,13 +7,16 @@ from __future__ import absolute_import, division, print_function, \ from requests import HTTPError, Session, post from collections import defaultdict import logging -import string import time from ..record import Record from .base import BaseProvider +def _value_keyer(v): + return '{}-{}-{}'.format(v.get('type', ''), v['name'], v.get('data', '')) + + def add_trailing_dot(s): assert s assert s[-1] != '.' @@ -28,12 +31,12 @@ def remove_trailing_dot(s): def escape_semicolon(s): assert s - return string.replace(s, ';', '\\;') + return s.replace(';', '\\;') def unescape_semicolon(s): assert s - return string.replace(s, '\\;', ';') + return s.replace('\\;', ';') class RackspaceProvider(BaseProvider): @@ -367,11 +370,9 @@ class RackspaceProvider(BaseProvider): self._delete('domains/{}/records?{}'.format(domain_id, params)) if updates: - data = {"records": sorted(updates, key=lambda v: v['name'])} + data = {"records": sorted(updates, key=_value_keyer)} self._put('domains/{}/records'.format(domain_id), data=data) if creates: - data = {"records": sorted(creates, key=lambda v: v['type'] + - v['name'] + - v.get('data', ''))} + data = {"records": sorted(creates, key=_value_keyer)} self._post('domains/{}/records'.format(domain_id), data=data) diff --git a/octodns/record/__init__.py b/octodns/record/__init__.py index b377d14..a782018 100644 --- a/octodns/record/__init__.py +++ b/octodns/record/__init__.py @@ -723,7 +723,8 @@ class _IpList(object): @classmethod def process(cls, values): - return values + # Translating None into '' so that the list will be sortable in python3 + return [v if v is not None else '' for v in values] class Ipv4List(_IpList): @@ -918,6 +919,9 @@ class MxValue(object): 'exchange': self.exchange, } + def __hash__(self): + return hash('{} {}'.format(self.preference, self.exchange)) + def __eq__(self, other): return ((self.preference, self.exchange) == (other.preference, other.exchange)) @@ -1010,6 +1014,9 @@ class NaptrValue(object): 'replacement': self.replacement, } + def __hash__(self): + return hash(self.__repr__()) + def __eq__(self, other): return ((self.order, self.preference, self.flags, self.service, self.regexp, self.replacement) == @@ -1145,6 +1152,9 @@ class SshfpValue(object): 'fingerprint': self.fingerprint, } + def __hash__(self): + return hash(self.__repr__()) + def __eq__(self, other): return ((self.algorithm, self.fingerprint_type, self.fingerprint) == (other.algorithm, other.fingerprint_type, other.fingerprint)) @@ -1283,6 +1293,9 @@ class SrvValue(object): 'target': self.target, } + def __hash__(self): + return hash(self.__repr__()) + def __eq__(self, other): return ((self.priority, self.weight, self.port, self.target) == (other.priority, other.weight, other.port, other.target)) diff --git a/tests/test_octodns_provider_rackspace.py b/tests/test_octodns_provider_rackspace.py index 3dfdd5f..0a6564d 100644 --- a/tests/test_octodns_provider_rackspace.py +++ b/tests/test_octodns_provider_rackspace.py @@ -792,13 +792,13 @@ class TestRackspaceProvider(TestCase): ExpectedUpdates = { "records": [{ "name": "unit.tests", - "id": "A-222222", - "data": "1.2.3.5", + "id": "A-111111", + "data": "1.2.3.4", "ttl": 3600 }, { "name": "unit.tests", - "id": "A-111111", - "data": "1.2.3.4", + "id": "A-222222", + "data": "1.2.3.5", "ttl": 3600 }, { "name": "unit.tests", diff --git a/tests/test_octodns_record.py b/tests/test_octodns_record.py index c26b301..0845ddf 100644 --- a/tests/test_octodns_record.py +++ b/tests/test_octodns_record.py @@ -594,6 +594,30 @@ class TestRecord(TestCase): # __repr__ doesn't blow up a.__repr__() + # Hash + v = NaptrValue({ + 'order': 30, + 'preference': 31, + 'flags': 'M', + 'service': 'N', + 'regexp': 'O', + 'replacement': 'z', + }) + o = NaptrValue({ + 'order': 30, + 'preference': 32, + 'flags': 'M', + 'service': 'N', + 'regexp': 'O', + 'replacement': 'z', + }) + values = set() + values.add(v) + self.assertTrue(v in values) + self.assertFalse(o in values) + values.add(o) + self.assertTrue(o in values) + def test_ns(self): a_values = ['5.6.7.8.', '6.7.8.9.', '7.8.9.0.'] a_data = {'ttl': 30, 'values': a_values} @@ -1114,6 +1138,14 @@ class TestRecord(TestCase): self.assertTrue(c >= c) self.assertTrue(c <= c) + # Hash + values = set() + values.add(a) + self.assertTrue(a in values) + self.assertFalse(b in values) + values.add(b) + self.assertTrue(b in values) + def test_srv_value(self): a = SrvValue({'priority': 0, 'weight': 0, 'port': 0, 'target': 'foo.'}) b = SrvValue({'priority': 1, 'weight': 0, 'port': 0, 'target': 'foo.'}) @@ -1172,6 +1204,14 @@ class TestRecord(TestCase): self.assertTrue(c >= c) self.assertTrue(c <= c) + # Hash + values = set() + values.add(a) + self.assertTrue(a in values) + self.assertFalse(b in values) + values.add(b) + self.assertTrue(b in values) + class TestRecordValidation(TestCase): zone = Zone('unit.tests.', []) From 25768c476fee352148a8af583709410426469679 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Mon, 7 Oct 2019 07:48:55 -0700 Subject: [PATCH 19/49] SelectelProvider python3 (tests) --- tests/test_octodns_provider_selectel.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/test_octodns_provider_selectel.py b/tests/test_octodns_provider_selectel.py index a2ba39e..7ad1e6b 100644 --- a/tests/test_octodns_provider_selectel.py +++ b/tests/test_octodns_provider_selectel.py @@ -6,6 +6,7 @@ from __future__ import absolute_import, division, print_function, \ unicode_literals from unittest import TestCase +from six import text_type import requests_mock @@ -288,7 +289,7 @@ class TestSelectelProvider(TestCase): with self.assertRaises(Exception) as ctx: SelectelProvider(123, 'fail_token') - self.assertEquals(ctx.exception.message, + self.assertEquals(text_type(ctx.exception), 'Authorization failed. Invalid or empty token.') @requests_mock.Mocker() From 90a60d3dbdd453fb691b698bc2fe8f7936e37584 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Mon, 7 Oct 2019 07:53:08 -0700 Subject: [PATCH 20/49] TransipProvider python3 --- octodns/provider/transip.py | 2 +- tests/test_octodns_provider_transip.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/octodns/provider/transip.py b/octodns/provider/transip.py index 09920a9..7458e36 100644 --- a/octodns/provider/transip.py +++ b/octodns/provider/transip.py @@ -137,7 +137,7 @@ class TransipProvider(BaseProvider): try: self._client.get_info(plan.desired.name[:-1]) except WebFault as e: - self.log.warning('_apply: %s ', e.message) + self.log.exception('_apply: get_info failed') raise e _dns_entries = [] diff --git a/tests/test_octodns_provider_transip.py b/tests/test_octodns_provider_transip.py index c56509a..3fbfc44 100644 --- a/tests/test_octodns_provider_transip.py +++ b/tests/test_octodns_provider_transip.py @@ -5,8 +5,8 @@ from __future__ import absolute_import, division, print_function, \ unicode_literals -# from mock import Mock, call from os.path import dirname, join +from six import text_type from suds import WebFault @@ -176,7 +176,7 @@ N4OiVz1I3rbZGYa396lpxO6ku8yCglisL1yrSP6DdEUp66ntpKVd self.assertEquals( 'populate: (102) Transip used as target' + ' for non-existing zone: notfound.unit.tests.', - ctx.exception.message) + text_type(ctx.exception)) # Happy Plan - Zone does not exists # Won't trigger an exception if provider is NOT used as a target for a From 14063186f3e9bc419fa6eb50f30e9a7dc4feaa9c Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Mon, 7 Oct 2019 07:56:10 -0700 Subject: [PATCH 21/49] YamlProvider python3, tests --- tests/test_octodns_provider_yaml.py | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/tests/test_octodns_provider_yaml.py b/tests/test_octodns_provider_yaml.py index 700f3c3..e16aaa5 100644 --- a/tests/test_octodns_provider_yaml.py +++ b/tests/test_octodns_provider_yaml.py @@ -82,9 +82,9 @@ class TestYamlProvider(TestCase): target.populate(reloaded) self.assertDictEqual( {'included': ['test']}, - filter( + list(filter( lambda x: x.name == 'included', reloaded.records - )[0]._octodns) + ))[0]._octodns) self.assertFalse(zone.changes(reloaded, target=source)) @@ -120,7 +120,7 @@ class TestYamlProvider(TestCase): self.assertTrue('value' in data.pop('www.sub')) # make sure nothing is left - self.assertEquals([], data.keys()) + self.assertEquals([], list(data.keys())) with open(dynamic_yaml_file) as fh: data = safe_load(fh.read()) @@ -149,7 +149,7 @@ class TestYamlProvider(TestCase): # self.assertTrue('dynamic' in dyna) # make sure nothing is left - self.assertEquals([], data.keys()) + self.assertEquals([], list(data.keys())) def test_empty(self): source = YamlProvider('test', join(dirname(__file__), 'config')) @@ -255,8 +255,8 @@ class TestSplitYamlProvider(TestCase): # We add everything plan = target.plan(zone) - self.assertEquals(15, len(filter(lambda c: isinstance(c, Create), - plan.changes))) + self.assertEquals(15, len(list(filter( + lambda c: isinstance(c, Create), plan.changes)))) self.assertFalse(isdir(zone_dir)) # Now actually do it @@ -264,8 +264,8 @@ class TestSplitYamlProvider(TestCase): # Dynamic plan plan = target.plan(dynamic_zone) - self.assertEquals(5, len(filter(lambda c: isinstance(c, Create), - plan.changes))) + self.assertEquals(5, len(list(filter( + lambda c: isinstance(c, Create), plan.changes)))) self.assertFalse(isdir(dynamic_zone_dir)) # Apply it self.assertEquals(5, target.apply(plan)) @@ -276,16 +276,16 @@ class TestSplitYamlProvider(TestCase): target.populate(reloaded) self.assertDictEqual( {'included': ['test']}, - filter( + list(filter( lambda x: x.name == 'included', reloaded.records - )[0]._octodns) + ))[0]._octodns) self.assertFalse(zone.changes(reloaded, target=source)) # A 2nd sync should still create everything plan = target.plan(zone) - self.assertEquals(15, len(filter(lambda c: isinstance(c, Create), - plan.changes))) + self.assertEquals(15, len(list(filter( + lambda c: isinstance(c, Create), plan.changes)))) yaml_file = join(zone_dir, '$unit.tests.yaml') self.assertTrue(isfile(yaml_file)) From e0c5962d79df12ace77d763833010b0193b6dce4 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Mon, 7 Oct 2019 07:58:44 -0700 Subject: [PATCH 22/49] AxfrSource python3 --- octodns/source/axfr.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/octodns/source/axfr.py b/octodns/source/axfr.py index f35c4b3..be2acf5 100644 --- a/octodns/source/axfr.py +++ b/octodns/source/axfr.py @@ -15,6 +15,7 @@ from dns.exception import DNSException from collections import defaultdict from os import listdir from os.path import join +from six import text_type import logging from ..record import Record @@ -179,8 +180,7 @@ class ZoneFileSourceNotFound(ZoneFileSourceException): class ZoneFileSourceLoadFailure(ZoneFileSourceException): def __init__(self, error): - super(ZoneFileSourceLoadFailure, self).__init__( - error.message) + super(ZoneFileSourceLoadFailure, self).__init__(text_type(error)) class ZoneFileSource(AxfrBaseSource): From 0708b797da2b1208bd66b10881deca40585ce6d4 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Mon, 7 Oct 2019 08:29:50 -0700 Subject: [PATCH 23/49] TinyDnsSource python3 --- octodns/source/tinydns.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/octodns/source/tinydns.py b/octodns/source/tinydns.py index dc2bc1b..0ee100a 100755 --- a/octodns/source/tinydns.py +++ b/octodns/source/tinydns.py @@ -67,7 +67,8 @@ class TinyDnsBaseSource(BaseSource): values = [] for record in records: - new_value = record[0].decode('unicode-escape').replace(";", "\\;") + new_value = record[0].encode('latin1').decode('unicode-escape') \ + .replace(";", "\\;") values.append(new_value) try: From db8de8acb888713db8d88784de090c0e45c122ad Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Mon, 7 Oct 2019 08:41:28 -0700 Subject: [PATCH 24/49] Fix Manager ordering assumptions --- tests/config/provider-problems.yaml | 28 ++++++++++++++++++++++++++++ tests/config/unknown-provider.yaml | 16 ---------------- tests/test_octodns_manager.py | 10 +++++----- 3 files changed, 33 insertions(+), 21 deletions(-) create mode 100644 tests/config/provider-problems.yaml diff --git a/tests/config/provider-problems.yaml b/tests/config/provider-problems.yaml new file mode 100644 index 0000000..9071046 --- /dev/null +++ b/tests/config/provider-problems.yaml @@ -0,0 +1,28 @@ +providers: + yaml: + class: octodns.provider.yaml.YamlProvider + directory: ./config + simple_source: + class: helpers.SimpleSource +zones: + missing.sources.: + targets: + - yaml + missing.targets.: + sources: + - yaml + unknown.source.: + sources: + - not-there + targets: + - yaml + unknown.target.: + sources: + - yaml + targets: + - not-there-either + not.targetable.: + sources: + - yaml + targets: + - simple_source diff --git a/tests/config/unknown-provider.yaml b/tests/config/unknown-provider.yaml index 9071046..a0e9f55 100644 --- a/tests/config/unknown-provider.yaml +++ b/tests/config/unknown-provider.yaml @@ -5,24 +5,8 @@ providers: simple_source: class: helpers.SimpleSource zones: - missing.sources.: - targets: - - yaml - missing.targets.: - sources: - - yaml unknown.source.: sources: - not-there targets: - yaml - unknown.target.: - sources: - - yaml - targets: - - not-there-either - not.targetable.: - sources: - - yaml - targets: - - simple_source diff --git a/tests/test_octodns_manager.py b/tests/test_octodns_manager.py index 43feed5..60a1922 100644 --- a/tests/test_octodns_manager.py +++ b/tests/test_octodns_manager.py @@ -62,25 +62,25 @@ class TestManager(TestCase): def test_missing_source(self): with self.assertRaises(Exception) as ctx: - Manager(get_config_filename('unknown-provider.yaml')) \ + Manager(get_config_filename('provider-problems.yaml')) \ .sync(['missing.sources.']) self.assertTrue('missing sources' in text_type(ctx.exception)) def test_missing_targets(self): with self.assertRaises(Exception) as ctx: - Manager(get_config_filename('unknown-provider.yaml')) \ + Manager(get_config_filename('provider-problems.yaml')) \ .sync(['missing.targets.']) self.assertTrue('missing targets' in text_type(ctx.exception)) def test_unknown_source(self): with self.assertRaises(Exception) as ctx: - Manager(get_config_filename('unknown-provider.yaml')) \ + Manager(get_config_filename('provider-problems.yaml')) \ .sync(['unknown.source.']) self.assertTrue('unknown source' in text_type(ctx.exception)) def test_unknown_target(self): with self.assertRaises(Exception) as ctx: - Manager(get_config_filename('unknown-provider.yaml')) \ + Manager(get_config_filename('provider-problems.yaml')) \ .sync(['unknown.target.']) self.assertTrue('unknown target' in text_type(ctx.exception)) @@ -99,7 +99,7 @@ class TestManager(TestCase): def test_source_only_as_a_target(self): with self.assertRaises(Exception) as ctx: - Manager(get_config_filename('unknown-provider.yaml')) \ + Manager(get_config_filename('provider-problems.yaml')) \ .sync(['not.targetable.']) self.assertTrue('does not support targeting' in text_type(ctx.exception)) From bfa1fadde97f7a3567103e2bcb500d7e484f0967 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Mon, 7 Oct 2019 08:44:25 -0700 Subject: [PATCH 25/49] Fix CloudflareProvider test ordering assumptions --- tests/test_octodns_provider_cloudflare.py | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/tests/test_octodns_provider_cloudflare.py b/tests/test_octodns_provider_cloudflare.py index c656cab..013c5f7 100644 --- a/tests/test_octodns_provider_cloudflare.py +++ b/tests/test_octodns_provider_cloudflare.py @@ -313,7 +313,7 @@ class TestCloudflareProvider(TestCase): 'dns_records/fc12ab34cd5611334422ab3322997653'), call('DELETE', '/zones/ff12ab34cd5611334422ab3322997650/' 'dns_records/fc12ab34cd5611334422ab3322997654') - ]) + ], any_order=True) def test_update_add_swap(self): provider = CloudflareProvider('test', 'email', 'token') @@ -743,23 +743,25 @@ class TestCloudflareProvider(TestCase): # the CDN. self.assertEquals(3, len(zone.records)) - record = list(zone.records)[0] - self.assertEquals('multi', record.name) - self.assertEquals('multi.unit.tests.', record.fqdn) + ordered = sorted(zone.records, key=lambda r: r.name) + + record = ordered[0] + self.assertEquals('a', record.name) + self.assertEquals('a.unit.tests.', record.fqdn) self.assertEquals('CNAME', record._type) - self.assertEquals('multi.unit.tests.cdn.cloudflare.net.', record.value) + self.assertEquals('a.unit.tests.cdn.cloudflare.net.', record.value) - record = list(zone.records)[1] + record = ordered[1] self.assertEquals('cname', record.name) self.assertEquals('cname.unit.tests.', record.fqdn) self.assertEquals('CNAME', record._type) self.assertEquals('cname.unit.tests.cdn.cloudflare.net.', record.value) - record = list(zone.records)[2] - self.assertEquals('a', record.name) - self.assertEquals('a.unit.tests.', record.fqdn) + record = ordered[2] + self.assertEquals('multi', record.name) + self.assertEquals('multi.unit.tests.', record.fqdn) self.assertEquals('CNAME', record._type) - self.assertEquals('a.unit.tests.cdn.cloudflare.net.', record.value) + self.assertEquals('multi.unit.tests.cdn.cloudflare.net.', record.value) # CDN enabled records can't be updated, we don't know the real values # never point a Cloudflare record to itself. From 7958233fccf9ea22d95e2fd06c48d7d0a4529e26 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Mon, 7 Oct 2019 09:17:48 -0700 Subject: [PATCH 26/49] Consistently order changes :-/ Many providers make their modifications in the order that changes comes. In python3 this causes things to be inconsistently ordered. That mostly works, but could result in hidenbugs (e.g. Route53Provider's batching could be completely different based on the order it sees changes.) Sorting changes consistently is a good thing and it shouldn't hurt situations where providers are already doing their own ordering. All-in-all more consistent is better and we have to be explicit with python 3. --- octodns/provider/plan.py | 6 +- octodns/record/__init__.py | 6 ++ tests/test_octodns_provider_cloudflare.py | 2 +- tests/test_octodns_provider_digitalocean.py | 15 ++- tests/test_octodns_provider_dnsimple.py | 27 +++++- tests/test_octodns_provider_dnsmadeeasy.py | 22 ++++- tests/test_octodns_provider_ns1.py | 10 +- tests/test_octodns_provider_ovh.py | 101 ++++++++++---------- tests/test_octodns_provider_route53.py | 4 +- 9 files changed, 130 insertions(+), 63 deletions(-) diff --git a/octodns/provider/plan.py b/octodns/provider/plan.py index d4589f2..5b8be68 100644 --- a/octodns/provider/plan.py +++ b/octodns/provider/plan.py @@ -28,7 +28,11 @@ class Plan(object): delete_pcent_threshold=MAX_SAFE_DELETE_PCENT): self.existing = existing self.desired = desired - self.changes = changes + # Sort changes to ensure we always have a consistent ordering for + # things that make assumptions about that. Many providers will do their + # own ordering to ensure things happen in a way that makes sense to + # them and/or is as safe as possible. + self.changes = sorted(changes) self.exists = exists self.update_pcent_threshold = update_pcent_threshold self.delete_pcent_threshold = delete_pcent_threshold diff --git a/octodns/record/__init__.py b/octodns/record/__init__.py index a782018..47d2e9b 100644 --- a/octodns/record/__init__.py +++ b/octodns/record/__init__.py @@ -25,6 +25,12 @@ class Change(object): 'Returns new if we have one, existing otherwise' return self.new or self.existing + def __lt__(self, other): + self_record = self.record + other_record = other.record + return ((self_record.name, self_record._type) < + (other_record.name, other_record._type)) + class Create(Change): diff --git a/tests/test_octodns_provider_cloudflare.py b/tests/test_octodns_provider_cloudflare.py index 013c5f7..3581033 100644 --- a/tests/test_octodns_provider_cloudflare.py +++ b/tests/test_octodns_provider_cloudflare.py @@ -313,7 +313,7 @@ class TestCloudflareProvider(TestCase): 'dns_records/fc12ab34cd5611334422ab3322997653'), call('DELETE', '/zones/ff12ab34cd5611334422ab3322997650/' 'dns_records/fc12ab34cd5611334422ab3322997654') - ], any_order=True) + ]) def test_update_add_swap(self): provider = CloudflareProvider('test', 'email', 'token') diff --git a/tests/test_octodns_provider_digitalocean.py b/tests/test_octodns_provider_digitalocean.py index 448b26e..ebb5319 100644 --- a/tests/test_octodns_provider_digitalocean.py +++ b/tests/test_octodns_provider_digitalocean.py @@ -176,7 +176,20 @@ class TestDigitalOceanProvider(TestCase): call('GET', '/domains/unit.tests/records', {'page': 1}), # delete the initial A record call('DELETE', '/domains/unit.tests/records/11189877'), - # created at least one of the record with expected data + # created at least some of the record with expected data + call('POST', '/domains/unit.tests/records', data={ + 'data': '1.2.3.4', + 'name': '@', + 'ttl': 300, 'type': 'A'}), + call('POST', '/domains/unit.tests/records', data={ + 'data': '1.2.3.5', + 'name': '@', + 'ttl': 300, 'type': 'A'}), + call('POST', '/domains/unit.tests/records', data={ + 'data': 'ca.unit.tests.', + 'flags': 0, 'name': '@', + 'tag': 'issue', + 'ttl': 3600, 'type': 'CAA'}), call('POST', '/domains/unit.tests/records', data={ 'name': '_srv._tcp', 'weight': 20, diff --git a/tests/test_octodns_provider_dnsimple.py b/tests/test_octodns_provider_dnsimple.py index 9e8586c..e3a9b8d 100644 --- a/tests/test_octodns_provider_dnsimple.py +++ b/tests/test_octodns_provider_dnsimple.py @@ -139,7 +139,32 @@ class TestDnsimpleProvider(TestCase): provider._client._request.assert_has_calls([ # created the domain call('POST', '/domains', data={'name': 'unit.tests'}), - # created at least one of the record with expected data + # created at least some of the record with expected data + call('POST', '/zones/unit.tests/records', data={ + 'content': '1.2.3.4', + 'type': 'A', + 'name': '', + 'ttl': 300}), + call('POST', '/zones/unit.tests/records', data={ + 'content': '1.2.3.5', + 'type': 'A', + 'name': '', + 'ttl': 300}), + call('POST', '/zones/unit.tests/records', data={ + 'content': '0 issue "ca.unit.tests"', + 'type': 'CAA', + 'name': '', + 'ttl': 3600}), + call('POST', '/zones/unit.tests/records', data={ + 'content': '1 1 7491973e5f8b39d5327cd4e08bc81b05f7710b49', + 'type': 'SSHFP', + 'name': '', + 'ttl': 3600}), + call('POST', '/zones/unit.tests/records', data={ + 'content': '1 1 bf6b6825d2977c511a475bbefb88aad54a92ac73', + 'type': 'SSHFP', + 'name': '', + 'ttl': 3600}), call('POST', '/zones/unit.tests/records', data={ 'content': '20 30 foo-1.unit.tests.', 'priority': 10, diff --git a/tests/test_octodns_provider_dnsmadeeasy.py b/tests/test_octodns_provider_dnsmadeeasy.py index ea14376..ba61b94 100644 --- a/tests/test_octodns_provider_dnsmadeeasy.py +++ b/tests/test_octodns_provider_dnsmadeeasy.py @@ -149,7 +149,27 @@ class TestDnsMadeEasyProvider(TestCase): call('POST', '/', data={'name': 'unit.tests'}), # get all domains to build the cache call('GET', '/'), - # created at least one of the record with expected data + # created at least some of the record with expected data + call('POST', '/123123/records', data={ + 'type': 'A', + 'name': '', + 'value': '1.2.3.4', + 'ttl': 300}), + call('POST', '/123123/records', data={ + 'type': 'A', + 'name': '', + 'value': '1.2.3.5', + 'ttl': 300}), + call('POST', '/123123/records', data={ + 'type': 'ANAME', + 'name': '', + 'value': 'aname.unit.tests.', + 'ttl': 1800}), + call('POST', '/123123/records', data={ + 'name': '', + 'value': 'ca.unit.tests', + 'issuerCritical': 0, 'caaType': 'issue', + 'ttl': 3600, 'type': 'CAA'}), call('POST', '/123123/records', data={ 'name': '_srv._tcp', 'weight': 20, diff --git a/tests/test_octodns_provider_ns1.py b/tests/test_octodns_provider_ns1.py index 91d1a3f..9d23806 100644 --- a/tests/test_octodns_provider_ns1.py +++ b/tests/test_octodns_provider_ns1.py @@ -400,9 +400,9 @@ class TestNs1Provider(TestCase): self.assertEquals(3, got_n) nsone_zone.loadRecord.assert_has_calls([ call('unit.tests', u'A'), - call('geo', u'A'), call('delete-me', u'A'), - ], any_order=True) + call('geo', u'A'), + ]) mock_record.assert_has_calls([ call.update(answers=[{'answer': [u'1.2.3.4'], 'meta': {}}], filters=[], @@ -410,6 +410,8 @@ class TestNs1Provider(TestCase): call.update(answers=[{u'answer': [u'1.2.3.4'], u'meta': {}}], filters=[], ttl=32), + call.delete(), + call.delete(), call.update( answers=[ {u'answer': [u'101.102.103.104'], u'meta': {}}, @@ -427,9 +429,7 @@ class TestNs1Provider(TestCase): {u'filter': u'select_first_n', u'config': {u'N': 1}}, ], ttl=34), - call.delete(), - call.delete() - ], any_order=True) + ]) def test_escaping(self): provider = Ns1Provider('test', 'api-key') diff --git a/tests/test_octodns_provider_ovh.py b/tests/test_octodns_provider_ovh.py index 1f41abf..924591f 100644 --- a/tests/test_octodns_provider_ovh.py +++ b/tests/test_octodns_provider_ovh.py @@ -382,65 +382,64 @@ class TestOvhProvider(TestCase): get_mock.side_effect = [[100], [101], [102], [103]] provider.apply(plan) wanted_calls = [ - call(u'/domain/zone/unit.tests/record', fieldType=u'TXT', - subDomain='txt', target=u'TXT text', ttl=1400), - call(u'/domain/zone/unit.tests/record', fieldType=u'DKIM', - subDomain='dkim', target=self.valid_dkim_key, - ttl=1300), - call(u'/domain/zone/unit.tests/record', fieldType=u'A', - subDomain=u'', target=u'1.2.3.4', ttl=100), - call(u'/domain/zone/unit.tests/record', fieldType=u'SRV', + call('/domain/zone/unit.tests/record', fieldType='A', + subDomain='', target='1.2.3.4', ttl=100), + call('/domain/zone/unit.tests/record', fieldType='AAAA', + subDomain='', target='1:1ec:1::1', ttl=200), + call('/domain/zone/unit.tests/record', fieldType='MX', + subDomain='', target='10 mx1.unit.tests.', ttl=400), + call('/domain/zone/unit.tests/record', fieldType='SPF', + subDomain='', + target='v=spf1 include:unit.texts.redirect ~all', + ttl=1000), + call('/domain/zone/unit.tests/record', fieldType='SSHFP', + subDomain='', + target='1 1 bf6b6825d2977c511a475bbefb88aad54a92ac73', + ttl=1100), + call('/domain/zone/unit.tests/record', fieldType='PTR', + subDomain='4', target='unit.tests.', ttl=900), + call('/domain/zone/unit.tests/record', fieldType='SRV', subDomain='_srv._tcp', - target=u'10 20 30 foo-1.unit.tests.', ttl=800), - call(u'/domain/zone/unit.tests/record', fieldType=u'SRV', + target='10 20 30 foo-1.unit.tests.', ttl=800), + call('/domain/zone/unit.tests/record', fieldType='SRV', subDomain='_srv._tcp', - target=u'40 50 60 foo-2.unit.tests.', ttl=800), - call(u'/domain/zone/unit.tests/record', fieldType=u'PTR', - subDomain='4', target=u'unit.tests.', ttl=900), - call(u'/domain/zone/unit.tests/record', fieldType=u'NS', - subDomain='www3', target=u'ns3.unit.tests.', ttl=700), - call(u'/domain/zone/unit.tests/record', fieldType=u'NS', - subDomain='www3', target=u'ns4.unit.tests.', ttl=700), - call(u'/domain/zone/unit.tests/record', - fieldType=u'SSHFP', subDomain=u'', ttl=1100, - target=u'1 1 bf6b6825d2977c511a475bbefb88a' - u'ad54' - u'a92ac73', - ), - call(u'/domain/zone/unit.tests/record', fieldType=u'AAAA', - subDomain=u'', target=u'1:1ec:1::1', ttl=200), - call(u'/domain/zone/unit.tests/record', fieldType=u'MX', - subDomain=u'', target=u'10 mx1.unit.tests.', ttl=400), - call(u'/domain/zone/unit.tests/record', fieldType=u'CNAME', - subDomain='www2', target=u'unit.tests.', ttl=300), - call(u'/domain/zone/unit.tests/record', fieldType=u'SPF', - subDomain=u'', ttl=1000, - target=u'v=spf1 include:unit.texts.' - u'redirect ~all', - ), - call(u'/domain/zone/unit.tests/record', fieldType=u'A', - subDomain='sub', target=u'1.2.3.4', ttl=200), - call(u'/domain/zone/unit.tests/record', fieldType=u'NAPTR', - subDomain='naptr', ttl=500, - target=u'10 100 "S" "SIP+D2U" "!^.*$!sip:' - u'info@bar' - u'.example.com!" .' - ), - call(u'/domain/zone/unit.tests/refresh')] - - post_mock.assert_has_calls(wanted_calls, any_order=True) + target='40 50 60 foo-2.unit.tests.', ttl=800), + call('/domain/zone/unit.tests/record', fieldType='DKIM', + subDomain='dkim', + target='p=MIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKBgQCxLaG' + '16G4SaEcXVdiIxTg7gKSGbHKQLm30CHib1h9FzS9nkcyvQSyQj1r' + 'MFyqC//tft3ohx3nvJl+bGCWxdtLYDSmir9PW54e5CTdxEh8MWRk' + 'BO3StF6QG/tAh3aTGDmkqhIJGLb87iHvpmVKqURmEUzJPv5KPJfW' + 'LofADI+q9lQIDAQAB', ttl=1300), + call('/domain/zone/unit.tests/record', fieldType='NAPTR', + subDomain='naptr', + target='10 100 "S" "SIP+D2U" "!^.*$!sip:info@bar.exam' + 'ple.com!" .', ttl=500), + call('/domain/zone/unit.tests/record', fieldType='A', + subDomain='sub', target='1.2.3.4', ttl=200), + call('/domain/zone/unit.tests/record', fieldType='TXT', + subDomain='txt', target='TXT text', ttl=1400), + call('/domain/zone/unit.tests/record', fieldType='CNAME', + subDomain='www2', target='unit.tests.', ttl=300), + call('/domain/zone/unit.tests/record', fieldType='NS', + subDomain='www3', target='ns3.unit.tests.', ttl=700), + call('/domain/zone/unit.tests/record', fieldType='NS', + subDomain='www3', target='ns4.unit.tests.', ttl=700), + call('/domain/zone/unit.tests/refresh')] + + post_mock.assert_has_calls(wanted_calls) # Get for delete calls wanted_get_calls = [ - call(u'/domain/zone/unit.tests/record', fieldType=u'TXT', - subDomain='txt'), - call(u'/domain/zone/unit.tests/record', fieldType=u'DKIM', - subDomain='dkim'), call(u'/domain/zone/unit.tests/record', fieldType=u'A', subDomain=u''), + call(u'/domain/zone/unit.tests/record', fieldType=u'DKIM', + subDomain='dkim'), call(u'/domain/zone/unit.tests/record', fieldType=u'A', - subDomain='fake')] - get_mock.assert_has_calls(wanted_get_calls, any_order=True) + subDomain='fake'), + call(u'/domain/zone/unit.tests/record', fieldType=u'TXT', + subDomain='txt')] + get_mock.assert_has_calls(wanted_get_calls) # 4 delete calls for update and delete delete_mock.assert_has_calls( [call(u'/domain/zone/unit.tests/record/100'), diff --git a/tests/test_octodns_provider_route53.py b/tests/test_octodns_provider_route53.py index b0ee342..7691804 100644 --- a/tests/test_octodns_provider_route53.py +++ b/tests/test_octodns_provider_route53.py @@ -1882,10 +1882,10 @@ class TestRoute53Provider(TestCase): @patch('octodns.provider.route53.Route53Provider._really_apply') def test_apply_3(self, really_apply_mock, _): - # with a max of seven modifications, four calls + # with a max of seven modifications, three calls provider, plan = self._get_test_plan(7) provider.apply(plan) - self.assertEquals(4, really_apply_mock.call_count) + self.assertEquals(3, really_apply_mock.call_count) @patch('octodns.provider.route53.Route53Provider._load_records') @patch('octodns.provider.route53.Route53Provider._really_apply') From be06a5da94fe3ffdbac2d44dec9e426e7ba320e3 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Mon, 7 Oct 2019 09:31:15 -0700 Subject: [PATCH 27/49] Make sure map and keys are lists when needed --- octodns/provider/dyn.py | 4 ++-- tests/test_octodns_provider_dyn.py | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/octodns/provider/dyn.py b/octodns/provider/dyn.py index c306238..30ba8bd 100644 --- a/octodns/provider/dyn.py +++ b/octodns/provider/dyn.py @@ -903,10 +903,10 @@ class DynProvider(BaseProvider): # Sort the values for consistent ordering so that we can compare values = sorted(values, key=_dynamic_value_sort_key) # Ensure that weight is included and if not use the default - values = map(lambda v: { + values = list(map(lambda v: { 'value': v['value'], 'weight': v.get('weight', 1), - }, values) + }, values)) # Walk through our existing pools looking for a match we can use for pool in pools: diff --git a/tests/test_octodns_provider_dyn.py b/tests/test_octodns_provider_dyn.py index 4f224fc..7c023fd 100644 --- a/tests/test_octodns_provider_dyn.py +++ b/tests/test_octodns_provider_dyn.py @@ -670,8 +670,8 @@ class TestDynProviderGeo(TestCase): tds = provider.traffic_directors self.assertEquals(set(['unit.tests.', 'geo.unit.tests.']), set(tds.keys())) - self.assertEquals(['A'], tds['unit.tests.'].keys()) - self.assertEquals(['A'], tds['geo.unit.tests.'].keys()) + self.assertEquals(['A'], list(tds['unit.tests.'].keys())) + self.assertEquals(['A'], list(tds['geo.unit.tests.'].keys())) provider.log.warn.assert_called_with("Unsupported TrafficDirector " "'%s'", 'something else') From 25cc4f42db439327747bb8101a6d4d7381f0ebb8 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Mon, 7 Oct 2019 09:33:14 -0700 Subject: [PATCH 28/49] Explicit list on filter when checking for non-existant targets --- octodns/manager.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/octodns/manager.py b/octodns/manager.py index 3984a5a..1c6a401 100644 --- a/octodns/manager.py +++ b/octodns/manager.py @@ -263,7 +263,8 @@ class Manager(object): except KeyError: raise Exception('Zone {} is missing targets'.format(zone_name)) if eligible_targets: - targets = filter(lambda d: d in eligible_targets, targets) + targets = list(filter(lambda d: d in eligible_targets, + targets)) if not targets: # Don't bother planning (and more importantly populating) zones From 3c75dc81f890c707895af25e22941050062c2ea4 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Mon, 7 Oct 2019 09:34:10 -0700 Subject: [PATCH 29/49] Require python 3.7 to pass --- .travis.yml | 3 --- 1 file changed, 3 deletions(-) diff --git a/.travis.yml b/.travis.yml index b1a8204..eb609ff 100644 --- a/.travis.yml +++ b/.travis.yml @@ -3,9 +3,6 @@ matrix: include: - python: 2.7 - python: 3.7 - dist: xenial # required for Python >= 3.7 on Travis CI - allow_failures: - - python: 3.7 before_install: pip install --upgrade pip script: ./script/cibuild notifications: From 7867ad2093f775fbb4d83564600c299e9e51f31f Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Mon, 7 Oct 2019 09:40:23 -0700 Subject: [PATCH 30/49] Use six's StringIO, remove compat.py --- octodns/compat.py | 10 ---------- octodns/provider/plan.py | 3 +-- tests/test_octodns_plan.py | 3 +-- tests/test_octodns_yaml.py | 2 +- 4 files changed, 3 insertions(+), 15 deletions(-) delete mode 100644 octodns/compat.py diff --git a/octodns/compat.py b/octodns/compat.py deleted file mode 100644 index 6586cff..0000000 --- a/octodns/compat.py +++ /dev/null @@ -1,10 +0,0 @@ -# -# Python 2/3 compat bits -# - -try: # pragma: no cover - from StringIO import StringIO -except ImportError: # pragma: no cover - from io import StringIO - -StringIO diff --git a/octodns/provider/plan.py b/octodns/provider/plan.py index 5b8be68..af6863a 100644 --- a/octodns/provider/plan.py +++ b/octodns/provider/plan.py @@ -8,8 +8,7 @@ from __future__ import absolute_import, division, print_function, \ from logging import DEBUG, ERROR, INFO, WARN, getLogger from sys import stdout -from six import text_type -from ..compat import StringIO +from six import StringIO, text_type class UnsafePlan(Exception): diff --git a/tests/test_octodns_plan.py b/tests/test_octodns_plan.py index a017431..9cf812d 100644 --- a/tests/test_octodns_plan.py +++ b/tests/test_octodns_plan.py @@ -6,10 +6,9 @@ from __future__ import absolute_import, division, print_function, \ unicode_literals from logging import getLogger -from six import text_type +from six import StringIO, text_type from unittest import TestCase -from octodns.compat import StringIO from octodns.provider.plan import Plan, PlanHtml, PlanLogger, PlanMarkdown from octodns.record import Create, Delete, Record, Update from octodns.zone import Zone diff --git a/tests/test_octodns_yaml.py b/tests/test_octodns_yaml.py index ddcd818..f211854 100644 --- a/tests/test_octodns_yaml.py +++ b/tests/test_octodns_yaml.py @@ -5,10 +5,10 @@ from __future__ import absolute_import, division, print_function, \ unicode_literals +from six import StringIO from unittest import TestCase from yaml.constructor import ConstructorError -from octodns.compat import StringIO from octodns.yaml import safe_dump, safe_load From 0a7d63ef06e59dab5f0cd6b110c39b30b0114800 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Wed, 9 Oct 2019 14:35:53 -0700 Subject: [PATCH 31/49] Show line numbers missing coverage --- script/coverage | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/script/coverage b/script/coverage index 8552eba..ad8189e 100755 --- a/script/coverage +++ b/script/coverage @@ -29,7 +29,7 @@ export GOOGLE_APPLICATION_CREDENTIALS= coverage run --branch --source=octodns --omit=octodns/cmds/* "$(command -v nosetests)" --with-xunit "$@" coverage html coverage xml -coverage report +coverage report --show-missing coverage report | grep ^TOTAL | grep -qv 100% && { echo "Incomplete code coverage" >&2 exit 1 From 3f487197df7e6dacdd703b8e9888d29b67f74ba1 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Wed, 9 Oct 2019 15:09:37 -0700 Subject: [PATCH 32/49] Manager throws ManagerException rather than Exception, more robust tests --- octodns/manager.py | 71 ++++++++++++++++++----------------- tests/test_octodns_manager.py | 39 +++++++++---------- 2 files changed, 57 insertions(+), 53 deletions(-) diff --git a/octodns/manager.py b/octodns/manager.py index 1c6a401..85de76b 100644 --- a/octodns/manager.py +++ b/octodns/manager.py @@ -69,6 +69,10 @@ class MainThreadExecutor(object): return MakeThreadFuture(func, args, kwargs) +class ManagerException(Exception): + pass + + class Manager(object): log = logging.getLogger('Manager') @@ -105,16 +109,16 @@ class Manager(object): _class = provider_config.pop('class') except KeyError: self.log.exception('Invalid provider class') - raise Exception('Provider {} is missing class' - .format(provider_name)) + raise ManagerException('Provider {} is missing class' + .format(provider_name)) _class = self._get_named_class('provider', _class) kwargs = self._build_kwargs(provider_config) try: self.providers[provider_name] = _class(provider_name, **kwargs) except TypeError: self.log.exception('Invalid provider config') - raise Exception('Incorrect provider config for {}' - .format(provider_name)) + raise ManagerException('Incorrect provider config for {}' + .format(provider_name)) zone_tree = {} # sort by reversed strings so that parent zones always come first @@ -148,8 +152,8 @@ class Manager(object): _class = plan_output_config.pop('class') except KeyError: self.log.exception('Invalid plan_output class') - raise Exception('plan_output {} is missing class' - .format(plan_output_name)) + raise ManagerException('plan_output {} is missing class' + .format(plan_output_name)) _class = self._get_named_class('plan_output', _class) kwargs = self._build_kwargs(plan_output_config) try: @@ -157,8 +161,8 @@ class Manager(object): _class(plan_output_name, **kwargs) except TypeError: self.log.exception('Invalid plan_output config') - raise Exception('Incorrect plan_output config for {}' - .format(plan_output_name)) + raise ManagerException('Incorrect plan_output config for {}' + .format(plan_output_name)) def _get_named_class(self, _type, _class): try: @@ -167,13 +171,15 @@ class Manager(object): except (ImportError, ValueError): self.log.exception('_get_{}_class: Unable to import ' 'module %s', _class) - raise Exception('Unknown {} class: {}'.format(_type, _class)) + raise ManagerException('Unknown {} class: {}' + .format(_type, _class)) try: return getattr(module, class_name) except AttributeError: self.log.exception('_get_{}_class: Unable to get class %s ' 'from module %s', class_name, module) - raise Exception('Unknown {} class: {}'.format(_type, _class)) + raise ManagerException('Unknown {} class: {}' + .format(_type, _class)) def _build_kwargs(self, source): # Build up the arguments we need to pass to the provider @@ -186,9 +192,9 @@ class Manager(object): v = environ[env_var] except KeyError: self.log.exception('Invalid provider config') - raise Exception('Incorrect provider config, ' - 'missing env var {}' - .format(env_var)) + raise ManagerException('Incorrect provider config, ' + 'missing env var {}' + .format(env_var)) except AttributeError: pass kwargs[k] = v @@ -256,12 +262,14 @@ class Manager(object): try: sources = config['sources'] except KeyError: - raise Exception('Zone {} is missing sources'.format(zone_name)) + raise ManagerException('Zone {} is missing sources' + .format(zone_name)) try: targets = config['targets'] except KeyError: - raise Exception('Zone {} is missing targets'.format(zone_name)) + raise ManagerException('Zone {} is missing targets' + .format(zone_name)) if eligible_targets: targets = list(filter(lambda d: d in eligible_targets, targets)) @@ -276,26 +284,23 @@ class Manager(object): self.log.info('sync: sources=%s -> targets=%s', sources, targets) try: - collected = [] - for source in sources: - collected.append(self.providers[source]) - sources = collected + sources = [self.providers[source] for source in sources] except KeyError: - raise Exception('Zone {}, unknown source: {}'.format(zone_name, - source)) + raise ManagerException('Zone {}, unknown source: {}' + .format(zone_name, source)) try: trgs = [] for target in targets: trg = self.providers[target] if not isinstance(trg, BaseProvider): - raise Exception('{} - "{}" does not support targeting' - .format(trg, target)) + raise ManagerException('{} - "{}" does not support ' + 'targeting'.format(trg, target)) trgs.append(trg) targets = trgs except KeyError: - raise Exception('Zone {}, unknown target: {}'.format(zone_name, - target)) + raise ManagerException('Zone {}, unknown target: {}' + .format(zone_name, target)) futures.append(self._executor.submit(self._populate_and_plan, zone_name, sources, targets)) @@ -348,7 +353,7 @@ class Manager(object): a = [self.providers[source] for source in a] b = [self.providers[source] for source in b] except KeyError as e: - raise Exception('Unknown source: {}'.format(e.args[0])) + raise ManagerException('Unknown source: {}'.format(e.args[0])) sub_zones = self.configured_sub_zones(zone) za = Zone(zone, sub_zones) @@ -374,7 +379,7 @@ class Manager(object): try: sources = [self.providers[s] for s in sources] except KeyError as e: - raise Exception('Unknown source: {}'.format(e.args[0])) + raise ManagerException('Unknown source: {}'.format(e.args[0])) clz = YamlProvider if split: @@ -397,16 +402,14 @@ class Manager(object): try: sources = config['sources'] except KeyError: - raise Exception('Zone {} is missing sources'.format(zone_name)) + raise ManagerException('Zone {} is missing sources' + .format(zone_name)) try: - collected = [] - for source in sources: - collected.append(self.providers[source]) - sources = collected + sources = [self.providers[source] for source in sources] except KeyError: - raise Exception('Zone {}, unknown source: {}'.format(zone_name, - source)) + raise ManagerException('Zone {}, unknown source: {}' + .format(zone_name, source)) for source in sources: if isinstance(source, YamlProvider): diff --git a/tests/test_octodns_manager.py b/tests/test_octodns_manager.py index 60a1922..13eea95 100644 --- a/tests/test_octodns_manager.py +++ b/tests/test_octodns_manager.py @@ -11,7 +11,8 @@ from six import text_type from unittest import TestCase from octodns.record import Record -from octodns.manager import _AggregateTarget, MainThreadExecutor, Manager +from octodns.manager import _AggregateTarget, MainThreadExecutor, Manager, \ + ManagerException from octodns.yaml import safe_load from octodns.zone import Zone @@ -28,77 +29,77 @@ def get_config_filename(which): class TestManager(TestCase): def test_missing_provider_class(self): - with self.assertRaises(Exception) as ctx: + with self.assertRaises(ManagerException) as ctx: Manager(get_config_filename('missing-provider-class.yaml')).sync() self.assertTrue('missing class' in text_type(ctx.exception)) def test_bad_provider_class(self): - with self.assertRaises(Exception) as ctx: + with self.assertRaises(ManagerException) as ctx: Manager(get_config_filename('bad-provider-class.yaml')).sync() self.assertTrue('Unknown provider class' in text_type(ctx.exception)) def test_bad_provider_class_module(self): - with self.assertRaises(Exception) as ctx: + with self.assertRaises(ManagerException) as ctx: Manager(get_config_filename('bad-provider-class-module.yaml')) \ .sync() self.assertTrue('Unknown provider class' in text_type(ctx.exception)) def test_bad_provider_class_no_module(self): - with self.assertRaises(Exception) as ctx: + with self.assertRaises(ManagerException) as ctx: Manager(get_config_filename('bad-provider-class-no-module.yaml')) \ .sync() self.assertTrue('Unknown provider class' in text_type(ctx.exception)) def test_missing_provider_config(self): # Missing provider config - with self.assertRaises(Exception) as ctx: + with self.assertRaises(ManagerException) as ctx: Manager(get_config_filename('missing-provider-config.yaml')).sync() self.assertTrue('provider config' in text_type(ctx.exception)) def test_missing_env_config(self): - with self.assertRaises(Exception) as ctx: + with self.assertRaises(ManagerException) as ctx: Manager(get_config_filename('missing-provider-env.yaml')).sync() self.assertTrue('missing env var' in text_type(ctx.exception)) def test_missing_source(self): - with self.assertRaises(Exception) as ctx: + with self.assertRaises(ManagerException) as ctx: Manager(get_config_filename('provider-problems.yaml')) \ .sync(['missing.sources.']) self.assertTrue('missing sources' in text_type(ctx.exception)) def test_missing_targets(self): - with self.assertRaises(Exception) as ctx: + with self.assertRaises(ManagerException) as ctx: Manager(get_config_filename('provider-problems.yaml')) \ .sync(['missing.targets.']) self.assertTrue('missing targets' in text_type(ctx.exception)) def test_unknown_source(self): - with self.assertRaises(Exception) as ctx: + with self.assertRaises(ManagerException) as ctx: Manager(get_config_filename('provider-problems.yaml')) \ .sync(['unknown.source.']) self.assertTrue('unknown source' in text_type(ctx.exception)) def test_unknown_target(self): - with self.assertRaises(Exception) as ctx: + with self.assertRaises(ManagerException) as ctx: Manager(get_config_filename('provider-problems.yaml')) \ .sync(['unknown.target.']) self.assertTrue('unknown target' in text_type(ctx.exception)) def test_bad_plan_output_class(self): - with self.assertRaises(Exception) as ctx: + with self.assertRaises(ManagerException) as ctx: name = 'bad-plan-output-missing-class.yaml' Manager(get_config_filename(name)).sync() self.assertEquals('plan_output bad is missing class', text_type(ctx.exception)) def test_bad_plan_output_config(self): - with self.assertRaises(Exception) as ctx: + with self.assertRaises(ManagerException) as ctx: Manager(get_config_filename('bad-plan-output-config.yaml')).sync() self.assertEqual('Incorrect plan_output config for bad', text_type(ctx.exception)) def test_source_only_as_a_target(self): - with self.assertRaises(Exception) as ctx: + with self.assertRaises(ManagerException) as ctx: Manager(get_config_filename('provider-problems.yaml')) \ .sync(['not.targetable.']) self.assertTrue('does not support targeting' in @@ -182,7 +183,7 @@ class TestManager(TestCase): 'unit.tests.') self.assertEquals(14, len(changes)) - with self.assertRaises(Exception) as ctx: + with self.assertRaises(ManagerException) as ctx: manager.compare(['nope'], ['dump'], 'unit.tests.') self.assertEquals('Unknown source: nope', text_type(ctx.exception)) @@ -222,7 +223,7 @@ class TestManager(TestCase): environ['YAML_TMP_DIR'] = tmpdir.dirname manager = Manager(get_config_filename('simple.yaml')) - with self.assertRaises(Exception) as ctx: + with self.assertRaises(ManagerException) as ctx: manager.dump('unit.tests.', tmpdir.dirname, False, False, 'nope') self.assertEquals('Unknown source: nope', text_type(ctx.exception)) @@ -251,7 +252,7 @@ class TestManager(TestCase): environ['YAML_TMP_DIR'] = tmpdir.dirname manager = Manager(get_config_filename('simple-split.yaml')) - with self.assertRaises(Exception) as ctx: + with self.assertRaises(ManagerException) as ctx: manager.dump('unit.tests.', tmpdir.dirname, False, True, 'nope') self.assertEquals('Unknown source: nope', text_type(ctx.exception)) @@ -267,12 +268,12 @@ class TestManager(TestCase): def test_validate_configs(self): Manager(get_config_filename('simple-validate.yaml')).validate_configs() - with self.assertRaises(Exception) as ctx: + with self.assertRaises(ManagerException) as ctx: Manager(get_config_filename('missing-sources.yaml')) \ .validate_configs() self.assertTrue('missing sources' in text_type(ctx.exception)) - with self.assertRaises(Exception) as ctx: + with self.assertRaises(ManagerException) as ctx: Manager(get_config_filename('unknown-provider.yaml')) \ .validate_configs() self.assertTrue('unknown source' in text_type(ctx.exception)) From 00fa158c59b243a80901e1802a437eece6a1264c Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Wed, 9 Oct 2019 15:31:27 -0700 Subject: [PATCH 33/49] filter -> [... if]s --- octodns/manager.py | 3 +- octodns/provider/base.py | 2 +- tests/test_octodns_provider_mythicbeasts.py | 12 +++---- tests/test_octodns_provider_yaml.py | 37 +++++++++------------ 4 files changed, 24 insertions(+), 30 deletions(-) diff --git a/octodns/manager.py b/octodns/manager.py index 85de76b..9a1b598 100644 --- a/octodns/manager.py +++ b/octodns/manager.py @@ -271,8 +271,7 @@ class Manager(object): raise ManagerException('Zone {} is missing targets' .format(zone_name)) if eligible_targets: - targets = list(filter(lambda d: d in eligible_targets, - targets)) + targets = [t for t in targets if t in eligible_targets] if not targets: # Don't bother planning (and more importantly populating) zones diff --git a/octodns/provider/base.py b/octodns/provider/base.py index 9f03e78..ae87844 100644 --- a/octodns/provider/base.py +++ b/octodns/provider/base.py @@ -60,7 +60,7 @@ class BaseProvider(BaseSource): # allow the provider to filter out false positives before = len(changes) - changes = list(filter(self._include_change, changes)) + changes = [c for c in changes if self._include_change(c)] after = len(changes) if before != after: self.log.info('plan: filtered out %s changes', before - after) diff --git a/tests/test_octodns_provider_mythicbeasts.py b/tests/test_octodns_provider_mythicbeasts.py index 498b408..960bd65 100644 --- a/tests/test_octodns_provider_mythicbeasts.py +++ b/tests/test_octodns_provider_mythicbeasts.py @@ -441,11 +441,11 @@ class TestMythicBeastsProvider(TestCase): plan = provider.plan(wanted) # Octo ignores NS records (15-1) - self.assertEquals(1, len(list(filter( - lambda u: isinstance(u, Update), plan.changes)))) - self.assertEquals(1, len(list(filter( - lambda d: isinstance(d, Delete), plan.changes)))) - self.assertEquals(14, len(list(filter( - lambda c: isinstance(c, Create), plan.changes)))) + self.assertEquals(1, len([c for c in plan.changes + if isinstance(c, Update)])) + self.assertEquals(1, len([c for c in plan.changes + if isinstance(c, Delete)])) + self.assertEquals(14, len([c for c in plan.changes + if isinstance(c, Create)])) self.assertEquals(16, provider.apply(plan)) self.assertTrue(plan.exists) diff --git a/tests/test_octodns_provider_yaml.py b/tests/test_octodns_provider_yaml.py index e16aaa5..a47dca1 100644 --- a/tests/test_octodns_provider_yaml.py +++ b/tests/test_octodns_provider_yaml.py @@ -58,9 +58,8 @@ class TestYamlProvider(TestCase): # We add everything plan = target.plan(zone) - self.assertEquals(15, len(list(filter(lambda c: - isinstance(c, Create), - plan.changes)))) + self.assertEquals(15, len([c for c in plan.changes + if isinstance(c, Create)])) self.assertFalse(isfile(yaml_file)) # Now actually do it @@ -69,9 +68,8 @@ class TestYamlProvider(TestCase): # Dynamic plan plan = target.plan(dynamic_zone) - self.assertEquals(5, len(list(filter(lambda c: - isinstance(c, Create), - plan.changes)))) + self.assertEquals(5, len([c for c in plan.changes + if isinstance(c, Create)])) self.assertFalse(isfile(dynamic_yaml_file)) # Apply it self.assertEquals(5, target.apply(plan)) @@ -82,17 +80,15 @@ class TestYamlProvider(TestCase): target.populate(reloaded) self.assertDictEqual( {'included': ['test']}, - list(filter( - lambda x: x.name == 'included', reloaded.records - ))[0]._octodns) + [x for x in reloaded.records + if x.name == 'included'][0]._octodns) self.assertFalse(zone.changes(reloaded, target=source)) # A 2nd sync should still create everything plan = target.plan(zone) - self.assertEquals(15, len(list(filter(lambda c: - isinstance(c, Create), - plan.changes)))) + self.assertEquals(15, len([c for c in plan.changes + if isinstance(c, Create)])) with open(yaml_file) as fh: data = safe_load(fh.read()) @@ -255,8 +251,8 @@ class TestSplitYamlProvider(TestCase): # We add everything plan = target.plan(zone) - self.assertEquals(15, len(list(filter( - lambda c: isinstance(c, Create), plan.changes)))) + self.assertEquals(15, len([c for c in plan.changes + if isinstance(c, Create)])) self.assertFalse(isdir(zone_dir)) # Now actually do it @@ -264,8 +260,8 @@ class TestSplitYamlProvider(TestCase): # Dynamic plan plan = target.plan(dynamic_zone) - self.assertEquals(5, len(list(filter( - lambda c: isinstance(c, Create), plan.changes)))) + self.assertEquals(5, len([c for c in plan.changes + if isinstance(c, Create)])) self.assertFalse(isdir(dynamic_zone_dir)) # Apply it self.assertEquals(5, target.apply(plan)) @@ -276,16 +272,15 @@ class TestSplitYamlProvider(TestCase): target.populate(reloaded) self.assertDictEqual( {'included': ['test']}, - list(filter( - lambda x: x.name == 'included', reloaded.records - ))[0]._octodns) + [x for x in reloaded.records + if x.name == 'included'][0]._octodns) self.assertFalse(zone.changes(reloaded, target=source)) # A 2nd sync should still create everything plan = target.plan(zone) - self.assertEquals(15, len(list(filter( - lambda c: isinstance(c, Create), plan.changes)))) + self.assertEquals(15, len([c for c in plan.changes + if isinstance(c, Create)])) yaml_file = join(zone_dir, '$unit.tests.yaml') self.assertTrue(isfile(yaml_file)) From 4d0bc29acc5286e382edf3af72c7c5b8f3656b21 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Wed, 9 Oct 2019 15:36:25 -0700 Subject: [PATCH 34/49] Remove a couple more filters( --- octodns/manager.py | 2 +- octodns/source/tinydns.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/octodns/manager.py b/octodns/manager.py index 9a1b598..1a86336 100644 --- a/octodns/manager.py +++ b/octodns/manager.py @@ -254,7 +254,7 @@ class Manager(object): zones = self.config['zones'].items() if eligible_zones: - zones = filter(lambda d: d[0] in eligible_zones, zones) + zones = [z for z in zones if z[0] in eligible_zones] futures = [] for zone_name, config in zones: diff --git a/octodns/source/tinydns.py b/octodns/source/tinydns.py index 0ee100a..9c44ed8 100755 --- a/octodns/source/tinydns.py +++ b/octodns/source/tinydns.py @@ -253,7 +253,7 @@ class TinyDnsFileSource(TinyDnsBaseSource): # Ignore hidden files continue with open(join(self.directory, filename), 'r') as fh: - lines += filter(lambda l: l, fh.read().split('\n')) + lines += [l for l in fh.read().split('\n') if l] self._cache = lines From 10ad30e7ea4a463838989258428245e0d7c55b1e Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Wed, 9 Oct 2019 15:41:18 -0700 Subject: [PATCH 35/49] map( to [...] --- octodns/provider/dyn.py | 4 ++-- octodns/provider/ovh.py | 3 +-- octodns/zone.py | 4 ++-- 3 files changed, 5 insertions(+), 6 deletions(-) diff --git a/octodns/provider/dyn.py b/octodns/provider/dyn.py index 30ba8bd..f4f8e53 100644 --- a/octodns/provider/dyn.py +++ b/octodns/provider/dyn.py @@ -903,10 +903,10 @@ class DynProvider(BaseProvider): # Sort the values for consistent ordering so that we can compare values = sorted(values, key=_dynamic_value_sort_key) # Ensure that weight is included and if not use the default - values = list(map(lambda v: { + values = [{ 'value': v['value'], 'weight': v.get('weight', 1), - }, values)) + } for v in values] # Walk through our existing pools looking for a match we can use for pool in pools: diff --git a/octodns/provider/ovh.py b/octodns/provider/ovh.py index a0e47f8..17aff8d 100644 --- a/octodns/provider/ovh.py +++ b/octodns/provider/ovh.py @@ -326,8 +326,7 @@ class OvhProvider(BaseProvider): splitted = value.split('\\;') found_key = False for splitted_value in splitted: - sub_split = list(map(lambda x: x.strip(), - splitted_value.split("=", 1))) + sub_split = [x.strip() for x in splitted_value.split("=", 1)] if len(sub_split) < 2: return False key, value = sub_split[0], sub_split[1] diff --git a/octodns/zone.py b/octodns/zone.py index 1191b6f..5f099ac 100644 --- a/octodns/zone.py +++ b/octodns/zone.py @@ -84,8 +84,8 @@ class Zone(object): raise DuplicateRecordException('Duplicate record {}, type {}' .format(record.fqdn, record._type)) - elif not lenient and (((record._type == 'CNAME' and len(node) > 0) or - ('CNAME' in map(lambda r: r._type, node)))): + elif not lenient and ((record._type == 'CNAME' and len(node) > 0) or + ('CNAME' in [r._type for r in node])): # We're adding a CNAME to existing records or adding to an existing # CNAME raise InvalidNodeException('Invalid state, CNAME at {} cannot ' From b5c75d189c7d9b0fe4f30abe8f8e074c1a1e36dd Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Wed, 9 Oct 2019 16:01:39 -0700 Subject: [PATCH 36/49] Convert sources building back out to for x in y from list comprehension --- octodns/manager.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/octodns/manager.py b/octodns/manager.py index 1a86336..14a8f07 100644 --- a/octodns/manager.py +++ b/octodns/manager.py @@ -283,7 +283,10 @@ class Manager(object): self.log.info('sync: sources=%s -> targets=%s', sources, targets) try: - sources = [self.providers[source] for source in sources] + collected = [] + for source in sources: + collected.append(self.providers[source]) + sources = collected except KeyError: raise ManagerException('Zone {}, unknown source: {}' .format(zone_name, source)) @@ -405,7 +408,10 @@ class Manager(object): .format(zone_name)) try: - sources = [self.providers[source] for source in sources] + collected = [] + for source in sources: + collected.append(self.providers[source]) + sources = collected except KeyError: raise ManagerException('Zone {}, unknown source: {}' .format(zone_name, source)) From 6959b58b7533a94de78d49e0db706ee182009339 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Wed, 9 Oct 2019 16:03:06 -0700 Subject: [PATCH 37/49] Update requirements and setup.py, remove incf.countryutils, promote pycountry-convert --- requirements-dev.txt | 2 -- requirements.txt | 6 ++++-- setup.py | 3 ++- 3 files changed, 6 insertions(+), 5 deletions(-) diff --git a/requirements-dev.txt b/requirements-dev.txt index a2833ae..d9888b8 100644 --- a/requirements-dev.txt +++ b/requirements-dev.txt @@ -2,8 +2,6 @@ coverage mock nose pycodestyle==2.4.0 -pycountry>=18.12.8 -pycountry_convert>=0.7.2 pyflakes==1.6.0 readme_renderer[md]==24.0 requests_mock diff --git a/requirements.txt b/requirements.txt index 4d98736..204fd63 100644 --- a/requirements.txt +++ b/requirements.txt @@ -6,8 +6,8 @@ botocore==1.10.5 dnspython==1.15.0 docutils==0.14 dyn==1.8.1 -futures==3.2.0; python_version < '3.0' edgegrid-python==1.1.1 +futures==3.2.0; python_version < '3.0' google-cloud-core==0.28.1 google-cloud-dns==0.29.0 ipaddress==1.0.22 @@ -16,9 +16,11 @@ msrestazure==0.6.2 natsort==5.5.0 ns1-python==0.12.0 ovh==0.4.8 +pycountry-convert==0.7.2 +pycountry==19.8.18 python-dateutil==2.6.1 requests==2.22.0 s3transfer==0.1.13 -six==1.12.0 setuptools==38.5.2 +six==1.12.0 transip==2.0.0 diff --git a/setup.py b/setup.py index 5cb741b..d58ed2c 100644 --- a/setup.py +++ b/setup.py @@ -66,9 +66,10 @@ setup( 'PyYaml>=4.2b1', 'dnspython>=1.15.0', 'futures>=3.2.0', - 'incf.countryutils>=1.0', 'ipaddress>=1.0.22', 'natsort>=5.5.0', + 'pycountry>=19.8.18', + 'pycountry-convert>=0.7.2', # botocore doesn't like >=2.7.0 for some reason 'python-dateutil>=2.6.0,<2.7.0', 'requests>=2.20.0' From 294c2cc9b3f8a1820437a16436ace7c4508f65e1 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Wed, 9 Oct 2019 16:07:03 -0700 Subject: [PATCH 38/49] Update CHANGELOG for python 3 work --- CHANGELOG.md | 26 +++++++++++++++++++++++--- 1 file changed, 23 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e30d0df..7e5e6ac 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,8 +1,28 @@ ## v0.9.9 - 2019-??-?? - Python 3.7 Support -* Route53 _mod_keyer ordering wasn't complete/reliable and in python 3 this - resulted in randomness. This has been addressed and may result in value - reordering on next plan, no actual changes in behavior should occur. +* Extensive pass through the whole codebase to support Python 3 + * Tons of updates to replace `def __cmp__` with `__eq__` and friends to + preserve custom equality and ordering behaviors that are essential to + octoDNS's processes. + * Quite a few objects required the addition of `__eq__` and friends so that + they're sortable in Python 3 now that those things are more strict. A few + places this required jumping through hoops of sorts. Thankfully our tests + are pretty thorough and caught a lot of issues and hopefully the whole + plan, review, apply process will backstop that. + * Explicit ordering of changes by (name, type) to address inconsistent + ordering for a number of providers that just convert changes into API + calls as they come. Python 2 sets ordered consistently, Python 3 they do + not. https://github.com/github/octodns/pull/384/commits/7958233fccf9ea22d95e2fd06c48d7d0a4529e26 + * Route53 _mod_keyer ordering wasn't 100% complete and thus unreliable and + random in Python 3. This has been addressed and may result in value + reordering on next plan, no actual changes in behavior should occur. + * `incf.countryutils` (in pypi) was last released in 2009 is not python 3 + compatible (it's country data is also pretty stale.) `pycountry_convert` + appears to have the functionality required to replace its usage so it has + been removed as a dependency/requirement. + * Bunch of additional unit tests and supporting config to exercise new code + and verify things that were run into during the Python 3 work + * lots of `six`ing of things ## v0.9.8 - 2019-09-30 - One with no changes b/c PyPi description problems From b3bd4382cc6c32c45bf874320a99c241b314c26e Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Mon, 14 Oct 2019 07:32:09 -0700 Subject: [PATCH 39/49] Apply suggestions from code review Co-Authored-By: Theo Julienne --- octodns/manager.py | 4 ++++ octodns/provider/rackspace.py | 2 +- octodns/record/__init__.py | 2 +- 3 files changed, 6 insertions(+), 2 deletions(-) diff --git a/octodns/manager.py b/octodns/manager.py index 14a8f07..bc78f5b 100644 --- a/octodns/manager.py +++ b/octodns/manager.py @@ -283,6 +283,8 @@ class Manager(object): self.log.info('sync: sources=%s -> targets=%s', sources, targets) try: + # rather than using a list comprehension, we break this loop out + # so that the `except` block below can reference the `source` collected = [] for source in sources: collected.append(self.providers[source]) @@ -408,6 +410,8 @@ class Manager(object): .format(zone_name)) try: + # rather than using a list comprehension, we break this loop out + # so that the `except` block below can reference the `source` collected = [] for source in sources: collected.append(self.providers[source]) diff --git a/octodns/provider/rackspace.py b/octodns/provider/rackspace.py index 28b7f05..7fed05b 100644 --- a/octodns/provider/rackspace.py +++ b/octodns/provider/rackspace.py @@ -14,7 +14,7 @@ from .base import BaseProvider def _value_keyer(v): - return '{}-{}-{}'.format(v.get('type', ''), v['name'], v.get('data', '')) + return (v.get('type', ''), v['name'], v.get('data', '')) def add_trailing_dot(s): diff --git a/octodns/record/__init__.py b/octodns/record/__init__.py index 47d2e9b..c0f6482 100644 --- a/octodns/record/__init__.py +++ b/octodns/record/__init__.py @@ -926,7 +926,7 @@ class MxValue(object): } def __hash__(self): - return hash('{} {}'.format(self.preference, self.exchange)) + return hash((self.preference, self.exchange)) def __eq__(self, other): return ((self.preference, self.exchange) == From 25b41a4a92cfde89f157c0e116908b509c63a266 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Mon, 14 Oct 2019 07:47:36 -0700 Subject: [PATCH 40/49] Adopt Route53Provider _equality_tuple suggestion --- octodns/provider/route53.py | 70 +++++++------------------------------ 1 file changed, 12 insertions(+), 58 deletions(-) diff --git a/octodns/provider/route53.py b/octodns/provider/route53.py index 6d2cfeb..decbd4d 100644 --- a/octodns/provider/route53.py +++ b/octodns/provider/route53.py @@ -156,34 +156,29 @@ class _Route53Record(object): 'sub-classes should never use this method' return '{}:{}'.format(self.fqdn, self._type).__hash__() + def _equality_tuple(self): + return (self.__class__.__name__, self.fqdn, self._type) + def __eq__(self, other): '''Sub-classes should call up to this and return its value if true. When it's false they should compute their own __eq__, same for other ordering methods.''' - return self.__class__.__name__ == other.__class__.__name__ and \ - self.fqdn == other.fqdn and \ - self._type == other._type + return self._equality_tuple() == other._equality_tuple() def __ne__(self, other): - return self.__class__.__name__ != other.__class__.__name__ or \ - self.fqdn != other.fqdn or \ - self._type != other._type + return self._equality_tuple() != other._equality_tuple() def __lt__(self, other): - return (((self.__class__.__name__, self.fqdn, self._type)) < - ((other.__class__.__name__, other.fqdn, other._type))) + return self._equality_tuple() < other._equality_tuple() def __le__(self, other): - return (((self.__class__.__name__, self.fqdn, self._type)) <= - ((other.__class__.__name__, other.fqdn, other._type))) + return self._equality_tuple() <= other._equality_tuple() def __gt__(self, other): - return (((self.__class__.__name__, self.fqdn, self._type)) > - ((other.__class__.__name__, other.fqdn, other._type))) + return self._equality_tuple() > other._equality_tuple() def __ge__(self, other): - return (((self.__class__.__name__, self.fqdn, self._type)) >= - ((other.__class__.__name__, other.fqdn, other._type))) + return self._equality_tuple() >= other._equality_tuple() def __repr__(self): return '_Route53Record<{} {} {} {}>'.format(self.fqdn, self._type, @@ -524,50 +519,9 @@ class _Route53GeoRecord(_Route53Record): return '{}:{}:{}'.format(self.fqdn, self._type, self.geo.code).__hash__() - def __eq__(self, other): - return super(_Route53GeoRecord, self).__eq__(other) and \ - self.geo.code == other.geo.code - - def __ne__(self, other): - # super will handle class != class, so if it's true we have 2 geo - # objects with the same name and type, so just need to compare codes - return super(_Route53GeoRecord, self).__ne__(other) or \ - self.geo.code != other.geo.code - - def __lt__(self, other): - # super eq will check class, name, and type - if super(_Route53GeoRecord, self).__eq__(other): - # if it's True we're dealing with two geo's with the same name and - # type, so we just need to compare codes - return self.geo.code < other.geo.code - # Super is not equal so we'll just let it decide lt - return super(_Route53GeoRecord, self).__lt__(other) - - def __le__(self, other): - # super eq will check class, name, and type - if super(_Route53GeoRecord, self).__eq__(other): - # Just need to compare codes, everything else is equal - return self.geo.code <= other.geo.code - # Super is not equal so geo.code doesn't matter, let it decide with lt, - # can't be eq - return super(_Route53GeoRecord, self).__lt__(other) - - def __gt__(self, other): - # super eq will check class, name, and type - if super(_Route53GeoRecord, self).__eq__(other): - # Just need to compare codes, everything else is equal - return self.geo.code > other.geo.code - # Super is not equal so we'll just let it decide gt - return super(_Route53GeoRecord, self).__gt__(other) - - def __ge__(self, other): - # super eq will check class, name, and type - if super(_Route53GeoRecord, self).__eq__(other): - # Just need to compare codes, everything else is equal - return self.geo.code >= other.geo.code - # Super is not equal so geo.code doesn't matter, let it decide with gt, - # can't be eq - return super(_Route53GeoRecord, self).__gt__(other) + def _equality_tuple(self): + return super(_Route53GeoRecord, self)._equality_tuple() + \ + (self.geo.code,) def __repr__(self): return '_Route53GeoRecord<{} {} {} {} {}>'.format(self.fqdn, From b8e2ec124b1783f0e57ea435da0e93ef694eb750 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Mon, 14 Oct 2019 07:48:17 -0700 Subject: [PATCH 41/49] Fix Manager comment wrapping --- octodns/manager.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/octodns/manager.py b/octodns/manager.py index bc78f5b..3c91aa9 100644 --- a/octodns/manager.py +++ b/octodns/manager.py @@ -283,8 +283,9 @@ class Manager(object): self.log.info('sync: sources=%s -> targets=%s', sources, targets) try: - # rather than using a list comprehension, we break this loop out - # so that the `except` block below can reference the `source` + # rather than using a list comprehension, we break this loop + # out so that the `except` block below can reference the + # `source` collected = [] for source in sources: collected.append(self.providers[source]) @@ -410,8 +411,9 @@ class Manager(object): .format(zone_name)) try: - # rather than using a list comprehension, we break this loop out - # so that the `except` block below can reference the `source` + # rather than using a list comprehension, we break this loop + # out so that the `except` block below can reference the + # `source` collected = [] for source in sources: collected.append(self.providers[source]) From 74048bf974a81d0b85802b6bb1da617ecf07cbe7 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Mon, 14 Oct 2019 07:48:47 -0700 Subject: [PATCH 42/49] Use if, else rather than try, except KeyError --- octodns/provider/route53.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/octodns/provider/route53.py b/octodns/provider/route53.py index decbd4d..968d8b8 100644 --- a/octodns/provider/route53.py +++ b/octodns/provider/route53.py @@ -564,9 +564,9 @@ def _mod_keyer(mod): if rrset.get('GeoLocation', False): unique_id = rrset['SetIdentifier'] else: - try: + if 'SetIdentifier' in rrset: unique_id = '{}-{}'.format(rrset['Name'], rrset['SetIdentifier']) - except KeyError: + else: unique_id = rrset['Name'] # Prioritise within the action_priority, ensuring targets come first. From 2b33f95c1759e61698797963c04727cf19f1893e Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Mon, 14 Oct 2019 08:13:07 -0700 Subject: [PATCH 43/49] EqualityTupleMixin impl, use everywhere we were doing tuple compares --- octodns/equality.py | 30 +++++ octodns/provider/route53.py | 26 +---- octodns/record/__init__.py | 204 ++++----------------------------- tests/test_octodns_equality.py | 68 +++++++++++ 4 files changed, 126 insertions(+), 202 deletions(-) create mode 100644 octodns/equality.py create mode 100644 tests/test_octodns_equality.py diff --git a/octodns/equality.py b/octodns/equality.py new file mode 100644 index 0000000..965b8a0 --- /dev/null +++ b/octodns/equality.py @@ -0,0 +1,30 @@ +# +# +# + +from __future__ import absolute_import, division, print_function, \ + unicode_literals + + +class EqualityTupleMixin: + + def _equality_tuple(self): + raise NotImplementedError('_equality_tuple method not implemented') + + def __eq__(self, other): + return self._equality_tuple() == other._equality_tuple() + + def __ne__(self, other): + return self._equality_tuple() != other._equality_tuple() + + def __lt__(self, other): + return self._equality_tuple() < other._equality_tuple() + + def __le__(self, other): + return self._equality_tuple() <= other._equality_tuple() + + def __gt__(self, other): + return self._equality_tuple() > other._equality_tuple() + + def __ge__(self, other): + return self._equality_tuple() >= other._equality_tuple() diff --git a/octodns/provider/route53.py b/octodns/provider/route53.py index 968d8b8..66da6b5 100644 --- a/octodns/provider/route53.py +++ b/octodns/provider/route53.py @@ -16,6 +16,7 @@ import re from six import text_type +from ..equality import EqualityTupleMixin from ..record import Record, Update from ..record.geo import GeoCodes from .base import BaseProvider @@ -29,7 +30,7 @@ def _octal_replace(s): return octal_re.sub(lambda m: chr(int(m.group(1), 8)), s) -class _Route53Record(object): +class _Route53Record(EqualityTupleMixin): @classmethod def _new_dynamic(cls, provider, record, hosted_zone_id, creating): @@ -157,29 +158,10 @@ class _Route53Record(object): return '{}:{}'.format(self.fqdn, self._type).__hash__() def _equality_tuple(self): + '''Sub-classes should call up to this and return its value and add + any additional fields they need to hav considered.''' return (self.__class__.__name__, self.fqdn, self._type) - def __eq__(self, other): - '''Sub-classes should call up to this and return its value if true. - When it's false they should compute their own __eq__, same for other - ordering methods.''' - return self._equality_tuple() == other._equality_tuple() - - def __ne__(self, other): - return self._equality_tuple() != other._equality_tuple() - - def __lt__(self, other): - return self._equality_tuple() < other._equality_tuple() - - def __le__(self, other): - return self._equality_tuple() <= other._equality_tuple() - - def __gt__(self, other): - return self._equality_tuple() > other._equality_tuple() - - def __ge__(self, other): - return self._equality_tuple() >= other._equality_tuple() - def __repr__(self): return '_Route53Record<{} {} {} {}>'.format(self.fqdn, self._type, self.ttl, self.values) diff --git a/octodns/record/__init__.py b/octodns/record/__init__.py index c0f6482..0847018 100644 --- a/octodns/record/__init__.py +++ b/octodns/record/__init__.py @@ -11,6 +11,7 @@ import re from six import string_types, text_type +from ..equality import EqualityTupleMixin from .geo import GeoCodes @@ -76,7 +77,7 @@ class ValidationError(Exception): self.reasons = reasons -class Record(object): +class Record(EqualityTupleMixin): log = getLogger('Record') @classmethod @@ -209,30 +210,15 @@ class Record(object): def __hash__(self): return '{}:{}'.format(self.name, self._type).__hash__() - def __eq__(self, other): - return ((self.name, self._type) == (other.name, other._type)) - - def __ne__(self, other): - return ((self.name, self._type) != (other.name, other._type)) - - def __lt__(self, other): - return ((self.name, self._type) < (other.name, other._type)) - - def __le__(self, other): - return ((self.name, self._type) <= (other.name, other._type)) - - def __gt__(self, other): - return ((self.name, self._type) > (other.name, other._type)) - - def __ge__(self, other): - return ((self.name, self._type) >= (other.name, other._type)) + def _equality_tuple(self): + return (self.name, self._type) def __repr__(self): # Make sure this is always overridden raise NotImplementedError('Abstract base class, __repr__ required') -class GeoValue(object): +class GeoValue(EqualityTupleMixin): geo_re = re.compile(r'^(?P\w\w)(-(?P\w\w)' r'(-(?P\w\w))?)?$') @@ -259,35 +245,9 @@ class GeoValue(object): yield '-'.join(bits) bits.pop() - def __eq__(self, other): - return ((self.continent_code, self.country_code, self.subdivision_code, - self.values) == (other.continent_code, other.country_code, - other.subdivision_code, other.values)) - - def __ne__(self, other): - return ((self.continent_code, self.country_code, self.subdivision_code, - self.values) != (other.continent_code, other.country_code, - other.subdivision_code, other.values)) - - def __lt__(self, other): - return ((self.continent_code, self.country_code, self.subdivision_code, - self.values) < (other.continent_code, other.country_code, - other.subdivision_code, other.values)) - - def __le__(self, other): - return ((self.continent_code, self.country_code, self.subdivision_code, - self.values) <= (other.continent_code, other.country_code, - other.subdivision_code, other.values)) - - def __gt__(self, other): - return ((self.continent_code, self.country_code, self.subdivision_code, - self.values) > (other.continent_code, other.country_code, - other.subdivision_code, other.values)) - - def __ge__(self, other): - return ((self.continent_code, self.country_code, self.subdivision_code, - self.values) >= (other.continent_code, other.country_code, - other.subdivision_code, other.values)) + def _equality_tuple(self): + return (self.continent_code, self.country_code, self.subdivision_code, + self.values) def __repr__(self): return "'Geo {} {} {} {}'".format(self.continent_code, @@ -787,7 +747,7 @@ class AliasRecord(_ValueMixin, Record): _value_type = AliasValue -class CaaValue(object): +class CaaValue(EqualityTupleMixin): # https://tools.ietf.org/html/rfc6844#page-5 @classmethod @@ -826,29 +786,8 @@ class CaaValue(object): 'value': self.value, } - def __eq__(self, other): - return ((self.flags, self.tag, self.value) == - (other.flags, other.tag, other.value)) - - def __ne__(self, other): - return ((self.flags, self.tag, self.value) != - (other.flags, other.tag, other.value)) - - def __lt__(self, other): - return ((self.flags, self.tag, self.value) < - (other.flags, other.tag, other.value)) - - def __le__(self, other): - return ((self.flags, self.tag, self.value) <= - (other.flags, other.tag, other.value)) - - def __gt__(self, other): - return ((self.flags, self.tag, self.value) > - (other.flags, other.tag, other.value)) - - def __ge__(self, other): - return ((self.flags, self.tag, self.value) >= - (other.flags, other.tag, other.value)) + def _equality_tuple(self): + return (self.flags, self.tag, self.value) def __repr__(self): return '{} {} "{}"'.format(self.flags, self.tag, self.value) @@ -872,7 +811,7 @@ class CnameRecord(_DynamicMixin, _ValueMixin, Record): return reasons -class MxValue(object): +class MxValue(EqualityTupleMixin): @classmethod def validate(cls, data, _type): @@ -928,29 +867,8 @@ class MxValue(object): def __hash__(self): return hash((self.preference, self.exchange)) - def __eq__(self, other): - return ((self.preference, self.exchange) == - (other.preference, other.exchange)) - - def __ne__(self, other): - return ((self.preference, self.exchange) != - (other.preference, other.exchange)) - - def __lt__(self, other): - return ((self.preference, self.exchange) < - (other.preference, other.exchange)) - - def __le__(self, other): - return ((self.preference, self.exchange) <= - (other.preference, other.exchange)) - - def __gt__(self, other): - return ((self.preference, self.exchange) > - (other.preference, other.exchange)) - - def __ge__(self, other): - return ((self.preference, self.exchange) >= - (other.preference, other.exchange)) + def _equality_tuple(self): + return (self.preference, self.exchange) def __repr__(self): return "'{} {}'".format(self.preference, self.exchange) @@ -961,7 +879,7 @@ class MxRecord(_ValuesMixin, Record): _value_type = MxValue -class NaptrValue(object): +class NaptrValue(EqualityTupleMixin): VALID_FLAGS = ('S', 'A', 'U', 'P') @classmethod @@ -1023,41 +941,9 @@ class NaptrValue(object): def __hash__(self): return hash(self.__repr__()) - def __eq__(self, other): - return ((self.order, self.preference, self.flags, self.service, - self.regexp, self.replacement) == - (other.order, other.preference, other.flags, other.service, - other.regexp, other.replacement)) - - def __ne__(self, other): - return ((self.order, self.preference, self.flags, self.service, - self.regexp, self.replacement) != - (other.order, other.preference, other.flags, other.service, - other.regexp, other.replacement)) - - def __lt__(self, other): - return ((self.order, self.preference, self.flags, self.service, - self.regexp, self.replacement) < - (other.order, other.preference, other.flags, other.service, - other.regexp, other.replacement)) - - def __le__(self, other): - return ((self.order, self.preference, self.flags, self.service, - self.regexp, self.replacement) <= - (other.order, other.preference, other.flags, other.service, - other.regexp, other.replacement)) - - def __gt__(self, other): - return ((self.order, self.preference, self.flags, self.service, - self.regexp, self.replacement) > - (other.order, other.preference, other.flags, other.service, - other.regexp, other.replacement)) - - def __ge__(self, other): - return ((self.order, self.preference, self.flags, self.service, - self.regexp, self.replacement) >= - (other.order, other.preference, other.flags, other.service, - other.regexp, other.replacement)) + def _equality_tuple(self): + return (self.order, self.preference, self.flags, self.service, + self.regexp, self.replacement) def __repr__(self): flags = self.flags if self.flags is not None else '' @@ -1107,7 +993,7 @@ class PtrRecord(_ValueMixin, Record): _value_type = PtrValue -class SshfpValue(object): +class SshfpValue(EqualityTupleMixin): VALID_ALGORITHMS = (1, 2, 3, 4) VALID_FINGERPRINT_TYPES = (1, 2) @@ -1161,29 +1047,8 @@ class SshfpValue(object): def __hash__(self): return hash(self.__repr__()) - def __eq__(self, other): - return ((self.algorithm, self.fingerprint_type, self.fingerprint) == - (other.algorithm, other.fingerprint_type, other.fingerprint)) - - def __ne__(self, other): - return ((self.algorithm, self.fingerprint_type, self.fingerprint) != - (other.algorithm, other.fingerprint_type, other.fingerprint)) - - def __lt__(self, other): - return ((self.algorithm, self.fingerprint_type, self.fingerprint) < - (other.algorithm, other.fingerprint_type, other.fingerprint)) - - def __le__(self, other): - return ((self.algorithm, self.fingerprint_type, self.fingerprint) <= - (other.algorithm, other.fingerprint_type, other.fingerprint)) - - def __gt__(self, other): - return ((self.algorithm, self.fingerprint_type, self.fingerprint) > - (other.algorithm, other.fingerprint_type, other.fingerprint)) - - def __ge__(self, other): - return ((self.algorithm, self.fingerprint_type, self.fingerprint) >= - (other.algorithm, other.fingerprint_type, other.fingerprint)) + def _equality_tuple(self): + return (self.algorithm, self.fingerprint_type, self.fingerprint) def __repr__(self): return "'{} {} {}'".format(self.algorithm, self.fingerprint_type, @@ -1244,7 +1109,7 @@ class SpfRecord(_ChunkedValuesMixin, Record): _value_type = _ChunkedValue -class SrvValue(object): +class SrvValue(EqualityTupleMixin): @classmethod def validate(cls, data, _type): @@ -1302,29 +1167,8 @@ class SrvValue(object): def __hash__(self): return hash(self.__repr__()) - def __eq__(self, other): - return ((self.priority, self.weight, self.port, self.target) == - (other.priority, other.weight, other.port, other.target)) - - def __ne__(self, other): - return ((self.priority, self.weight, self.port, self.target) != - (other.priority, other.weight, other.port, other.target)) - - def __lt__(self, other): - return ((self.priority, self.weight, self.port, self.target) < - (other.priority, other.weight, other.port, other.target)) - - def __le__(self, other): - return ((self.priority, self.weight, self.port, self.target) <= - (other.priority, other.weight, other.port, other.target)) - - def __gt__(self, other): - return ((self.priority, self.weight, self.port, self.target) > - (other.priority, other.weight, other.port, other.target)) - - def __ge__(self, other): - return ((self.priority, self.weight, self.port, self.target) >= - (other.priority, other.weight, other.port, other.target)) + def _equality_tuple(self): + return (self.priority, self.weight, self.port, self.target) def __repr__(self): return "'{} {} {} {}'".format(self.priority, self.weight, self.port, diff --git a/tests/test_octodns_equality.py b/tests/test_octodns_equality.py new file mode 100644 index 0000000..dcdc460 --- /dev/null +++ b/tests/test_octodns_equality.py @@ -0,0 +1,68 @@ +# +# +# + +from __future__ import absolute_import, division, print_function, \ + unicode_literals + +from unittest import TestCase + +from octodns.equality import EqualityTupleMixin + + +class TestEqualityTupleMixin(TestCase): + + def test_basics(self): + + class Simple(EqualityTupleMixin): + + def __init__(self, a, b, c): + self.a = a + self.b = b + self.c = c + + def _equality_tuple(self): + return (self.a, self.b) + + one = Simple(1, 2, 3) + same = Simple(1, 2, 3) + matches = Simple(1, 2, 'ignored') + doesnt = Simple(2, 3, 4) + + # equality + self.assertEquals(one, one) + self.assertEquals(one, same) + self.assertEquals(same, one) + # only a & c are considered + self.assertEquals(one, matches) + self.assertEquals(matches, one) + self.assertNotEquals(one, doesnt) + self.assertNotEquals(doesnt, one) + + # lt + self.assertTrue(one < doesnt) + self.assertFalse(doesnt < one) + self.assertFalse(one < same) + + # le + self.assertTrue(one <= doesnt) + self.assertFalse(doesnt <= one) + self.assertTrue(one <= same) + + # gt + self.assertFalse(one > doesnt) + self.assertTrue(doesnt > one) + self.assertFalse(one > same) + + # ge + self.assertFalse(one >= doesnt) + self.assertTrue(doesnt >= one) + self.assertTrue(one >= same) + + def test_not_implemented(self): + + class MissingMethod(EqualityTupleMixin): + pass + + with self.assertRaises(NotImplementedError): + MissingMethod() == MissingMethod() From e1daccc0b762181f96fbc86cd0b300d2f36655ac Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Mon, 14 Oct 2019 08:14:30 -0700 Subject: [PATCH 44/49] Update CHANGELOG.md Co-Authored-By: Theo Julienne --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7e5e6ac..503e53c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,7 +13,7 @@ ordering for a number of providers that just convert changes into API calls as they come. Python 2 sets ordered consistently, Python 3 they do not. https://github.com/github/octodns/pull/384/commits/7958233fccf9ea22d95e2fd06c48d7d0a4529e26 - * Route53 _mod_keyer ordering wasn't 100% complete and thus unreliable and + * Route53 `_mod_keyer` ordering wasn't 100% complete and thus unreliable and random in Python 3. This has been addressed and may result in value reordering on next plan, no actual changes in behavior should occur. * `incf.countryutils` (in pypi) was last released in 2009 is not python 3 From 759c44f35be46f50e87ed9dc7022c177504a4ecc Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Mon, 14 Oct 2019 08:39:45 -0700 Subject: [PATCH 45/49] EqualityTupleMixin needs an explicit inhert from object to make 2.7 happy --- octodns/equality.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/octodns/equality.py b/octodns/equality.py index 965b8a0..bd22c7d 100644 --- a/octodns/equality.py +++ b/octodns/equality.py @@ -6,7 +6,7 @@ from __future__ import absolute_import, division, print_function, \ unicode_literals -class EqualityTupleMixin: +class EqualityTupleMixin(object): def _equality_tuple(self): raise NotImplementedError('_equality_tuple method not implemented') From 4e09f8a838e767aaefa3886ec8208dce9345ab37 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Mon, 14 Oct 2019 19:07:12 -0700 Subject: [PATCH 46/49] Use six.StringIO in setup.py --- setup.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/setup.py b/setup.py index d58ed2c..362db58 100644 --- a/setup.py +++ b/setup.py @@ -1,6 +1,6 @@ #!/usr/bin/env python -from StringIO import StringIO +from six import StringIO from os.path import dirname, join import octodns From 2f45cbc086d11f74c91e4b50da9224c6079849a5 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Mon, 14 Oct 2019 19:10:47 -0700 Subject: [PATCH 47/49] No six for setup.py, try/except both options --- setup.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/setup.py b/setup.py index 362db58..6ae8fd5 100644 --- a/setup.py +++ b/setup.py @@ -1,6 +1,9 @@ #!/usr/bin/env python -from six import StringIO +try: + from io import StringIO +except ImportError: + from StringIO import StringIO from os.path import dirname, join import octodns From 4a41c98c1681ebad6c8b9630301284fd4df25d01 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Mon, 14 Oct 2019 19:14:38 -0700 Subject: [PATCH 48/49] setup.py install_requires futures only on 2.7 --- setup.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/setup.py b/setup.py index 6ae8fd5..b6c2cc4 100644 --- a/setup.py +++ b/setup.py @@ -68,7 +68,7 @@ setup( install_requires=[ 'PyYaml>=4.2b1', 'dnspython>=1.15.0', - 'futures>=3.2.0', + 'futures>=3.2.0; python_version<"3.2"', 'ipaddress>=1.0.22', 'natsort>=5.5.0', 'pycountry>=19.8.18', From 6f9842301ed4e21b80bd36dfb1ac96dbb98c7d4d Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Mon, 14 Oct 2019 19:31:04 -0700 Subject: [PATCH 49/49] Prefer StringIO.StringIO over io. --- setup.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/setup.py b/setup.py index b6c2cc4..4f28232 100644 --- a/setup.py +++ b/setup.py @@ -1,9 +1,9 @@ #!/usr/bin/env python try: - from io import StringIO -except ImportError: from StringIO import StringIO +except ImportError: + from io import StringIO from os.path import dirname, join import octodns