From 1e74d59a76791f6d5f9c610a10166f7cac400fca Mon Sep 17 00:00:00 2001
From: Ross McFarland <ross@github.com>
Date: Mon, 9 Dec 2019 06:35:31 -0800
Subject: [PATCH 01/56] Bump ns1-python to 0.13.0

---
 requirements.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/requirements.txt b/requirements.txt
index 66fa130..6a26ad3 100644
--- a/requirements.txt
+++ b/requirements.txt
@@ -14,7 +14,7 @@ ipaddress==1.0.22
 jmespath==0.9.3
 msrestazure==0.6.2
 natsort==5.5.0
-ns1-python==0.12.0
+ns1-python==0.13.0
 ovh==0.4.8
 pycountry-convert==0.7.2
 pycountry==19.8.18

From f599d91902ab3f8e5b09638efd6872e772657d93 Mon Sep 17 00:00:00 2001
From: Ross McFarland <ross@github.com>
Date: Mon, 9 Dec 2019 08:09:41 -0800
Subject: [PATCH 02/56] Update ns1 to 0.13.0 and use newer API methods

Results in lots of churn in the tests, but actually for the better as it
doesn't have to jump through nearly as many hoops to mock things now.
---
 octodns/provider/ns1.py            |  57 +++---
 tests/test_octodns_provider_ns1.py | 285 +++++++++++++++--------------
 2 files changed, 177 insertions(+), 165 deletions(-)

diff --git a/octodns/provider/ns1.py b/octodns/provider/ns1.py
index b737a19..19b33af 100644
--- a/octodns/provider/ns1.py
+++ b/octodns/provider/ns1.py
@@ -38,7 +38,9 @@ class Ns1Provider(BaseProvider):
         self.log = getLogger('Ns1Provider[{}]'.format(id))
         self.log.debug('__init__: id=%s, api_key=***', id)
         super(Ns1Provider, self).__init__(id, *args, **kwargs)
-        self._client = NS1(apiKey=api_key)
+        client = NS1(apiKey=api_key)
+        self._records = client.records()
+        self._zones = client.zones()
 
     def _data_for_A(self, _type, record):
         # record meta (which would include geo information is only
@@ -189,18 +191,29 @@ class Ns1Provider(BaseProvider):
                        target, lenient)
 
         try:
-            nsone_zone = self._client.loadZone(zone.name[:-1])
-            records = nsone_zone.data['records']
+            nsone_zone_name = zone.name[:-1]
+            nsone_zone = self._zones.retrieve(nsone_zone_name)
+
+            records = []
+            geo_records = []
 
             # change answers for certain types to always be absolute
-            for record in records:
+            for record in nsone_zone['records']:
                 if record['type'] in ['ALIAS', 'CNAME', 'MX', 'NS', 'PTR',
                                       'SRV']:
                     for i, a in enumerate(record['short_answers']):
                         if not a.endswith('.'):
                             record['short_answers'][i] = '{}.'.format(a)
 
-            geo_records = nsone_zone.search(has_geo=True)
+                if record.get('tier', 1) > 1:
+                    # Need to get the full record data for geo records
+                    record = self._records.retrieve(nsone_zone_name,
+                                                    record['domain'],
+                                                    record['type'])
+                    geo_records.append(record)
+                else:
+                    records.append(record)
+
             exists = True
         except ResourceException as e:
             if e.message != self.ZONE_NOT_FOUND_MESSAGE:
@@ -299,53 +312,49 @@ class Ns1Provider(BaseProvider):
                   for v in record.values]
         return {'answers': values, 'ttl': record.ttl}
 
-    def _get_name(self, record):
-        return record.fqdn[:-1] if record.name == '' else record.name
-
     def _apply_Create(self, nsone_zone, change):
         new = change.new
-        name = self._get_name(new)
+        zone = new.zone.name[:-1]
+        domain = new.fqdn[:-1]
         _type = new._type
         params = getattr(self, '_params_for_{}'.format(_type))(new)
-        meth = getattr(nsone_zone, 'add_{}'.format(_type))
         try:
-            meth(name, **params)
+            self._records.create(zone, domain, _type, **params)
         except RateLimitException as e:
             period = float(e.period)
             self.log.warn('_apply_Create: rate limit encountered, pausing '
                           'for %ds and trying again', period)
             sleep(period)
-            meth(name, **params)
+            self._records.create(zone, domain, _type, **params)
 
     def _apply_Update(self, nsone_zone, change):
-        existing = change.existing
-        name = self._get_name(existing)
-        _type = existing._type
-        record = nsone_zone.loadRecord(name, _type)
         new = change.new
+        zone = new.zone.name[:-1]
+        domain = new.fqdn[:-1]
+        _type = new._type
         params = getattr(self, '_params_for_{}'.format(_type))(new)
         try:
-            record.update(**params)
+            self._records.update(zone, domain, _type, **params)
         except RateLimitException as e:
             period = float(e.period)
             self.log.warn('_apply_Update: rate limit encountered, pausing '
                           'for %ds and trying again', period)
             sleep(period)
-            record.update(**params)
+            self._records.update(zone, domain, _type, **params)
 
     def _apply_Delete(self, nsone_zone, change):
         existing = change.existing
-        name = self._get_name(existing)
+        zone = existing.zone.name[:-1]
+        domain = existing.fqdn[:-1]
         _type = existing._type
-        record = nsone_zone.loadRecord(name, _type)
         try:
-            record.delete()
+            self._records.delete(zone, domain, _type)
         except RateLimitException as e:
             period = float(e.period)
             self.log.warn('_apply_Delete: rate limit encountered, pausing '
                           'for %ds and trying again', period)
             sleep(period)
-            record.delete()
+            self._records.delete(zone, domain, _type)
 
     def _apply(self, plan):
         desired = plan.desired
@@ -355,12 +364,12 @@ class Ns1Provider(BaseProvider):
 
         domain_name = desired.name[:-1]
         try:
-            nsone_zone = self._client.loadZone(domain_name)
+            nsone_zone = self._zones.retrieve(domain_name)
         except ResourceException as e:
             if e.message != self.ZONE_NOT_FOUND_MESSAGE:
                 raise
             self.log.debug('_apply:   no matching zone, creating')
-            nsone_zone = self._client.createZone(domain_name)
+            nsone_zone = self._zones.create(domain_name)
 
         for change in changes:
             class_name = change.__class__.__name__
diff --git a/tests/test_octodns_provider_ns1.py b/tests/test_octodns_provider_ns1.py
index 7ef182c..9034552 100644
--- a/tests/test_octodns_provider_ns1.py
+++ b/tests/test_octodns_provider_ns1.py
@@ -6,7 +6,7 @@ from __future__ import absolute_import, division, print_function, \
     unicode_literals
 
 from collections import defaultdict
-from mock import Mock, call, patch
+from mock import call, patch
 from ns1.rest.errors import AuthException, RateLimitException, \
     ResourceException
 from unittest import TestCase
@@ -16,14 +16,6 @@ from octodns.provider.ns1 import Ns1Provider
 from octodns.zone import Zone
 
 
-class DummyZone(object):
-
-    def __init__(self, records):
-        self.data = {
-            'records': records
-        }
-
-
 class TestNs1Provider(TestCase):
     zone = Zone('unit.tests.', [])
     expected = set()
@@ -172,43 +164,40 @@ class TestNs1Provider(TestCase):
         'domain': 'unit.tests.',
     }]
 
-    @patch('ns1.NS1.loadZone')
-    def test_populate(self, load_mock):
+    @patch('ns1.rest.zones.Zones.retrieve')
+    def test_populate(self, zone_retrieve_mock):
         provider = Ns1Provider('test', 'api-key')
 
         # Bad auth
-        load_mock.side_effect = AuthException('unauthorized')
+        zone_retrieve_mock.side_effect = AuthException('unauthorized')
         zone = Zone('unit.tests.', [])
         with self.assertRaises(AuthException) as ctx:
             provider.populate(zone)
-        self.assertEquals(load_mock.side_effect, ctx.exception)
+        self.assertEquals(zone_retrieve_mock.side_effect, ctx.exception)
 
         # General error
-        load_mock.reset_mock()
-        load_mock.side_effect = ResourceException('boom')
+        zone_retrieve_mock.reset_mock()
+        zone_retrieve_mock.side_effect = ResourceException('boom')
         zone = Zone('unit.tests.', [])
         with self.assertRaises(ResourceException) as ctx:
             provider.populate(zone)
-        self.assertEquals(load_mock.side_effect, ctx.exception)
-        self.assertEquals(('unit.tests',), load_mock.call_args[0])
+        self.assertEquals(zone_retrieve_mock.side_effect, ctx.exception)
+        self.assertEquals(('unit.tests',), zone_retrieve_mock.call_args[0])
 
         # Non-existent zone doesn't populate anything
-        load_mock.reset_mock()
-        load_mock.side_effect = \
+        zone_retrieve_mock.reset_mock()
+        zone_retrieve_mock.side_effect = \
             ResourceException('server error: zone not found')
         zone = Zone('unit.tests.', [])
         exists = provider.populate(zone)
         self.assertEquals(set(), zone.records)
-        self.assertEquals(('unit.tests',), load_mock.call_args[0])
+        self.assertEquals(('unit.tests',), zone_retrieve_mock.call_args[0])
         self.assertFalse(exists)
 
         # Existing zone w/o records
-        load_mock.reset_mock()
-        nsone_zone = DummyZone([])
-        load_mock.side_effect = [nsone_zone]
-        zone_search = Mock()
-        zone_search.return_value = [
-            {
+        zone_retrieve_mock.reset_mock()
+        nsone_zone = {
+            'records': [{
                 "domain": "geo.unit.tests",
                 "zone": "unit.tests",
                 "type": "A",
@@ -222,21 +211,18 @@ class TestNs1Provider(TestCase):
                      'meta': {'iso_region_code': ['NA-US-WA']}},
                 ],
                 'ttl': 34,
-            },
-        ]
-        nsone_zone.search = zone_search
+            }],
+        }
+        zone_retrieve_mock.side_effect = [nsone_zone]
         zone = Zone('unit.tests.', [])
         provider.populate(zone)
         self.assertEquals(1, len(zone.records))
-        self.assertEquals(('unit.tests',), load_mock.call_args[0])
+        self.assertEquals(('unit.tests',), zone_retrieve_mock.call_args[0])
 
         # Existing zone w/records
-        load_mock.reset_mock()
-        nsone_zone = DummyZone(self.nsone_records)
-        load_mock.side_effect = [nsone_zone]
-        zone_search = Mock()
-        zone_search.return_value = [
-            {
+        zone_retrieve_mock.reset_mock()
+        nsone_zone = {
+            'records': self.nsone_records + [{
                 "domain": "geo.unit.tests",
                 "zone": "unit.tests",
                 "type": "A",
@@ -250,26 +236,23 @@ class TestNs1Provider(TestCase):
                      'meta': {'iso_region_code': ['NA-US-WA']}},
                 ],
                 'ttl': 34,
-            },
-        ]
-        nsone_zone.search = zone_search
+            }],
+        }
+        zone_retrieve_mock.side_effect = [nsone_zone]
         zone = Zone('unit.tests.', [])
         provider.populate(zone)
         self.assertEquals(self.expected, zone.records)
-        self.assertEquals(('unit.tests',), load_mock.call_args[0])
+        self.assertEquals(('unit.tests',), zone_retrieve_mock.call_args[0])
 
         # Test skipping unsupported record type
-        load_mock.reset_mock()
-        nsone_zone = DummyZone(self.nsone_records + [{
-            'type': 'UNSUPPORTED',
-            'ttl': 42,
-            'short_answers': ['unsupported'],
-            'domain': 'unsupported.unit.tests.',
-        }])
-        load_mock.side_effect = [nsone_zone]
-        zone_search = Mock()
-        zone_search.return_value = [
-            {
+        zone_retrieve_mock.reset_mock()
+        nsone_zone = {
+            'records': self.nsone_records + [{
+                'type': 'UNSUPPORTED',
+                'ttl': 42,
+                'short_answers': ['unsupported'],
+                'domain': 'unsupported.unit.tests.',
+            }, {
                 "domain": "geo.unit.tests",
                 "zone": "unit.tests",
                 "type": "A",
@@ -283,17 +266,23 @@ class TestNs1Provider(TestCase):
                      'meta': {'iso_region_code': ['NA-US-WA']}},
                 ],
                 'ttl': 34,
-            },
-        ]
-        nsone_zone.search = zone_search
+            }],
+        }
+        zone_retrieve_mock.side_effect = [nsone_zone]
         zone = Zone('unit.tests.', [])
         provider.populate(zone)
         self.assertEquals(self.expected, zone.records)
-        self.assertEquals(('unit.tests',), load_mock.call_args[0])
+        self.assertEquals(('unit.tests',), zone_retrieve_mock.call_args[0])
 
-    @patch('ns1.NS1.createZone')
-    @patch('ns1.NS1.loadZone')
-    def test_sync(self, load_mock, create_mock):
+    @patch('ns1.rest.records.Records.delete')
+    @patch('ns1.rest.records.Records.update')
+    @patch('ns1.rest.records.Records.create')
+    @patch('ns1.rest.records.Records.retrieve')
+    @patch('ns1.rest.zones.Zones.create')
+    @patch('ns1.rest.zones.Zones.retrieve')
+    def test_sync(self, zone_retrieve_mock, zone_create_mock,
+                  record_retrieve_mock, record_create_mock,
+                  record_update_mock, record_delete_mock):
         provider = Ns1Provider('test', 'api-key')
 
         desired = Zone('unit.tests.', [])
@@ -307,71 +296,98 @@ class TestNs1Provider(TestCase):
         self.assertTrue(plan.exists)
 
         # Fails, general error
-        load_mock.reset_mock()
-        create_mock.reset_mock()
-        load_mock.side_effect = ResourceException('boom')
+        zone_retrieve_mock.reset_mock()
+        zone_create_mock.reset_mock()
+        zone_retrieve_mock.side_effect = ResourceException('boom')
         with self.assertRaises(ResourceException) as ctx:
             provider.apply(plan)
-        self.assertEquals(load_mock.side_effect, ctx.exception)
+        self.assertEquals(zone_retrieve_mock.side_effect, ctx.exception)
 
         # Fails, bad auth
-        load_mock.reset_mock()
-        create_mock.reset_mock()
-        load_mock.side_effect = \
+        zone_retrieve_mock.reset_mock()
+        zone_create_mock.reset_mock()
+        zone_retrieve_mock.side_effect = \
             ResourceException('server error: zone not found')
-        create_mock.side_effect = AuthException('unauthorized')
+        zone_create_mock.side_effect = AuthException('unauthorized')
         with self.assertRaises(AuthException) as ctx:
             provider.apply(plan)
-        self.assertEquals(create_mock.side_effect, ctx.exception)
+        self.assertEquals(zone_create_mock.side_effect, ctx.exception)
 
         # non-existent zone, create
-        load_mock.reset_mock()
-        create_mock.reset_mock()
-        load_mock.side_effect = \
+        zone_retrieve_mock.reset_mock()
+        zone_create_mock.reset_mock()
+        zone_retrieve_mock.side_effect = \
             ResourceException('server error: zone not found')
-        # ugh, need a mock zone with a mock prop since we're using getattr, we
-        # can actually control side effects on `meth` with that.
-        mock_zone = Mock()
-        mock_zone.add_SRV = Mock()
-        mock_zone.add_SRV.side_effect = [
+
+        zone_create_mock.side_effect = ['foo']
+        # Test out the create rate-limit handling, then 9 successes
+        record_create_mock.side_effect = [
             RateLimitException('boo', period=0),
-            None,
-        ]
-        create_mock.side_effect = [mock_zone]
+        ] + ([None] * 9)
+
         got_n = provider.apply(plan)
         self.assertEquals(expected_n, got_n)
 
+        # Zone was created
+        zone_create_mock.assert_has_calls([call('unit.tests')])
+        # Checking that we got some of the expected records too
+        record_create_mock.assert_has_calls([
+            call('unit.tests', 'unit.tests', 'A', answers=[
+                {'answer': ['1.2.3.4'], 'meta': {}}
+            ], filters=[], ttl=32),
+            call('unit.tests', 'unit.tests', 'CAA', answers=[
+                (0, 'issue', 'ca.unit.tests')
+            ], ttl=40),
+            call('unit.tests', 'unit.tests', 'MX', answers=[
+                (10, 'mx1.unit.tests.'), (20, 'mx2.unit.tests.')
+            ], ttl=35),
+        ])
+
         # Update & delete
-        load_mock.reset_mock()
-        create_mock.reset_mock()
-        nsone_zone = DummyZone(self.nsone_records + [{
-            'type': 'A',
-            'ttl': 42,
-            'short_answers': ['9.9.9.9'],
-            'domain': 'delete-me.unit.tests.',
-        }])
-        nsone_zone.data['records'][0]['short_answers'][0] = '2.2.2.2'
-        nsone_zone.loadRecord = Mock()
-        zone_search = Mock()
-        zone_search.return_value = [
-            {
+        zone_retrieve_mock.reset_mock()
+        zone_create_mock.reset_mock()
+
+        nsone_zone = {
+            'records': self.nsone_records + [{
+                'type': 'A',
+                'ttl': 42,
+                'short_answers': ['9.9.9.9'],
+                'domain': 'delete-me.unit.tests.',
+            }, {
                 "domain": "geo.unit.tests",
                 "zone": "unit.tests",
                 "type": "A",
-                "answers": [
-                    {'answer': ['1.1.1.1'], 'meta': {}},
-                    {'answer': ['1.2.3.4'],
-                     'meta': {'ca_province': ['ON']}},
-                    {'answer': ['2.3.4.5'], 'meta': {'us_state': ['NY']}},
-                    {'answer': ['3.4.5.6'], 'meta': {'country': ['US']}},
-                    {'answer': ['4.5.6.7'],
-                     'meta': {'iso_region_code': ['NA-US-WA']}},
+                "short_answers": [
+                    '1.1.1.1',
+                    '1.2.3.4',
+                    '2.3.4.5',
+                    '3.4.5.6',
+                    '4.5.6.7',
                 ],
+                'tier': 3,  # This flags it as advacned, full load required
                 'ttl': 34,
-            },
-        ]
-        nsone_zone.search = zone_search
-        load_mock.side_effect = [nsone_zone, nsone_zone]
+            }],
+        }
+        nsone_zone['records'][0]['short_answers'][0] = '2.2.2.2'
+
+        record_retrieve_mock.side_effect = [{
+            "domain": "geo.unit.tests",
+            "zone": "unit.tests",
+            "type": "A",
+            "answers": [
+                {'answer': ['1.1.1.1'], 'meta': {}},
+                {'answer': ['1.2.3.4'],
+                 'meta': {'ca_province': ['ON']}},
+                {'answer': ['2.3.4.5'], 'meta': {'us_state': ['NY']}},
+                {'answer': ['3.4.5.6'], 'meta': {'country': ['US']}},
+                {'answer': ['4.5.6.7'],
+                 'meta': {'iso_region_code': ['NA-US-WA']}},
+            ],
+            'tier': 3,
+            'ttl': 34,
+        }]
+
+        zone_retrieve_mock.side_effect = [nsone_zone, nsone_zone]
         plan = provider.plan(desired)
         self.assertEquals(3, len(plan.changes))
         # Shouldn't rely on order so just count classes
@@ -380,55 +396,42 @@ class TestNs1Provider(TestCase):
             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
-        mock_record = Mock()
-        mock_record.update.side_effect = [
+
+        record_update_mock.side_effect = [
             RateLimitException('one', period=0),
             None,
             None,
         ]
-        mock_record.delete.side_effect = [
+        record_delete_mock.side_effect = [
             RateLimitException('two', period=0),
             None,
             None,
         ]
-        nsone_zone.loadRecord.side_effect = [mock_record, mock_record,
-                                             mock_record]
+
         got_n = provider.apply(plan)
         self.assertEquals(3, got_n)
-        nsone_zone.loadRecord.assert_has_calls([
-            call('unit.tests', u'A'),
-            call('delete-me', u'A'),
-            call('geo', u'A'),
-        ])
-        mock_record.assert_has_calls([
-            call.update(answers=[{'answer': [u'1.2.3.4'], 'meta': {}}],
-                        filters=[],
-                        ttl=32),
-            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': {}},
-                    {u'answer': [u'101.102.103.105'], u'meta': {}},
-                    {
-                        u'answer': [u'201.202.203.204'],
-                        u'meta': {
-                            u'iso_region_code': [u'NA-US-NY']
-                        },
-                    },
-                ],
+
+        record_update_mock.assert_has_calls([
+            call('unit.tests', 'unit.tests', 'A', answers=[
+                {'answer': ['1.2.3.4'], 'meta': {}}],
+                filters=[],
+                ttl=32),
+            call('unit.tests', 'unit.tests', 'A', answers=[
+                {'answer': ['1.2.3.4'], 'meta': {}}],
+                filters=[],
+                ttl=32),
+            call('unit.tests', 'geo.unit.tests', 'A', answers=[
+                {'answer': ['101.102.103.104'], 'meta': {}},
+                {'answer': ['101.102.103.105'], 'meta': {}},
+                {
+                    'answer': ['201.202.203.204'],
+                    'meta': {'iso_region_code': ['NA-US-NY']}
+                }],
                 filters=[
-                    {u'filter': u'shuffle', u'config': {}},
-                    {u'filter': u'geotarget_country', u'config': {}},
-                    {u'filter': u'select_first_n', u'config': {u'N': 1}},
-                ],
-                ttl=34),
+                    {'filter': 'shuffle', 'config': {}},
+                    {'filter': 'geotarget_country', 'config': {}},
+                    {'filter': 'select_first_n', 'config': {'N': 1}}],
+                ttl=34)
         ])
 
     def test_escaping(self):

From c4987f1a09048b6b557ce19bc194ee6598a175fe Mon Sep 17 00:00:00 2001
From: Ross McFarland <ross@github.com>
Date: Mon, 9 Dec 2019 08:13:09 -0800
Subject: [PATCH 03/56] s/nsone/ns1/g

---
 octodns/provider/ns1.py            | 22 +++++++++++-----------
 tests/test_octodns_provider_ns1.py | 30 +++++++++++++++---------------
 2 files changed, 26 insertions(+), 26 deletions(-)

diff --git a/octodns/provider/ns1.py b/octodns/provider/ns1.py
index 19b33af..cf78241 100644
--- a/octodns/provider/ns1.py
+++ b/octodns/provider/ns1.py
@@ -23,7 +23,7 @@ class Ns1Provider(BaseProvider):
     '''
     Ns1 provider
 
-    nsone:
+    ns1:
         class: octodns.provider.ns1.Ns1Provider
         api_key: env/NS1_API_KEY
     '''
@@ -191,14 +191,14 @@ class Ns1Provider(BaseProvider):
                        target, lenient)
 
         try:
-            nsone_zone_name = zone.name[:-1]
-            nsone_zone = self._zones.retrieve(nsone_zone_name)
+            ns1_zone_name = zone.name[:-1]
+            ns1_zone = self._zones.retrieve(ns1_zone_name)
 
             records = []
             geo_records = []
 
             # change answers for certain types to always be absolute
-            for record in nsone_zone['records']:
+            for record in ns1_zone['records']:
                 if record['type'] in ['ALIAS', 'CNAME', 'MX', 'NS', 'PTR',
                                       'SRV']:
                     for i, a in enumerate(record['short_answers']):
@@ -207,7 +207,7 @@ class Ns1Provider(BaseProvider):
 
                 if record.get('tier', 1) > 1:
                     # Need to get the full record data for geo records
-                    record = self._records.retrieve(nsone_zone_name,
+                    record = self._records.retrieve(ns1_zone_name,
                                                     record['domain'],
                                                     record['type'])
                     geo_records.append(record)
@@ -312,7 +312,7 @@ class Ns1Provider(BaseProvider):
                   for v in record.values]
         return {'answers': values, 'ttl': record.ttl}
 
-    def _apply_Create(self, nsone_zone, change):
+    def _apply_Create(self, ns1_zone, change):
         new = change.new
         zone = new.zone.name[:-1]
         domain = new.fqdn[:-1]
@@ -327,7 +327,7 @@ class Ns1Provider(BaseProvider):
             sleep(period)
             self._records.create(zone, domain, _type, **params)
 
-    def _apply_Update(self, nsone_zone, change):
+    def _apply_Update(self, ns1_zone, change):
         new = change.new
         zone = new.zone.name[:-1]
         domain = new.fqdn[:-1]
@@ -342,7 +342,7 @@ class Ns1Provider(BaseProvider):
             sleep(period)
             self._records.update(zone, domain, _type, **params)
 
-    def _apply_Delete(self, nsone_zone, change):
+    def _apply_Delete(self, ns1_zone, change):
         existing = change.existing
         zone = existing.zone.name[:-1]
         domain = existing.fqdn[:-1]
@@ -364,14 +364,14 @@ class Ns1Provider(BaseProvider):
 
         domain_name = desired.name[:-1]
         try:
-            nsone_zone = self._zones.retrieve(domain_name)
+            ns1_zone = self._zones.retrieve(domain_name)
         except ResourceException as e:
             if e.message != self.ZONE_NOT_FOUND_MESSAGE:
                 raise
             self.log.debug('_apply:   no matching zone, creating')
-            nsone_zone = self._zones.create(domain_name)
+            ns1_zone = self._zones.create(domain_name)
 
         for change in changes:
             class_name = change.__class__.__name__
-            getattr(self, '_apply_{}'.format(class_name))(nsone_zone,
+            getattr(self, '_apply_{}'.format(class_name))(ns1_zone,
                                                           change)
diff --git a/tests/test_octodns_provider_ns1.py b/tests/test_octodns_provider_ns1.py
index 9034552..0f23222 100644
--- a/tests/test_octodns_provider_ns1.py
+++ b/tests/test_octodns_provider_ns1.py
@@ -108,7 +108,7 @@ class TestNs1Provider(TestCase):
         },
     }))
 
-    nsone_records = [{
+    ns1_records = [{
         'type': 'A',
         'ttl': 32,
         'short_answers': ['1.2.3.4'],
@@ -196,7 +196,7 @@ class TestNs1Provider(TestCase):
 
         # Existing zone w/o records
         zone_retrieve_mock.reset_mock()
-        nsone_zone = {
+        ns1_zone = {
             'records': [{
                 "domain": "geo.unit.tests",
                 "zone": "unit.tests",
@@ -213,7 +213,7 @@ class TestNs1Provider(TestCase):
                 'ttl': 34,
             }],
         }
-        zone_retrieve_mock.side_effect = [nsone_zone]
+        zone_retrieve_mock.side_effect = [ns1_zone]
         zone = Zone('unit.tests.', [])
         provider.populate(zone)
         self.assertEquals(1, len(zone.records))
@@ -221,8 +221,8 @@ class TestNs1Provider(TestCase):
 
         # Existing zone w/records
         zone_retrieve_mock.reset_mock()
-        nsone_zone = {
-            'records': self.nsone_records + [{
+        ns1_zone = {
+            'records': self.ns1_records + [{
                 "domain": "geo.unit.tests",
                 "zone": "unit.tests",
                 "type": "A",
@@ -238,7 +238,7 @@ class TestNs1Provider(TestCase):
                 'ttl': 34,
             }],
         }
-        zone_retrieve_mock.side_effect = [nsone_zone]
+        zone_retrieve_mock.side_effect = [ns1_zone]
         zone = Zone('unit.tests.', [])
         provider.populate(zone)
         self.assertEquals(self.expected, zone.records)
@@ -246,8 +246,8 @@ class TestNs1Provider(TestCase):
 
         # Test skipping unsupported record type
         zone_retrieve_mock.reset_mock()
-        nsone_zone = {
-            'records': self.nsone_records + [{
+        ns1_zone = {
+            'records': self.ns1_records + [{
                 'type': 'UNSUPPORTED',
                 'ttl': 42,
                 'short_answers': ['unsupported'],
@@ -268,7 +268,7 @@ class TestNs1Provider(TestCase):
                 'ttl': 34,
             }],
         }
-        zone_retrieve_mock.side_effect = [nsone_zone]
+        zone_retrieve_mock.side_effect = [ns1_zone]
         zone = Zone('unit.tests.', [])
         provider.populate(zone)
         self.assertEquals(self.expected, zone.records)
@@ -347,8 +347,8 @@ class TestNs1Provider(TestCase):
         zone_retrieve_mock.reset_mock()
         zone_create_mock.reset_mock()
 
-        nsone_zone = {
-            'records': self.nsone_records + [{
+        ns1_zone = {
+            'records': self.ns1_records + [{
                 'type': 'A',
                 'ttl': 42,
                 'short_answers': ['9.9.9.9'],
@@ -368,7 +368,7 @@ class TestNs1Provider(TestCase):
                 'ttl': 34,
             }],
         }
-        nsone_zone['records'][0]['short_answers'][0] = '2.2.2.2'
+        ns1_zone['records'][0]['short_answers'][0] = '2.2.2.2'
 
         record_retrieve_mock.side_effect = [{
             "domain": "geo.unit.tests",
@@ -387,7 +387,7 @@ class TestNs1Provider(TestCase):
             'ttl': 34,
         }]
 
-        zone_retrieve_mock.side_effect = [nsone_zone, nsone_zone]
+        zone_retrieve_mock.side_effect = [ns1_zone, ns1_zone]
         plan = provider.plan(desired)
         self.assertEquals(3, len(plan.changes))
         # Shouldn't rely on order so just count classes
@@ -470,7 +470,7 @@ class TestNs1Provider(TestCase):
     def test_data_for_CNAME(self):
         provider = Ns1Provider('test', 'api-key')
 
-        # answers from nsone
+        # answers from ns1
         a_record = {
             'ttl': 31,
             'type': 'CNAME',
@@ -484,7 +484,7 @@ class TestNs1Provider(TestCase):
         self.assertEqual(a_expected,
                          provider._data_for_CNAME(a_record['type'], a_record))
 
-        # no answers from nsone
+        # no answers from ns1
         b_record = {
             'ttl': 32,
             'type': 'CNAME',

From 9e8a417c356d8f00c5e5096f0864f70bf50d5ab5 Mon Sep 17 00:00:00 2001
From: Ross McFarland <ross@github.com>
Date: Mon, 9 Dec 2019 08:26:20 -0800
Subject: [PATCH 04/56] Refactor thin Ns1Client wrapper out of provider

---
 octodns/provider/ns1.py | 98 ++++++++++++++++++++++++++---------------
 1 file changed, 63 insertions(+), 35 deletions(-)

diff --git a/octodns/provider/ns1.py b/octodns/provider/ns1.py
index cf78241..8acbd53 100644
--- a/octodns/provider/ns1.py
+++ b/octodns/provider/ns1.py
@@ -19,6 +19,56 @@ from ..record import Record
 from .base import BaseProvider
 
 
+class Ns1Client(object):
+    log = getLogger('NS1Client')
+
+    def __init__(self, api_key, retry_delay=1):
+        self.retry_delay = retry_delay
+
+        client = NS1(apiKey=api_key)
+        self._records = client.records()
+        self._zones = client.zones()
+
+    def zones_retrieve(self, name):
+        return self._zones.retrieve(name)
+
+    def zones_create(self, name):
+        return self._zones.create(name)
+
+    def records_retrieve(self, zone, domain, _type):
+        return self._records.retrieve(zone, domain, _type)
+
+    def records_create(self, zone, domain, _type, **params):
+        try:
+            return self._records.create(zone, domain, _type, **params)
+        except RateLimitException as e:
+            period = float(e.period)
+            self.log.warn('_apply_Create: rate limit encountered, pausing '
+                          'for %ds and trying again', period)
+            sleep(period)
+            return self._records.create(zone, domain, _type, **params)
+
+    def records_update(self, zone, domain, _type, **params):
+        try:
+            return self._records.update(zone, domain, _type, **params)
+        except RateLimitException as e:
+            period = float(e.period)
+            self.log.warn('_apply_Update: rate limit encountered, pausing '
+                          'for %ds and trying again', period)
+            sleep(period)
+            return self._records.update(zone, domain, _type, **params)
+
+    def records_delete(self, zone, domain, _type):
+        try:
+            return self._records.delete(zone, domain, _type)
+        except RateLimitException as e:
+            period = float(e.period)
+            self.log.warn('_apply_Delete: rate limit encountered, pausing '
+                          'for %ds and trying again', period)
+            sleep(period)
+            return self._records.delete(zone, domain, _type)
+
+
 class Ns1Provider(BaseProvider):
     '''
     Ns1 provider
@@ -34,13 +84,12 @@ class Ns1Provider(BaseProvider):
 
     ZONE_NOT_FOUND_MESSAGE = 'server error: zone not found'
 
-    def __init__(self, id, api_key, *args, **kwargs):
+    def __init__(self, id, api_key, retry_delay=1, *args, **kwargs):
         self.log = getLogger('Ns1Provider[{}]'.format(id))
-        self.log.debug('__init__: id=%s, api_key=***', id)
+        self.log.debug('__init__: id=%s, api_key=***, retry_delay=%d', id,
+                       retry_delay)
         super(Ns1Provider, self).__init__(id, *args, **kwargs)
-        client = NS1(apiKey=api_key)
-        self._records = client.records()
-        self._zones = client.zones()
+        self._client = Ns1Client(api_key, retry_delay)
 
     def _data_for_A(self, _type, record):
         # record meta (which would include geo information is only
@@ -192,7 +241,7 @@ class Ns1Provider(BaseProvider):
 
         try:
             ns1_zone_name = zone.name[:-1]
-            ns1_zone = self._zones.retrieve(ns1_zone_name)
+            ns1_zone = self._client.zones_retrieve(ns1_zone_name)
 
             records = []
             geo_records = []
@@ -207,9 +256,9 @@ class Ns1Provider(BaseProvider):
 
                 if record.get('tier', 1) > 1:
                     # Need to get the full record data for geo records
-                    record = self._records.retrieve(ns1_zone_name,
-                                                    record['domain'],
-                                                    record['type'])
+                    record = self._client.records_retrieve(ns1_zone_name,
+                                                           record['domain'],
+                                                           record['type'])
                     geo_records.append(record)
                 else:
                     records.append(record)
@@ -318,14 +367,7 @@ class Ns1Provider(BaseProvider):
         domain = new.fqdn[:-1]
         _type = new._type
         params = getattr(self, '_params_for_{}'.format(_type))(new)
-        try:
-            self._records.create(zone, domain, _type, **params)
-        except RateLimitException as e:
-            period = float(e.period)
-            self.log.warn('_apply_Create: rate limit encountered, pausing '
-                          'for %ds and trying again', period)
-            sleep(period)
-            self._records.create(zone, domain, _type, **params)
+        self._client.records_create(zone, domain, _type, **params)
 
     def _apply_Update(self, ns1_zone, change):
         new = change.new
@@ -333,28 +375,14 @@ class Ns1Provider(BaseProvider):
         domain = new.fqdn[:-1]
         _type = new._type
         params = getattr(self, '_params_for_{}'.format(_type))(new)
-        try:
-            self._records.update(zone, domain, _type, **params)
-        except RateLimitException as e:
-            period = float(e.period)
-            self.log.warn('_apply_Update: rate limit encountered, pausing '
-                          'for %ds and trying again', period)
-            sleep(period)
-            self._records.update(zone, domain, _type, **params)
+        self._client.records_update(zone, domain, _type, **params)
 
     def _apply_Delete(self, ns1_zone, change):
         existing = change.existing
         zone = existing.zone.name[:-1]
         domain = existing.fqdn[:-1]
         _type = existing._type
-        try:
-            self._records.delete(zone, domain, _type)
-        except RateLimitException as e:
-            period = float(e.period)
-            self.log.warn('_apply_Delete: rate limit encountered, pausing '
-                          'for %ds and trying again', period)
-            sleep(period)
-            self._records.delete(zone, domain, _type)
+        self._client.records_delete(zone, domain, _type)
 
     def _apply(self, plan):
         desired = plan.desired
@@ -364,12 +392,12 @@ class Ns1Provider(BaseProvider):
 
         domain_name = desired.name[:-1]
         try:
-            ns1_zone = self._zones.retrieve(domain_name)
+            ns1_zone = self._client.zones_retrieve(domain_name)
         except ResourceException as e:
             if e.message != self.ZONE_NOT_FOUND_MESSAGE:
                 raise
             self.log.debug('_apply:   no matching zone, creating')
-            ns1_zone = self._zones.create(domain_name)
+            ns1_zone = self._client.zones_create(domain_name)
 
         for change in changes:
             class_name = change.__class__.__name__

From 4fd2daa8a990521ae89d3be1a183f75ddee0fcd5 Mon Sep 17 00:00:00 2001
From: Ross McFarland <ross@github.com>
Date: Mon, 9 Dec 2019 08:56:55 -0800
Subject: [PATCH 05/56] Implement reworked NS1 retry mechinism

---
 octodns/provider/ns1.py            | 59 +++++++++++++-----------------
 tests/test_octodns_provider_ns1.py | 46 ++++++++++++++++++++++-
 2 files changed, 71 insertions(+), 34 deletions(-)

diff --git a/octodns/provider/ns1.py b/octodns/provider/ns1.py
index 8acbd53..da2d64a 100644
--- a/octodns/provider/ns1.py
+++ b/octodns/provider/ns1.py
@@ -22,51 +22,44 @@ from .base import BaseProvider
 class Ns1Client(object):
     log = getLogger('NS1Client')
 
-    def __init__(self, api_key, retry_delay=1):
-        self.retry_delay = retry_delay
+    def __init__(self, api_key, retry_count=4):
+        self.retry_count = retry_count
 
         client = NS1(apiKey=api_key)
         self._records = client.records()
         self._zones = client.zones()
 
+    def _try(self, method, *args, **kwargs):
+        tries = self.retry_count
+        while tries:
+            try:
+                return method(*args, **kwargs)
+            except RateLimitException as e:
+                period = float(e.period)
+                self.log.warn('rate limit encountered, pausing '
+                              'for %ds and trying again, %d remaining',
+                              period, tries)
+                sleep(period)
+                tries -= 1
+        raise
+
     def zones_retrieve(self, name):
-        return self._zones.retrieve(name)
+        return self._try(self._zones.retrieve, name)
 
     def zones_create(self, name):
-        return self._zones.create(name)
+        return self._try(self._zones.create, name)
 
     def records_retrieve(self, zone, domain, _type):
-        return self._records.retrieve(zone, domain, _type)
+        return self._try(self._records.retrieve, zone, domain, _type)
 
     def records_create(self, zone, domain, _type, **params):
-        try:
-            return self._records.create(zone, domain, _type, **params)
-        except RateLimitException as e:
-            period = float(e.period)
-            self.log.warn('_apply_Create: rate limit encountered, pausing '
-                          'for %ds and trying again', period)
-            sleep(period)
-            return self._records.create(zone, domain, _type, **params)
+        return self._try(self._records.create, zone, domain, _type, **params)
 
     def records_update(self, zone, domain, _type, **params):
-        try:
-            return self._records.update(zone, domain, _type, **params)
-        except RateLimitException as e:
-            period = float(e.period)
-            self.log.warn('_apply_Update: rate limit encountered, pausing '
-                          'for %ds and trying again', period)
-            sleep(period)
-            return self._records.update(zone, domain, _type, **params)
+        return self._try(self._records.update, zone, domain, _type, **params)
 
     def records_delete(self, zone, domain, _type):
-        try:
-            return self._records.delete(zone, domain, _type)
-        except RateLimitException as e:
-            period = float(e.period)
-            self.log.warn('_apply_Delete: rate limit encountered, pausing '
-                          'for %ds and trying again', period)
-            sleep(period)
-            return self._records.delete(zone, domain, _type)
+        return self._try(self._records.delete, zone, domain, _type)
 
 
 class Ns1Provider(BaseProvider):
@@ -84,12 +77,12 @@ class Ns1Provider(BaseProvider):
 
     ZONE_NOT_FOUND_MESSAGE = 'server error: zone not found'
 
-    def __init__(self, id, api_key, retry_delay=1, *args, **kwargs):
+    def __init__(self, id, api_key, retry_count=4, *args, **kwargs):
         self.log = getLogger('Ns1Provider[{}]'.format(id))
-        self.log.debug('__init__: id=%s, api_key=***, retry_delay=%d', id,
-                       retry_delay)
+        self.log.debug('__init__: id=%s, api_key=***, retry_count=%d', id,
+                       retry_count)
         super(Ns1Provider, self).__init__(id, *args, **kwargs)
-        self._client = Ns1Client(api_key, retry_delay)
+        self._client = Ns1Client(api_key, retry_count)
 
     def _data_for_A(self, _type, record):
         # record meta (which would include geo information is only
diff --git a/tests/test_octodns_provider_ns1.py b/tests/test_octodns_provider_ns1.py
index 0f23222..0743943 100644
--- a/tests/test_octodns_provider_ns1.py
+++ b/tests/test_octodns_provider_ns1.py
@@ -9,10 +9,11 @@ from collections import defaultdict
 from mock import call, patch
 from ns1.rest.errors import AuthException, RateLimitException, \
     ResourceException
+from six import text_type
 from unittest import TestCase
 
 from octodns.record import Delete, Record, Update
-from octodns.provider.ns1 import Ns1Provider
+from octodns.provider.ns1 import Ns1Client, Ns1Provider
 from octodns.zone import Zone
 
 
@@ -497,3 +498,46 @@ class TestNs1Provider(TestCase):
         }
         self.assertEqual(b_expected,
                          provider._data_for_CNAME(b_record['type'], b_record))
+
+
+class TestNs1Client(TestCase):
+
+    @patch('ns1.rest.zones.Zones.retrieve')
+    def test_retry_behavior(self, zone_retrieve_mock):
+        client = Ns1Client('dummy-key')
+
+        # No retry required, just calls and is returned
+        zone_retrieve_mock.reset_mock()
+        zone_retrieve_mock.side_effect = ['foo']
+        self.assertEquals('foo', client.zones_retrieve('unit.tests'))
+        zone_retrieve_mock.assert_has_calls([call('unit.tests')])
+
+        # One retry required
+        zone_retrieve_mock.reset_mock()
+        zone_retrieve_mock.side_effect = [
+            RateLimitException('boo', period=0),
+            'foo'
+        ]
+        self.assertEquals('foo', client.zones_retrieve('unit.tests'))
+        zone_retrieve_mock.assert_has_calls([call('unit.tests')])
+
+        # Two retries required
+        zone_retrieve_mock.reset_mock()
+        zone_retrieve_mock.side_effect = [
+            RateLimitException('boo', period=0),
+            'foo'
+        ]
+        self.assertEquals('foo', client.zones_retrieve('unit.tests'))
+        zone_retrieve_mock.assert_has_calls([call('unit.tests')])
+
+        # Exhaust our retries
+        zone_retrieve_mock.reset_mock()
+        zone_retrieve_mock.side_effect = [
+            RateLimitException('first', period=0),
+            RateLimitException('boo', period=0),
+            RateLimitException('boo', period=0),
+            RateLimitException('last', period=0),
+        ]
+        with self.assertRaises(RateLimitException) as ctx:
+            client.zones_retrieve('unit.tests')
+        self.assertEquals('last', text_type(ctx.exception))

From f0bc9add22bd29b814e61d0740ac14ad6e6906ab Mon Sep 17 00:00:00 2001
From: Ross McFarland <ross@github.com>
Date: Mon, 9 Dec 2019 14:30:02 -0800
Subject: [PATCH 06/56] Rough draft/expirimentation on dynamic creation

---
 octodns/provider/ns1.py | 167 +++++++++++++++++++++++++++++++++++++++-
 1 file changed, 163 insertions(+), 4 deletions(-)

diff --git a/octodns/provider/ns1.py b/octodns/provider/ns1.py
index da2d64a..445e612 100644
--- a/octodns/provider/ns1.py
+++ b/octodns/provider/ns1.py
@@ -70,13 +70,54 @@ class Ns1Provider(BaseProvider):
         class: octodns.provider.ns1.Ns1Provider
         api_key: env/NS1_API_KEY
     '''
-    SUPPORTS_GEO = True
-    SUPPORTS_DYNAMIC = False
+    SUPPORTS_GEO = False
+    SUPPORTS_DYNAMIC = True
     SUPPORTS = set(('A', 'AAAA', 'ALIAS', 'CAA', 'CNAME', 'MX', 'NAPTR',
                     'NS', 'PTR', 'SPF', 'SRV', 'TXT'))
 
     ZONE_NOT_FOUND_MESSAGE = 'server error: zone not found'
 
+    _DYNAMIC_FILTERS = [{
+        'config': {},
+        'filter': 'up'
+    }, {
+        'config': {},
+        'filter': u'geotarget_regional'
+    }, {
+        'config': {},
+        'filter': u'select_first_region'
+    }, {
+        'config': {
+            'eliminate': u'1'
+        },
+        'filter': 'priority'
+    }, {
+        'config': {},
+        'filter': u'weighted_shuffle'
+    }, {
+        'config': {
+            'N': u'1'
+        },
+        'filter': u'select_first_n'
+    }]
+    _REGION_TO_CONTINENT = {
+        'AFRICA': 'AF',
+        'ASIAPAC': 'AS',
+        'EUROPE': 'EU',
+        'SOUTH-AMERICA': 'SA',
+        'US-CENTRAL': 'NA',
+        'US-EAST': 'NA',
+        'US-WEST': 'NA',
+    }
+    _CONTINENT_TO_REGIONS = {
+        'AF': ('AFRICA',),
+        'AS': ('ASIAPAC',),
+        'EU': ('EUROPE',),
+        'SA': ('SOUTH-AMERICA',),
+        # TODO: what about CA, MX, and all the other NA countries?
+        'NA': ('US-CENTRAL', 'US-EAST', 'US-WEST'),
+    }
+
     def __init__(self, id, api_key, retry_count=4, *args, **kwargs):
         self.log = getLogger('Ns1Provider[{}]'.format(id))
         self.log.debug('__init__: id=%s, api_key=***, retry_count=%d', id,
@@ -282,9 +323,124 @@ class Ns1Provider(BaseProvider):
                       len(zone.records) - before, exists)
         return exists
 
+    def _encode_notes(self, data):
+        return ' '.join(['{}:{}'.format(k, v)
+                         for k, v in sorted(data.items())])
+
     def _params_for_A(self, record):
-        params = {'answers': record.values, 'ttl': record.ttl}
-        if hasattr(record, 'geo'):
+        params = {'ttl': record.ttl}
+
+        if hasattr(record, 'dynamic'):
+
+            pools = record.dynamic.pools
+
+            # Convert rules to regions
+            regions = {}
+            for i, rule in enumerate(record.dynamic.rules):
+                pool_name = rule.data['pool']
+
+                notes = {
+                    'rule-order': i,
+                }
+
+                fallback = pools[pool_name].data.get('fallback', None)
+                if fallback:
+                    notes['fallback'] = fallback
+
+                country = set()
+                georegion = set()
+                us_state = set()
+
+                for geo in rule.data.get('geos', []):
+                    n = len(geo)
+                    if n == 8:
+                        # US state
+                        us_state.add(geo[-2:])
+                    elif n == 5:
+                        # Country
+                        country.add(geo[-2:])
+                    else:
+                        # Continent
+                        georegion.update(self._CONTINENT_TO_REGIONS[geo])
+
+                meta = {
+                    'note': self._encode_notes(notes),
+                }
+                if georegion:
+                    meta['georegion'] = sorted(georegion)
+                if country:
+                    meta['country'] = sorted(country)
+                if us_state:
+                    meta['us_state'] = sorted(us_state)
+
+                regions[pool_name] = {
+                    'meta': meta,
+                }
+
+            # Build a list of primary values for each pool
+            pool_answers = defaultdict(list)
+            for pool_name, pool in sorted(pools.items()):
+                for value in pool.data['values']:
+                    pool_answers[pool_name].append({
+                        'answer': [value['value']],
+                        'weight': value['weight'],
+                    })
+
+            default_answers = [{
+                'answer': [v],
+                'weight': 1,
+            } for v in record.values]
+
+            # Build our list of answers
+            answers = []
+            for pool_name in sorted(pools.keys()):
+                priority = 1
+
+                # Dynamic/health checked
+                current_pool_name = pool_name
+                while current_pool_name:
+                    pool = pools[current_pool_name]
+                    for answer in pool_answers[current_pool_name]:
+                        answer = {
+                            'answer': answer['answer'],
+                            'meta': {
+                                'priority': priority,
+                                'note': self._encode_notes({
+                                    'from': current_pool_name,
+                                }),
+                                'weight': answer['weight'],
+                            },
+                            'region': pool_name,  # the one we're answering
+                        }
+                        answers.append(answer)
+
+                    current_pool_name = pool.data.get('fallback', None)
+                    priority += 1
+
+                # Static/default
+                for answer in default_answers:
+                    answer = {
+                        'answer': answer['answer'],
+                        'meta': {
+                            'priority': priority,
+                            'note': self._encode_notes({
+                                'from': '--default--',
+                            }),
+                            'weight': 1,
+                        },
+                        'region': pool_name,  # the one we're answering
+                    }
+                    answers.append(answer)
+
+            params.update({
+                'answers': answers,
+                'filters': self._DYNAMIC_FILTERS,
+                'regions': regions,
+            })
+
+            return params
+
+        elif hasattr(record, 'geo'):
             # purposefully set non-geo answers to have an empty meta,
             # so that we know we did this on purpose if/when troubleshooting
             params['answers'] = [{"answer": [x], "meta": {}}
@@ -315,6 +471,9 @@ class Ns1Provider(BaseProvider):
                     {"filter": "select_first_n",
                      "config": {"N": 1}}
                 )
+        else:
+            params['answers'] = record.values
+
         self.log.debug("params for A: %s", params)
         return params
 

From 334e64c8f5148c1f25d9227b8793a4feff38a2b8 Mon Sep 17 00:00:00 2001
From: Ross McFarland <ross@github.com>
Date: Tue, 10 Dec 2019 12:20:25 -0800
Subject: [PATCH 07/56] Python 3 friendly way to re-raise when tries expire

---
 octodns/provider/ns1.py | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/octodns/provider/ns1.py b/octodns/provider/ns1.py
index da2d64a..0383dbf 100644
--- a/octodns/provider/ns1.py
+++ b/octodns/provider/ns1.py
@@ -31,17 +31,18 @@ class Ns1Client(object):
 
     def _try(self, method, *args, **kwargs):
         tries = self.retry_count
-        while tries:
+        while True:  # We'll raise to break after our tries expire
             try:
                 return method(*args, **kwargs)
             except RateLimitException as e:
+                if tries <= 1:
+                    raise
                 period = float(e.period)
                 self.log.warn('rate limit encountered, pausing '
                               'for %ds and trying again, %d remaining',
                               period, tries)
                 sleep(period)
                 tries -= 1
-        raise
 
     def zones_retrieve(self, name):
         return self._try(self._zones.retrieve, name)

From ea2a52c307d5e03d7b1a7550273c1d3e607a2026 Mon Sep 17 00:00:00 2001
From: Ross McFarland <ross@github.com>
Date: Tue, 10 Dec 2019 12:20:25 -0800
Subject: [PATCH 08/56] Python 3 friendly way to re-raise when tries expire

---
 octodns/provider/ns1.py | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/octodns/provider/ns1.py b/octodns/provider/ns1.py
index 445e612..7244c3a 100644
--- a/octodns/provider/ns1.py
+++ b/octodns/provider/ns1.py
@@ -31,17 +31,18 @@ class Ns1Client(object):
 
     def _try(self, method, *args, **kwargs):
         tries = self.retry_count
-        while tries:
+        while True:  # We'll raise to break after our tries expire
             try:
                 return method(*args, **kwargs)
             except RateLimitException as e:
+                if tries <= 1:
+                    raise
                 period = float(e.period)
                 self.log.warn('rate limit encountered, pausing '
                               'for %ds and trying again, %d remaining',
                               period, tries)
                 sleep(period)
                 tries -= 1
-        raise
 
     def zones_retrieve(self, name):
         return self._try(self._zones.retrieve, name)

From 7a472506ccdadef44bc164f17829b32c2961c55e Mon Sep 17 00:00:00 2001
From: Ross McFarland <ross@github.com>
Date: Tue, 10 Dec 2019 13:50:11 -0800
Subject: [PATCH 09/56] Implement _data_for_dynamic_A w/some related
 refactoring

---
 octodns/provider/ns1.py            | 138 +++++++++++++++++++++++++++--
 tests/test_octodns_provider_ns1.py |  19 +++-
 2 files changed, 147 insertions(+), 10 deletions(-)

diff --git a/octodns/provider/ns1.py b/octodns/provider/ns1.py
index 7244c3a..0b6e16a 100644
--- a/octodns/provider/ns1.py
+++ b/octodns/provider/ns1.py
@@ -19,6 +19,10 @@ from ..record import Record
 from .base import BaseProvider
 
 
+class Ns1Exception(Exception):
+    pass
+
+
 class Ns1Client(object):
     log = getLogger('NS1Client')
 
@@ -126,7 +130,18 @@ class Ns1Provider(BaseProvider):
         super(Ns1Provider, self).__init__(id, *args, **kwargs)
         self._client = Ns1Client(api_key, retry_count)
 
-    def _data_for_A(self, _type, record):
+    def _encode_notes(self, data):
+        return ' '.join(['{}:{}'.format(k, v)
+                         for k, v in sorted(data.items())])
+
+    def _parse_notes(self, note):
+        data = {}
+        for piece in note.split(' '):
+            k, v = piece.split(':', 1)
+            data[k] = v
+        return data
+
+    def _data_for_geo_A(self, _type, record):
         # record meta (which would include geo information is only
         # returned when getting a record's detail, not from zone detail
         geo = defaultdict(list)
@@ -171,6 +186,116 @@ class Ns1Provider(BaseProvider):
         data['geo'] = geo
         return data
 
+    def _data_for_dynamic_A(self, _type, record):
+        # First make sure we have the expected filters config
+        if self._DYNAMIC_FILTERS != record['filters']:
+            self.log.error('_data_for_dynamic_A: %s %s has unsupported '
+                           'filters', record['domain'], _type)
+            raise Ns1Exception('Unrecognized advanced record')
+
+        # All regions (pools) will include the list of default values
+        # (eventually) at higher priorities, we'll just add them to this set to
+        # we'll have the complete collection.
+        default = set()
+        # Fill out the pools by walking the answers and looking at their
+        # region.
+        pools = defaultdict(lambda: {'fallback': None, 'values': []})
+        for answer in record['answers']:
+            # region (group name in the UI) is the pool name
+            pool_name = answer['region']
+            pool = pools[answer['region']]
+
+            meta = answer['meta']
+            value = text_type(answer['answer'][0])
+            if meta['priority'] == 1:
+                # priority 1 means this answer is part of the pools own values
+                pool['values'].append({
+                    'value': value,
+                    'weight': int(meta.get('weight', 1)),
+                })
+            else:
+                # It's a fallback, we only care about it if it's a
+                # final/default
+                notes = self._parse_notes(meta.get('note', ''))
+                if notes.get('from', False) == '--default--':
+                    default.add(value)
+
+        # The regions objects map to rules, but it's a bit fuzzy since they're
+        # tied to pools on the NS1 side, e.g. we can only have 1 rule per pool,
+        # that may eventually run into problems, but I don't have any use-cases
+        # examples currently where it would
+        rules = []
+        for pool_name, region in sorted(record['regions'].items()):
+            meta = region['meta']
+            notes = self._parse_notes(meta.get('note', ''))
+
+            # The group notes field in the UI is a `note` on the region here,
+            # that's where we can find our pool's fallback.
+            if 'fallback' in notes:
+                # set the fallback pool name
+                pools[pool_name]['fallback'] = notes['fallback']
+
+            geos = set()
+
+            # continents are mapped (imperfectly) to regions, but what about
+            # Canada/North America
+            for georegion in meta.get('georegion', []):
+                geos.add(self._REGION_TO_CONTINENT[georegion])
+
+            # Countries are easy enough to map, we just have ot find their
+            # continent
+            for country in meta.get('country', []):
+                con = country_alpha2_to_continent_code(country)
+                geos.add('{}-{}'.format(con, country))
+
+            # States are easy too, just assume NA-US (CA providences aren't
+            # supported by octoDNS currently)
+            for state in meta.get('us_state', []):
+                geos.add('NA-US-{}'.format(state))
+
+            rule = {
+                'pool': pool_name,
+                '_order': notes['rule-order'],
+            }
+            if geos:
+                rule['geos'] = geos
+            rules.append(rule)
+
+        # Order and convert to a list
+        default = sorted(default)
+        # Order
+        rules.sort(key=lambda r: (r['_order'], r['pool']))
+
+        return {
+            'dynamic': {
+                'pools': pools,
+                'rules': rules,
+            },
+            'ttl': record['ttl'],
+            'type': _type,
+            'values': sorted(default),
+        }
+
+    def _data_for_A(self, _type, record):
+        if record.get('tier', 1) > 1:
+            # Advanced record, see if it's first answer has a note
+            try:
+                first_answer_note = record['answers'][0]['meta']['note']
+            except (IndexError, KeyError):
+                first_answer_note = ''
+            # If that note includes a `from` (pool name) it's a dynamic record
+            if 'from:' in first_answer_note:
+                return self._data_for_dynamic_A(_type, record)
+            # If not it's an old geo record
+            return self._data_for_geo_A(_type, record)
+
+        # This is a basic record, just convert it
+        return {
+            'ttl': record['ttl'],
+            'type': _type,
+            'values': [text_type(x) for x in record['short_answers']]
+        }
+
     _data_for_AAAA = _data_for_A
 
     def _data_for_SPF(self, _type, record):
@@ -316,23 +441,18 @@ class Ns1Provider(BaseProvider):
                 continue
             data_for = getattr(self, '_data_for_{}'.format(_type))
             name = zone.hostname_from_fqdn(record['domain'])
-            record = Record.new(zone, name, data_for(_type, record),
-                                source=self, lenient=lenient)
+            data = data_for(_type, record)
+            record = Record.new(zone, name, data, source=self, lenient=lenient)
             zone_hash[(_type, name)] = record
         [zone.add_record(r, lenient=lenient) for r in zone_hash.values()]
         self.log.info('populate:   found %s records, exists=%s',
                       len(zone.records) - before, exists)
         return exists
 
-    def _encode_notes(self, data):
-        return ' '.join(['{}:{}'.format(k, v)
-                         for k, v in sorted(data.items())])
-
     def _params_for_A(self, record):
         params = {'ttl': record.ttl}
 
-        if hasattr(record, 'dynamic'):
-
+        if hasattr(record, 'dynamic') and record.dynamic:
             pools = record.dynamic.pools
 
             # Convert rules to regions
diff --git a/tests/test_octodns_provider_ns1.py b/tests/test_octodns_provider_ns1.py
index 0743943..fedcc2e 100644
--- a/tests/test_octodns_provider_ns1.py
+++ b/tests/test_octodns_provider_ns1.py
@@ -165,8 +165,9 @@ class TestNs1Provider(TestCase):
         'domain': 'unit.tests.',
     }]
 
+    @patch('ns1.rest.records.Records.retrieve')
     @patch('ns1.rest.zones.Zones.retrieve')
-    def test_populate(self, zone_retrieve_mock):
+    def test_populate(self, zone_retrieve_mock, record_retrieve_mock):
         provider = Ns1Provider('test', 'api-key')
 
         # Bad auth
@@ -197,6 +198,7 @@ class TestNs1Provider(TestCase):
 
         # Existing zone w/o records
         zone_retrieve_mock.reset_mock()
+        record_retrieve_mock.reset_mock()
         ns1_zone = {
             'records': [{
                 "domain": "geo.unit.tests",
@@ -211,17 +213,23 @@ class TestNs1Provider(TestCase):
                     {'answer': ['4.5.6.7'],
                      'meta': {'iso_region_code': ['NA-US-WA']}},
                 ],
+                'tier': 3,
                 'ttl': 34,
             }],
         }
         zone_retrieve_mock.side_effect = [ns1_zone]
+        # Its tier 3 so we'll do a full lookup
+        record_retrieve_mock.side_effect = ns1_zone['records']
         zone = Zone('unit.tests.', [])
         provider.populate(zone)
         self.assertEquals(1, len(zone.records))
         self.assertEquals(('unit.tests',), zone_retrieve_mock.call_args[0])
+        record_retrieve_mock.assert_has_calls([call('unit.tests',
+                                                    'geo.unit.tests', 'A')])
 
         # Existing zone w/records
         zone_retrieve_mock.reset_mock()
+        record_retrieve_mock.reset_mock()
         ns1_zone = {
             'records': self.ns1_records + [{
                 "domain": "geo.unit.tests",
@@ -236,17 +244,23 @@ class TestNs1Provider(TestCase):
                     {'answer': ['4.5.6.7'],
                      'meta': {'iso_region_code': ['NA-US-WA']}},
                 ],
+                'tier': 3,
                 'ttl': 34,
             }],
         }
         zone_retrieve_mock.side_effect = [ns1_zone]
+        # Its tier 3 so we'll do a full lookup
+        record_retrieve_mock.side_effect = ns1_zone['records']
         zone = Zone('unit.tests.', [])
         provider.populate(zone)
         self.assertEquals(self.expected, zone.records)
         self.assertEquals(('unit.tests',), zone_retrieve_mock.call_args[0])
+        record_retrieve_mock.assert_has_calls([call('unit.tests',
+                                                    'geo.unit.tests', 'A')])
 
         # Test skipping unsupported record type
         zone_retrieve_mock.reset_mock()
+        record_retrieve_mock.reset_mock()
         ns1_zone = {
             'records': self.ns1_records + [{
                 'type': 'UNSUPPORTED',
@@ -266,6 +280,7 @@ class TestNs1Provider(TestCase):
                     {'answer': ['4.5.6.7'],
                      'meta': {'iso_region_code': ['NA-US-WA']}},
                 ],
+                'tier': 3,
                 'ttl': 34,
             }],
         }
@@ -274,6 +289,8 @@ class TestNs1Provider(TestCase):
         provider.populate(zone)
         self.assertEquals(self.expected, zone.records)
         self.assertEquals(('unit.tests',), zone_retrieve_mock.call_args[0])
+        record_retrieve_mock.assert_has_calls([call('unit.tests',
+                                                    'geo.unit.tests', 'A')])
 
     @patch('ns1.rest.records.Records.delete')
     @patch('ns1.rest.records.Records.update')

From f6c60b69b72e9538d164ccfa41ceefe82bebc011 Mon Sep 17 00:00:00 2001
From: Ross McFarland <ross@github.com>
Date: Wed, 11 Dec 2019 15:05:52 -0800
Subject: [PATCH 10/56] WIP monitors management

---
 octodns/provider/ns1.py | 110 +++++++++++++++++++++++++++++++++++++---
 1 file changed, 103 insertions(+), 7 deletions(-)

diff --git a/octodns/provider/ns1.py b/octodns/provider/ns1.py
index 0b6e16a..8d230cf 100644
--- a/octodns/provider/ns1.py
+++ b/octodns/provider/ns1.py
@@ -32,6 +32,7 @@ class Ns1Client(object):
         client = NS1(apiKey=api_key)
         self._records = client.records()
         self._zones = client.zones()
+        self._monitors = client.monitors()
 
     def _try(self, method, *args, **kwargs):
         tries = self.retry_count
@@ -66,6 +67,17 @@ class Ns1Client(object):
     def records_delete(self, zone, domain, _type):
         return self._try(self._records.delete, zone, domain, _type)
 
+    def monitors_list(self):
+        return self._try(self._monitors.list)
+
+    def monitors_create(self, **params):
+        body = {}  # TODO: not clear what this is supposed to be
+        return self._try(self._monitors.create, body, **params)
+
+    def monitors_update(self, job_id, **params):
+        body = {}  # TODO: not clear what this is supposed to be
+        return self._try(self._monitors.update, job_id, body, **params)
+
 
 class Ns1Provider(BaseProvider):
     '''
@@ -136,9 +148,10 @@ class Ns1Provider(BaseProvider):
 
     def _parse_notes(self, note):
         data = {}
-        for piece in note.split(' '):
-            k, v = piece.split(':', 1)
-            data[k] = v
+        if note:
+            for piece in note.split(' '):
+                k, v = piece.split(':', 1)
+                data[k] = v
         return data
 
     def _data_for_geo_A(self, _type, record):
@@ -258,7 +271,7 @@ class Ns1Provider(BaseProvider):
                 '_order': notes['rule-order'],
             }
             if geos:
-                rule['geos'] = geos
+                rule['geos'] = sorted(geos)
             rules.append(rule)
 
         # Order and convert to a list
@@ -449,6 +462,79 @@ class Ns1Provider(BaseProvider):
                       len(zone.records) - before, exists)
         return exists
 
+    def _extra_changes(self, desired, changes, **kwargs):
+        # TODO: check monitors to see if they need updated
+        return []
+
+    def _monitors_for(self, record):
+        # TODO: should this just be a global cache by fqdn, type, and value?
+        expected_host = record.fqdn[:-1]
+        expected_type = record._type
+
+        monitors = {}
+
+        # TODO: cache here or in Ns1Client
+        for monitor in self._client.monitors_list():
+            data = self._parse_notes(monitor['notes'])
+            if expected_host == data['host'] or \
+               expected_type == data['type']:
+                # This monitor does not belong to this record
+                config = monitor['config']
+                value = config['host']
+                monitors[value] = monitor
+
+        return monitors
+
+    def _sync_monitor(self, record, value, existing):
+        host = record.fqdn[:-1]
+        _type = record._type
+
+        request = 'GET {path} HTTP/1.0\\r\\nHost: {host}\\r\\n' \
+            'User-agent: NS1\\r\\n\\r\\n'.format(path=record.healthcheck_path,
+                                                 host=host)
+
+        expected = {
+            'active': True,
+            'config': {
+                'connect_timeout': 2000,
+                'host': value,
+                'port': record.healthcheck_port,
+                'response_timeout': 10000,
+                'send': request,
+                'ssl': record.healthcheck_protocol == 'HTTPS',
+            },
+            'frequency': 60,
+            'job_type': 'tcp',
+            'name': '{} - {} - {}'.format(host, _type, value),
+            'notes': self._encode_notes({
+                'host': host,
+                'type': _type,
+            }),
+            'policy': 'quorum',
+            'rapid_recheck': False,
+            'region_scope': 'fixed',
+            # TODO: what should we do here dal, sjc, lga, sin, ams
+            'regions': ['lga'],
+            'rules': [{
+                'comparison': 'contains',
+                'key': 'output',
+                'value': '200 OK',
+            }],
+        }
+
+        if existing:
+            monitor_id = existing['id']
+            # See if the monitor needs updating
+            for k, v in expected.items():
+                if existing.get(k, '--missing--') != v:
+                    self._client.monitors_update(monitor_id, **expected)
+                    break
+        else:
+            return self._client.monitors_create(**expected)['id']
+
+        # TODO: this needs to return the feed
+        return None
+
     def _params_for_A(self, record):
         params = {'ttl': record.ttl}
 
@@ -498,13 +584,21 @@ class Ns1Provider(BaseProvider):
                     'meta': meta,
                 }
 
-            # Build a list of primary values for each pool
+            existing_monitors = self._monitors_for(record)
+
+            # Build a list of primary values for each pool, including their
+            # monitor
             pool_answers = defaultdict(list)
             for pool_name, pool in sorted(pools.items()):
                 for value in pool.data['values']:
+                    weight = value['weight']
+                    value = value['value']
+                    existing = existing_monitors.get(value)
+                    monitor_id = self._sync_monitor(record, value, existing)
                     pool_answers[pool_name].append({
-                        'answer': [value['value']],
-                        'weight': value['weight'],
+                        'answer': [value],
+                        'weight': weight,
+                        'monitor_id': monitor_id,
                     })
 
             default_answers = [{
@@ -531,6 +625,7 @@ class Ns1Provider(BaseProvider):
                                 }),
                                 'weight': answer['weight'],
                             },
+                            'up': True,  # TODO: this should be a monitor/feed
                             'region': pool_name,  # the one we're answering
                         }
                         answers.append(answer)
@@ -547,6 +642,7 @@ class Ns1Provider(BaseProvider):
                             'note': self._encode_notes({
                                 'from': '--default--',
                             }),
+                            'up': True,
                             'weight': 1,
                         },
                         'region': pool_name,  # the one we're answering

From 55f4194daf8ece7fb63c3b9a6bf2d2fdf9a8d38a Mon Sep 17 00:00:00 2001
From: Ross McFarland <ross@github.com>
Date: Thu, 12 Dec 2019 13:23:35 -0800
Subject: [PATCH 11/56] Functionally complement and untested ns1 dynamic
 support

---
 octodns/provider/ns1.py            | 559 +++++++++++++++++++----------
 tests/test_octodns_provider_ns1.py |   8 +-
 2 files changed, 374 insertions(+), 193 deletions(-)

diff --git a/octodns/provider/ns1.py b/octodns/provider/ns1.py
index 8d230cf..e5cb1ed 100644
--- a/octodns/provider/ns1.py
+++ b/octodns/provider/ns1.py
@@ -12,10 +12,11 @@ from ns1 import NS1
 from ns1.rest.errors import RateLimitException, ResourceException
 from pycountry_convert import country_alpha2_to_continent_code
 from time import sleep
+from uuid import uuid4
 
 from six import text_type
 
-from ..record import Record
+from ..record import Record, Update
 from .base import BaseProvider
 
 
@@ -33,6 +34,9 @@ class Ns1Client(object):
         self._records = client.records()
         self._zones = client.zones()
         self._monitors = client.monitors()
+        self._notifylists = client.notifylists()
+        self._datasource = client.datasource()
+        self._datafeed = client.datafeed()
 
     def _try(self, method, *args, **kwargs):
         tries = self.retry_count
@@ -49,35 +53,62 @@ class Ns1Client(object):
                 sleep(period)
                 tries -= 1
 
-    def zones_retrieve(self, name):
-        return self._try(self._zones.retrieve, name)
+    def datafeed_create(self, sourceid, name, config):
+        return self._try(self._datafeed.create, sourceid, name, config)
 
-    def zones_create(self, name):
-        return self._try(self._zones.create, name)
+    def datafeed_delete(self, sourceid, feedid):
+        return self._try(self._datafeed.delete, sourceid, feedid)
 
-    def records_retrieve(self, zone, domain, _type):
-        return self._try(self._records.retrieve, zone, domain, _type)
+    def datafeed_list(self, sourceid):
+        return self._try(self._datafeed.list, sourceid)
 
-    def records_create(self, zone, domain, _type, **params):
-        return self._try(self._records.create, zone, domain, _type, **params)
+    def datasource_create(self, **body):
+        return self._try(self._datasource.create, **body)
 
-    def records_update(self, zone, domain, _type, **params):
-        return self._try(self._records.update, zone, domain, _type, **params)
+    def datasource_list(self):
+        return self._try(self._datasource.list)
 
-    def records_delete(self, zone, domain, _type):
-        return self._try(self._records.delete, zone, domain, _type)
+    def monitors_create(self, **params):
+        body = {}
+        return self._try(self._monitors.create, body, **params)
+
+    def monitors_delete(self, jobid):
+        return self._try(self._monitors.delete, jobid)
 
     def monitors_list(self):
         return self._try(self._monitors.list)
 
-    def monitors_create(self, **params):
-        body = {}  # TODO: not clear what this is supposed to be
-        return self._try(self._monitors.create, body, **params)
-
     def monitors_update(self, job_id, **params):
-        body = {}  # TODO: not clear what this is supposed to be
+        body = {}
         return self._try(self._monitors.update, job_id, body, **params)
 
+    def notifylists_delete(self, nlid):
+        return self._try(self._notifylists.delete, nlid)
+
+    def notifylists_create(self, **body):
+        return self._try(self._notifylists.create, body)
+
+    def notifylists_list(self):
+        return self._try(self._notifylists.list)
+
+    def records_create(self, zone, domain, _type, **params):
+        return self._try(self._records.create, zone, domain, _type, **params)
+
+    def records_delete(self, zone, domain, _type):
+        return self._try(self._records.delete, zone, domain, _type)
+
+    def records_retrieve(self, zone, domain, _type):
+        return self._try(self._records.retrieve, zone, domain, _type)
+
+    def records_update(self, zone, domain, _type, **params):
+        return self._try(self._records.update, zone, domain, _type, **params)
+
+    def zones_create(self, name):
+        return self._try(self._zones.create, name)
+
+    def zones_retrieve(self, name):
+        return self._try(self._zones.retrieve, name)
+
 
 class Ns1Provider(BaseProvider):
     '''
@@ -140,7 +171,11 @@ class Ns1Provider(BaseProvider):
         self.log.debug('__init__: id=%s, api_key=***, retry_count=%d', id,
                        retry_count)
         super(Ns1Provider, self).__init__(id, *args, **kwargs)
+
         self._client = Ns1Client(api_key, retry_count)
+        self.__monitors = None
+        self.__datasource_id = None
+        self.__feeds_for_monitors = None
 
     def _encode_notes(self, data):
         return ' '.join(['{}:{}'.format(k, v)
@@ -462,38 +497,139 @@ class Ns1Provider(BaseProvider):
                       len(zone.records) - before, exists)
         return exists
 
-    def _extra_changes(self, desired, changes, **kwargs):
-        # TODO: check monitors to see if they need updated
-        return []
+    def _params_for_geo_A(self, record):
+        # purposefully set non-geo answers to have an empty meta,
+        # so that we know we did this on purpose if/when troubleshooting
+        params = {
+            'answers': [{"answer": [x], "meta": {}} for x in record.values],
+            'ttl': record.ttl,
+        }
 
-    def _monitors_for(self, record):
-        # TODO: should this just be a global cache by fqdn, type, and value?
-        expected_host = record.fqdn[:-1]
-        expected_type = record._type
+        has_country = False
+        for iso_region, target in record.geo.items():
+            key = 'iso_region_code'
+            value = iso_region
+            if not has_country and \
+               len(value.split('-')) > 1:  # pragma: nocover
+                has_country = True
+            for answer in target.values:
+                params['answers'].append(
+                    {
+                        'answer': [answer],
+                        'meta': {key: [value]},
+                    },
+                )
+
+        params['filters'] = []
+        if has_country:
+            params['filters'].append(
+                {"filter": "shuffle", "config": {}}
+            )
+            params['filters'].append(
+                {"filter": "geotarget_country", "config": {}}
+            )
+            params['filters'].append(
+                {"filter": "select_first_n",
+                 "config": {"N": 1}}
+            )
+
+        return params, None
+
+    @property
+    def _monitors(self):
+        # TODO: cache in sync, here and for others
+        if self.__monitors is None:
+            self.__monitors = {m['id']: m
+                               for m in self._client.monitors_list()}
+        return self.__monitors
 
+    def _monitors_for(self, record):
         monitors = {}
 
-        # TODO: cache here or in Ns1Client
-        for monitor in self._client.monitors_list():
-            data = self._parse_notes(monitor['notes'])
-            if expected_host == data['host'] or \
-               expected_type == data['type']:
-                # This monitor does not belong to this record
-                config = monitor['config']
-                value = config['host']
-                monitors[value] = monitor
+        if getattr(record, 'dynamic', False):
+            # TODO: should this just be a global cache by fqdn, type, and
+            # value?
+            expected_host = record.fqdn[:-1]
+            expected_type = record._type
+
+            # TODO: cache here or in Ns1Client
+            for monitor in self._monitors.values():
+                data = self._parse_notes(monitor['notes'])
+                if expected_host == data['host'] or \
+                   expected_type == data['type']:
+                    # This monitor does not belong to this record
+                    config = monitor['config']
+                    value = config['host']
+                    monitors[value] = monitor
 
         return monitors
 
-    def _sync_monitor(self, record, value, existing):
+    @property
+    def _datasource_id(self):
+        if self.__datasource_id is None:
+            name = 'octoDNS NS1 Data Source'
+            source = None
+            for candidate in self._client.datasource_list():
+                if candidate['name'] == name:
+                    # Found it
+                    source = candidate
+                    break
+
+            if source is None:
+                # We need to create it
+                source = self._client \
+                    .datasource_create(name=name,
+                                       sourcetype='nsone_monitoring')
+
+            self.__datasource_id = source['id']
+
+        return self.__datasource_id
+
+    def _feed_for_monitor(self, monitor):
+        if self.__feeds_for_monitors is None:
+            self.__feeds_for_monitors = {
+                f['config']['jobid']: f['id']
+                for f in self._client.datafeed_list(self._datasource_id)
+            }
+
+        return self.__feeds_for_monitors.get(monitor['id'])
+
+    def _create_monitor(self, monitor):
+        # TODO: looks like length limit is 64 char
+        name = '{} - {}'.format(monitor['name'], uuid4().hex[:6])
+
+        # Create the notify list
+        notify_list = [{
+            'config': {
+                'sourceid': self._datasource_id,
+            },
+            'type': 'datafeed',
+        }]
+        nl = self._client.notifylists_create(name=name,
+                                             notify_list=notify_list)
+
+        # Create the monitor
+        monitor['notify_list'] = nl['id']
+        monitor = self._client.monitors_create(**monitor)
+
+        # Create the data feed
+        config = {
+            'jobid': monitor['id'],
+        }
+        feed = self._client.datafeed_create(self._datasource_id, name,
+                                            config)
+
+        return monitor['id'], feed['id']
+
+    def _monitor_gen(self, record, value):
         host = record.fqdn[:-1]
         _type = record._type
 
         request = 'GET {path} HTTP/1.0\\r\\nHost: {host}\\r\\n' \
             'User-agent: NS1\\r\\n\\r\\n'.format(path=record.healthcheck_path,
-                                                 host=host)
+                                                 host=record.healthcheck_host)
 
-        expected = {
+        return {
             'active': True,
             'config': {
                 'connect_timeout': 2000,
@@ -522,177 +658,189 @@ class Ns1Provider(BaseProvider):
             }],
         }
 
+    def _monitor_is_match(self, expected, have):
+        # Make sure what we have matches what's in expected exactly. Anything
+        # else in have will be ignored.
+        for k, v in expected.items():
+            if have.get(k, '--missing--') != v:
+                return False
+
+        return True
+
+    def _monitor_sync(self, record, value, existing):
+        expected = self._monitor_gen(record, value)
+
         if existing:
             monitor_id = existing['id']
-            # See if the monitor needs updating
-            for k, v in expected.items():
-                if existing.get(k, '--missing--') != v:
-                    self._client.monitors_update(monitor_id, **expected)
-                    break
+
+            if not self._monitor_is_match(expected, existing):
+                # Update the monitor to match expected, everything else will be
+                # left alone and assumed correct
+                self._client.monitors_update(monitor_id, **expected)
+
+            try:
+                feed_id = self._feed_for_monitor(existing)
+            except KeyError:
+                raise Ns1Exception('Failed to find the feed for {} ({})'
+                                   .format(existing['name'], existing['id']))
         else:
-            return self._client.monitors_create(**expected)['id']
+            # We don't have an existing monitor create it (and related bits)
+            monitor_id, feed_id = self._create_monitor(expected)
 
-        # TODO: this needs to return the feed
-        return None
+        return monitor_id, feed_id
 
-    def _params_for_A(self, record):
-        params = {'ttl': record.ttl}
+    def _gc_monitors(self, record, active_monitor_ids=None):
 
-        if hasattr(record, 'dynamic') and record.dynamic:
-            pools = record.dynamic.pools
+        if active_monitor_ids is None:
+            active_monitor_ids = set()
 
-            # Convert rules to regions
-            regions = {}
-            for i, rule in enumerate(record.dynamic.rules):
-                pool_name = rule.data['pool']
+        for monitor in self._monitors_for(record).values():
+            monitor_id = monitor['id']
+            if monitor_id in active_monitor_ids:
+                continue
 
-                notes = {
-                    'rule-order': i,
-                }
+            feed_id = self._feed_for_monitor(monitor)
+            if feed_id:
+                self._client.datafeed_delete(self._datasource_id, feed_id)
 
-                fallback = pools[pool_name].data.get('fallback', None)
-                if fallback:
-                    notes['fallback'] = fallback
-
-                country = set()
-                georegion = set()
-                us_state = set()
-
-                for geo in rule.data.get('geos', []):
-                    n = len(geo)
-                    if n == 8:
-                        # US state
-                        us_state.add(geo[-2:])
-                    elif n == 5:
-                        # Country
-                        country.add(geo[-2:])
-                    else:
-                        # Continent
-                        georegion.update(self._CONTINENT_TO_REGIONS[geo])
-
-                meta = {
-                    'note': self._encode_notes(notes),
-                }
-                if georegion:
-                    meta['georegion'] = sorted(georegion)
-                if country:
-                    meta['country'] = sorted(country)
-                if us_state:
-                    meta['us_state'] = sorted(us_state)
-
-                regions[pool_name] = {
-                    'meta': meta,
-                }
+            self._client.monitors_delete(monitor_id)
 
-            existing_monitors = self._monitors_for(record)
-
-            # Build a list of primary values for each pool, including their
-            # monitor
-            pool_answers = defaultdict(list)
-            for pool_name, pool in sorted(pools.items()):
-                for value in pool.data['values']:
-                    weight = value['weight']
-                    value = value['value']
-                    existing = existing_monitors.get(value)
-                    monitor_id = self._sync_monitor(record, value, existing)
-                    pool_answers[pool_name].append({
-                        'answer': [value],
-                        'weight': weight,
-                        'monitor_id': monitor_id,
-                    })
-
-            default_answers = [{
-                'answer': [v],
-                'weight': 1,
-            } for v in record.values]
-
-            # Build our list of answers
-            answers = []
-            for pool_name in sorted(pools.keys()):
-                priority = 1
-
-                # Dynamic/health checked
-                current_pool_name = pool_name
-                while current_pool_name:
-                    pool = pools[current_pool_name]
-                    for answer in pool_answers[current_pool_name]:
-                        answer = {
-                            'answer': answer['answer'],
-                            'meta': {
-                                'priority': priority,
-                                'note': self._encode_notes({
-                                    'from': current_pool_name,
-                                }),
-                                'weight': answer['weight'],
-                            },
-                            'up': True,  # TODO: this should be a monitor/feed
-                            'region': pool_name,  # the one we're answering
-                        }
-                        answers.append(answer)
+            notify_list_id = monitor['notify_list']
+            self._client.notifylists_delete(notify_list_id)
+
+    def _params_for_dynamic_A(self, record):
+        pools = record.dynamic.pools
+
+        # Convert rules to regions
+        regions = {}
+        for i, rule in enumerate(record.dynamic.rules):
+            pool_name = rule.data['pool']
+
+            notes = {
+                'rule-order': i,
+            }
 
-                    current_pool_name = pool.data.get('fallback', None)
-                    priority += 1
+            fallback = pools[pool_name].data.get('fallback', None)
+            if fallback:
+                notes['fallback'] = fallback
+
+            country = set()
+            georegion = set()
+            us_state = set()
+
+            for geo in rule.data.get('geos', []):
+                n = len(geo)
+                if n == 8:
+                    # US state
+                    us_state.add(geo[-2:])
+                elif n == 5:
+                    # Country
+                    country.add(geo[-2:])
+                else:
+                    # Continent
+                    georegion.update(self._CONTINENT_TO_REGIONS[geo])
+
+            meta = {
+                'note': self._encode_notes(notes),
+            }
+            if georegion:
+                meta['georegion'] = sorted(georegion)
+            if country:
+                meta['country'] = sorted(country)
+            if us_state:
+                meta['us_state'] = sorted(us_state)
+
+            regions[pool_name] = {
+                'meta': meta,
+            }
 
-                # Static/default
-                for answer in default_answers:
+        existing_monitors = self._monitors_for(record)
+        active_monitors = set()
+
+        # Build a list of primary values for each pool, including their
+        # feed_id (monitor)
+        pool_answers = defaultdict(list)
+        for pool_name, pool in sorted(pools.items()):
+            for value in pool.data['values']:
+                weight = value['weight']
+                value = value['value']
+                existing = existing_monitors.get(value)
+                monitor_id, feed_id = self._monitor_sync(record, value,
+                                                         existing)
+                active_monitors.add(monitor_id)
+                pool_answers[pool_name].append({
+                    'answer': [value],
+                    'weight': weight,
+                    'feed_id': feed_id,
+                })
+
+        default_answers = [{
+            'answer': [v],
+            'weight': 1,
+        } for v in record.values]
+
+        # Build our list of answers
+        answers = []
+        for pool_name in sorted(pools.keys()):
+            priority = 1
+
+            # Dynamic/health checked
+            current_pool_name = pool_name
+            while current_pool_name:
+                pool = pools[current_pool_name]
+                for answer in pool_answers[current_pool_name]:
                     answer = {
                         'answer': answer['answer'],
                         'meta': {
                             'priority': priority,
                             'note': self._encode_notes({
-                                'from': '--default--',
+                                'from': current_pool_name,
                             }),
-                            'up': True,
-                            'weight': 1,
+                            'up': {
+                                'feed': answer['feed_id'],
+                            },
+                            'weight': answer['weight'],
                         },
                         'region': pool_name,  # the one we're answering
                     }
                     answers.append(answer)
 
-            params.update({
-                'answers': answers,
-                'filters': self._DYNAMIC_FILTERS,
-                'regions': regions,
-            })
+                current_pool_name = pool.data.get('fallback', None)
+                priority += 1
+
+            # Static/default
+            for answer in default_answers:
+                answer = {
+                    'answer': answer['answer'],
+                    'meta': {
+                        'priority': priority,
+                        'note': self._encode_notes({
+                            'from': '--default--',
+                        }),
+                        'up': True,
+                        'weight': 1,
+                    },
+                    'region': pool_name,  # the one we're answering
+                }
+                answers.append(answer)
 
-            return params
+        return {
+            'answers': answers,
+            'filters': self._DYNAMIC_FILTERS,
+            'regions': regions,
+            'ttl': record.ttl,
+        }, active_monitors
 
+    def _params_for_A(self, record):
+        if getattr(record, 'dynamic', False):
+            return self._params_for_dynamic_A(record)
         elif hasattr(record, 'geo'):
-            # purposefully set non-geo answers to have an empty meta,
-            # so that we know we did this on purpose if/when troubleshooting
-            params['answers'] = [{"answer": [x], "meta": {}}
-                                 for x in record.values]
-            has_country = False
-            for iso_region, target in record.geo.items():
-                key = 'iso_region_code'
-                value = iso_region
-                if not has_country and \
-                   len(value.split('-')) > 1:  # pragma: nocover
-                    has_country = True
-                for answer in target.values:
-                    params['answers'].append(
-                        {
-                            'answer': [answer],
-                            'meta': {key: [value]},
-                        },
-                    )
-            params['filters'] = []
-            if has_country:
-                params['filters'].append(
-                    {"filter": "shuffle", "config": {}}
-                )
-                params['filters'].append(
-                    {"filter": "geotarget_country", "config": {}}
-                )
-                params['filters'].append(
-                    {"filter": "select_first_n",
-                     "config": {"N": 1}}
-                )
-        else:
-            params['answers'] = record.values
+            return self._params_for_geo_A(record)
 
-        self.log.debug("params for A: %s", params)
-        return params
+        return {
+            'answers': record.values,
+            'ttl': record.ttl,
+        }, None
 
     _params_for_AAAA = _params_for_A
     _params_for_NS = _params_for_A
@@ -702,49 +850,81 @@ class Ns1Provider(BaseProvider):
         # escaped in values so we have to strip them here and add
         # them when going the other way
         values = [v.replace('\\;', ';') for v in record.values]
-        return {'answers': values, 'ttl': record.ttl}
+        return {'answers': values, 'ttl': record.ttl}, None
 
     _params_for_TXT = _params_for_SPF
 
     def _params_for_CAA(self, record):
         values = [(v.flags, v.tag, v.value) for v in record.values]
-        return {'answers': values, 'ttl': record.ttl}
+        return {'answers': values, 'ttl': record.ttl}, None
 
+    # TODO: dynamic CNAME support
     def _params_for_CNAME(self, record):
-        return {'answers': [record.value], 'ttl': record.ttl}
+        return {'answers': [record.value], 'ttl': record.ttl}, None
 
     _params_for_ALIAS = _params_for_CNAME
     _params_for_PTR = _params_for_CNAME
 
     def _params_for_MX(self, record):
         values = [(v.preference, v.exchange) for v in record.values]
-        return {'answers': values, 'ttl': record.ttl}
+        return {'answers': values, 'ttl': record.ttl}, None
 
     def _params_for_NAPTR(self, record):
         values = [(v.order, v.preference, v.flags, v.service, v.regexp,
                    v.replacement) for v in record.values]
-        return {'answers': values, 'ttl': record.ttl}
+        return {'answers': values, 'ttl': record.ttl}, None
 
     def _params_for_SRV(self, record):
         values = [(v.priority, v.weight, v.port, v.target)
                   for v in record.values]
-        return {'answers': values, 'ttl': record.ttl}
+        return {'answers': values, 'ttl': record.ttl}, None
+
+    def _extra_changes(self, desired, changes, **kwargs):
+        self.log.debug('_extra_changes: desired=%s', desired.name)
+
+        changed = set([c.record for c in changes])
+
+        extra = []
+        for record in desired.records:
+            if record in changed or not getattr(record, 'dynamic', False):
+                # Already changed, or no dynamic , no need to check it
+                continue
+
+            for have in self._monitors_for(record).values():
+                value = have['config']['host']
+                expected = self._monitor_gen(record, value)
+                if not expected:
+                    self.log.info('_extra_changes: monitor missing for %s',
+                                  expected['name'])
+                    extra.append(Update(record, record))
+                    break
+                if not self._monitor_is_match(expected, have):
+                    self.log.info('_extra_changes: monitor mis-match for %s',
+                                  expected['name'])
+                    extra.append(Update(record, record))
+                    break
+
+        return extra
 
     def _apply_Create(self, ns1_zone, change):
         new = change.new
         zone = new.zone.name[:-1]
         domain = new.fqdn[:-1]
         _type = new._type
-        params = getattr(self, '_params_for_{}'.format(_type))(new)
+        params, active_monitor_ids = \
+            getattr(self, '_params_for_{}'.format(_type))(new)
         self._client.records_create(zone, domain, _type, **params)
+        self._gc_monitors(new, active_monitor_ids)
 
     def _apply_Update(self, ns1_zone, change):
         new = change.new
         zone = new.zone.name[:-1]
         domain = new.fqdn[:-1]
         _type = new._type
-        params = getattr(self, '_params_for_{}'.format(_type))(new)
+        params, active_monitor_ids = \
+            getattr(self, '_params_for_{}'.format(_type))(new)
         self._client.records_update(zone, domain, _type, **params)
+        self._gc_monitors(new, active_monitor_ids)
 
     def _apply_Delete(self, ns1_zone, change):
         existing = change.existing
@@ -752,6 +932,7 @@ class Ns1Provider(BaseProvider):
         domain = existing.fqdn[:-1]
         _type = existing._type
         self._client.records_delete(zone, domain, _type)
+        self._gc_monitors(existing)
 
     def _apply(self, plan):
         desired = plan.desired
diff --git a/tests/test_octodns_provider_ns1.py b/tests/test_octodns_provider_ns1.py
index fedcc2e..539fcfb 100644
--- a/tests/test_octodns_provider_ns1.py
+++ b/tests/test_octodns_provider_ns1.py
@@ -474,16 +474,16 @@ class TestNs1Provider(TestCase):
             'type': 'SPF',
             'value': 'foo\\; bar baz\\; blip'
         })
-        self.assertEquals(['foo; bar baz; blip'],
-                          provider._params_for_SPF(record)['answers'])
+        params, _ = provider._params_for_SPF(record)
+        self.assertEquals(['foo; bar baz; blip'], params['answers'])
 
         record = Record.new(zone, 'txt', {
             'ttl': 35,
             'type': 'TXT',
             'value': 'foo\\; bar baz\\; blip'
         })
-        self.assertEquals(['foo; bar baz; blip'],
-                          provider._params_for_TXT(record)['answers'])
+        params, _ = provider._params_for_SPF(record)
+        self.assertEquals(['foo; bar baz; blip'], params['answers'])
 
     def test_data_for_CNAME(self):
         provider = Ns1Provider('test', 'api-key')

From c119f2e802a73e19c1918f1b37d2f1e44e5cac13 Mon Sep 17 00:00:00 2001
From: Ross McFarland <ross@github.com>
Date: Thu, 12 Dec 2019 14:03:09 -0800
Subject: [PATCH 12/56] Move ns1 caching to client where it's much
 safer/consistent

---
 octodns/provider/ns1.py | 182 +++++++++++++++++++++-------------------
 1 file changed, 97 insertions(+), 85 deletions(-)

diff --git a/octodns/provider/ns1.py b/octodns/provider/ns1.py
index e5cb1ed..321b439 100644
--- a/octodns/provider/ns1.py
+++ b/octodns/provider/ns1.py
@@ -38,26 +38,58 @@ class Ns1Client(object):
         self._datasource = client.datasource()
         self._datafeed = client.datafeed()
 
-    def _try(self, method, *args, **kwargs):
-        tries = self.retry_count
-        while True:  # We'll raise to break after our tries expire
-            try:
-                return method(*args, **kwargs)
-            except RateLimitException as e:
-                if tries <= 1:
-                    raise
-                period = float(e.period)
-                self.log.warn('rate limit encountered, pausing '
-                              'for %ds and trying again, %d remaining',
-                              period, tries)
-                sleep(period)
-                tries -= 1
+        self._datasource_id = None
+        self._feeds_for_monitors = None
+        self._monitors_cache = None
+
+    @property
+    def datasource_id(self):
+        if self._datasource_id is None:
+            name = 'octoDNS NS1 Data Source'
+            source = None
+            for candidate in self.datasource_list():
+                if candidate['name'] == name:
+                    # Found it
+                    source = candidate
+                    break
+
+            if source is None:
+                # We need to create it
+                source = self.datasource_create(name=name,
+                                                sourcetype='nsone_monitoring')
+
+            self._datasource_id = source['id']
+
+        return self._datasource_id
+
+    @property
+    def feeds_for_monitors(self):
+        if self._feeds_for_monitors is None:
+            self._feeds_for_monitors = {
+                f['config']['jobid']: f['id']
+                for f in self.datafeed_list(self.datasource_id)
+            }
+
+        return self._feeds_for_monitors
+
+    @property
+    def monitors(self):
+        if self._monitors_cache is None:
+            self._monitors_cache = \
+                {m['id']: m for m in self.monitors_list()}
+        return self._monitors_cache
 
     def datafeed_create(self, sourceid, name, config):
-        return self._try(self._datafeed.create, sourceid, name, config)
+        ret = self._try(self._datafeed.create, sourceid, name, config)
+        self.feeds_for_monitors[config['jobid']] = ret['id']
+        return ret
 
     def datafeed_delete(self, sourceid, feedid):
-        return self._try(self._datafeed.delete, sourceid, feedid)
+        ret = self._try(self._datafeed.delete, sourceid, feedid)
+        self._feeds_for_monitors = {
+            k: v for k, v in self._feeds_for_monitors.items() if v != feedid
+        }
+        return ret
 
     def datafeed_list(self, sourceid):
         return self._try(self._datafeed.list, sourceid)
@@ -70,10 +102,14 @@ class Ns1Client(object):
 
     def monitors_create(self, **params):
         body = {}
-        return self._try(self._monitors.create, body, **params)
+        ret = self._try(self._monitors.create, body, **params)
+        self.monitors[ret['id']] = ret
+        return ret
 
     def monitors_delete(self, jobid):
-        return self._try(self._monitors.delete, jobid)
+        ret = self._try(self._monitors.delete, jobid)
+        self.monitors.pop(jobid)
+        return ret
 
     def monitors_list(self):
         return self._try(self._monitors.list)
@@ -109,6 +145,21 @@ class Ns1Client(object):
     def zones_retrieve(self, name):
         return self._try(self._zones.retrieve, name)
 
+    def _try(self, method, *args, **kwargs):
+        tries = self.retry_count
+        while True:  # We'll raise to break after our tries expire
+            try:
+                return method(*args, **kwargs)
+            except RateLimitException as e:
+                if tries <= 1:
+                    raise
+                period = float(e.period)
+                self.log.warn('rate limit encountered, pausing '
+                              'for %ds and trying again, %d remaining',
+                              period, tries)
+                sleep(period)
+                tries -= 1
+
 
 class Ns1Provider(BaseProvider):
     '''
@@ -173,9 +224,6 @@ class Ns1Provider(BaseProvider):
         super(Ns1Provider, self).__init__(id, *args, **kwargs)
 
         self._client = Ns1Client(api_key, retry_count)
-        self.__monitors = None
-        self.__datasource_id = None
-        self.__feeds_for_monitors = None
 
     def _encode_notes(self, data):
         return ' '.join(['{}:{}'.format(k, v)
@@ -535,25 +583,14 @@ class Ns1Provider(BaseProvider):
 
         return params, None
 
-    @property
-    def _monitors(self):
-        # TODO: cache in sync, here and for others
-        if self.__monitors is None:
-            self.__monitors = {m['id']: m
-                               for m in self._client.monitors_list()}
-        return self.__monitors
-
     def _monitors_for(self, record):
         monitors = {}
 
         if getattr(record, 'dynamic', False):
-            # TODO: should this just be a global cache by fqdn, type, and
-            # value?
             expected_host = record.fqdn[:-1]
             expected_type = record._type
 
-            # TODO: cache here or in Ns1Client
-            for monitor in self._monitors.values():
+            for monitor in self._client.monitors.values():
                 data = self._parse_notes(monitor['notes'])
                 if expected_host == data['host'] or \
                    expected_type == data['type']:
@@ -564,62 +601,35 @@ class Ns1Provider(BaseProvider):
 
         return monitors
 
-    @property
-    def _datasource_id(self):
-        if self.__datasource_id is None:
-            name = 'octoDNS NS1 Data Source'
-            source = None
-            for candidate in self._client.datasource_list():
-                if candidate['name'] == name:
-                    # Found it
-                    source = candidate
-                    break
-
-            if source is None:
-                # We need to create it
-                source = self._client \
-                    .datasource_create(name=name,
-                                       sourcetype='nsone_monitoring')
-
-            self.__datasource_id = source['id']
-
-        return self.__datasource_id
+    def _create_feed(self, monitor):
+        # TODO: looks like length limit is 64 char
+        name = '{} - {}'.format(monitor['name'], uuid4().hex[:6])
 
-    def _feed_for_monitor(self, monitor):
-        if self.__feeds_for_monitors is None:
-            self.__feeds_for_monitors = {
-                f['config']['jobid']: f['id']
-                for f in self._client.datafeed_list(self._datasource_id)
-            }
+        # Create the data feed
+        config = {
+            'jobid': monitor['id'],
+        }
+        feed = self._client.datafeed_create(self._client.datasource_id, name,
+                                            config)
 
-        return self.__feeds_for_monitors.get(monitor['id'])
+        return feed['id']
 
     def _create_monitor(self, monitor):
-        # TODO: looks like length limit is 64 char
-        name = '{} - {}'.format(monitor['name'], uuid4().hex[:6])
-
         # Create the notify list
         notify_list = [{
             'config': {
-                'sourceid': self._datasource_id,
+                'sourceid': self._client.datasource_id,
             },
             'type': 'datafeed',
         }]
-        nl = self._client.notifylists_create(name=name,
+        nl = self._client.notifylists_create(name=monitor['name'],
                                              notify_list=notify_list)
 
         # Create the monitor
         monitor['notify_list'] = nl['id']
         monitor = self._client.monitors_create(**monitor)
 
-        # Create the data feed
-        config = {
-            'jobid': monitor['id'],
-        }
-        feed = self._client.datafeed_create(self._datasource_id, name,
-                                            config)
-
-        return monitor['id'], feed['id']
+        return monitor['id'], self._create_feed(monitor)
 
     def _monitor_gen(self, record, value):
         host = record.fqdn[:-1]
@@ -678,11 +688,11 @@ class Ns1Provider(BaseProvider):
                 # left alone and assumed correct
                 self._client.monitors_update(monitor_id, **expected)
 
-            try:
-                feed_id = self._feed_for_monitor(existing)
-            except KeyError:
-                raise Ns1Exception('Failed to find the feed for {} ({})'
-                                   .format(existing['name'], existing['id']))
+            feed_id = self._client.feeds_for_monitors.get(monitor_id)
+            if feed_id is None:
+                self.log.warn('_monitor_sync: %s (%s) missing feed, creating',
+                              existing['name'], monitor_id)
+                feed_id = self._create_feed(existing)
         else:
             # We don't have an existing monitor create it (and related bits)
             monitor_id, feed_id = self._create_monitor(expected)
@@ -699,9 +709,10 @@ class Ns1Provider(BaseProvider):
             if monitor_id in active_monitor_ids:
                 continue
 
-            feed_id = self._feed_for_monitor(monitor)
+            feed_id = self._client.feeds_for_monitors.get(monitor_id)
             if feed_id:
-                self._client.datafeed_delete(self._datasource_id, feed_id)
+                self._client.datafeed_delete(self._client.datasource_id,
+                                             feed_id)
 
             self._client.monitors_delete(monitor_id)
 
@@ -893,16 +904,17 @@ class Ns1Provider(BaseProvider):
             for have in self._monitors_for(record).values():
                 value = have['config']['host']
                 expected = self._monitor_gen(record, value)
-                if not expected:
-                    self.log.info('_extra_changes: monitor missing for %s',
-                                  expected['name'])
-                    extra.append(Update(record, record))
-                    break
+                # TODO: find values which have missing monitors
                 if not self._monitor_is_match(expected, have):
                     self.log.info('_extra_changes: monitor mis-match for %s',
                                   expected['name'])
                     extra.append(Update(record, record))
                     break
+                if not have.get('notify_list'):
+                    self.log.info('_extra_changes: broken monitor no notify '
+                                  'list %s (%s)', have['name'], have['id'])
+                    extra.append(Update(record, record))
+                    break
 
         return extra
 

From 674c29fb8b905b2754b8e3f93e811168a33f56a7 Mon Sep 17 00:00:00 2001
From: Ross McFarland <ross@github.com>
Date: Thu, 12 Dec 2019 14:17:42 -0800
Subject: [PATCH 13/56] Debug logging

---
 octodns/provider/ns1.py | 31 +++++++++++++++++++++++++++----
 1 file changed, 27 insertions(+), 4 deletions(-)

diff --git a/octodns/provider/ns1.py b/octodns/provider/ns1.py
index 321b439..5deb2c9 100644
--- a/octodns/provider/ns1.py
+++ b/octodns/provider/ns1.py
@@ -28,6 +28,7 @@ class Ns1Client(object):
     log = getLogger('NS1Client')
 
     def __init__(self, api_key, retry_count=4):
+        self.log.debug('__init__: retry_count=%d', retry_count)
         self.retry_count = retry_count
 
         client = NS1(apiKey=api_key)
@@ -54,9 +55,11 @@ class Ns1Client(object):
                     break
 
             if source is None:
+                self.log.info('datasource_id: creating datasource %s', name)
                 # We need to create it
                 source = self.datasource_create(name=name,
                                                 sourcetype='nsone_monitoring')
+                self.log.info('datasource_id:   id=%s', source['id'])
 
             self._datasource_id = source['id']
 
@@ -65,6 +68,7 @@ class Ns1Client(object):
     @property
     def feeds_for_monitors(self):
         if self._feeds_for_monitors is None:
+            self.log.debug('feeds_for_monitors: fetching & building')
             self._feeds_for_monitors = {
                 f['config']['jobid']: f['id']
                 for f in self.datafeed_list(self.datasource_id)
@@ -75,6 +79,7 @@ class Ns1Client(object):
     @property
     def monitors(self):
         if self._monitors_cache is None:
+            self.log.debug('monitors: fetching & building')
             self._monitors_cache = \
                 {m['id']: m for m in self.monitors_list()}
         return self._monitors_cache
@@ -602,19 +607,24 @@ class Ns1Provider(BaseProvider):
         return monitors
 
     def _create_feed(self, monitor):
+        monitor_id = monitor['id']
+        self.log.debug('_create_feed: monitor=%s', monitor_id)
         # TODO: looks like length limit is 64 char
         name = '{} - {}'.format(monitor['name'], uuid4().hex[:6])
 
         # Create the data feed
         config = {
-            'jobid': monitor['id'],
+            'jobid': monitor_id,
         }
         feed = self._client.datafeed_create(self._client.datasource_id, name,
                                             config)
+        feed_id = feed['id']
+        self.log.debug('_create_feed:   feed=%s', feed_id)
 
-        return feed['id']
+        return feed_id
 
     def _create_monitor(self, monitor):
+        self.log.debug('_create_monitor: monitor="%s"', monitor['name'])
         # Create the notify list
         notify_list = [{
             'config': {
@@ -624,12 +634,16 @@ class Ns1Provider(BaseProvider):
         }]
         nl = self._client.notifylists_create(name=monitor['name'],
                                              notify_list=notify_list)
+        nl_id = nl['id']
+        self.log.debug('_create_monitor:   notify_list=%s', nl_id)
 
         # Create the monitor
-        monitor['notify_list'] = nl['id']
+        monitor['notify_list'] = nl_id
         monitor = self._client.monitors_create(**monitor)
+        monitor_id = monitor['id']
+        self.log.debug('_create_monitor:   monitor=%s', monitor_id)
 
-        return monitor['id'], self._create_feed(monitor)
+        return monitor_id, self._create_feed(monitor)
 
     def _monitor_gen(self, record, value):
         host = record.fqdn[:-1]
@@ -678,12 +692,16 @@ class Ns1Provider(BaseProvider):
         return True
 
     def _monitor_sync(self, record, value, existing):
+        self.log.debug('_monitor_sync: record=%s, value=%s', record.fqdn,
+                       value)
         expected = self._monitor_gen(record, value)
 
         if existing:
+            self.log.debug('_monitor_sync:   existing=%s', existing['id'])
             monitor_id = existing['id']
 
             if not self._monitor_is_match(expected, existing):
+                self.log.debug('_monitor_sync:   existing needs update')
                 # Update the monitor to match expected, everything else will be
                 # left alone and assumed correct
                 self._client.monitors_update(monitor_id, **expected)
@@ -694,12 +712,15 @@ class Ns1Provider(BaseProvider):
                               existing['name'], monitor_id)
                 feed_id = self._create_feed(existing)
         else:
+            self.log.debug('_monitor_sync:   needs create')
             # We don't have an existing monitor create it (and related bits)
             monitor_id, feed_id = self._create_monitor(expected)
 
         return monitor_id, feed_id
 
     def _gc_monitors(self, record, active_monitor_ids=None):
+        self.log.debug('_gc_monitors: record=%s, active_monitor_ids=%s',
+                       record.fqdn, active_monitor_ids)
 
         if active_monitor_ids is None:
             active_monitor_ids = set()
@@ -709,6 +730,8 @@ class Ns1Provider(BaseProvider):
             if monitor_id in active_monitor_ids:
                 continue
 
+            self.log.debug('_gc_monitors:   deleting %s', monitor_id)
+
             feed_id = self._client.feeds_for_monitors.get(monitor_id)
             if feed_id:
                 self._client.datafeed_delete(self._client.datasource_id,

From 6c7abe1fd643a3444d2a261dd27391654b9350fe Mon Sep 17 00:00:00 2001
From: Ross McFarland <ross@github.com>
Date: Thu, 12 Dec 2019 14:19:16 -0800
Subject: [PATCH 14/56] Ns1 still SUPPORTS_GEO

---
 octodns/provider/ns1.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/octodns/provider/ns1.py b/octodns/provider/ns1.py
index 5deb2c9..f18646c 100644
--- a/octodns/provider/ns1.py
+++ b/octodns/provider/ns1.py
@@ -174,7 +174,7 @@ class Ns1Provider(BaseProvider):
         class: octodns.provider.ns1.Ns1Provider
         api_key: env/NS1_API_KEY
     '''
-    SUPPORTS_GEO = False
+    SUPPORTS_GEO = True
     SUPPORTS_DYNAMIC = True
     SUPPORTS = set(('A', 'AAAA', 'ALIAS', 'CAA', 'CNAME', 'MX', 'NAPTR',
                     'NS', 'PTR', 'SPF', 'SRV', 'TXT'))

From d7053a2e92012b7e5bee75952c116144c70bbff3 Mon Sep 17 00:00:00 2001
From: Ross McFarland <ross@github.com>
Date: Fri, 13 Dec 2019 11:58:18 -0800
Subject: [PATCH 15/56] Ns1Client tests for caching and minor logic

---
 octodns/provider/ns1.py            |  11 +-
 tests/test_octodns_provider_ns1.py | 186 +++++++++++++++++++++++++++++
 2 files changed, 194 insertions(+), 3 deletions(-)

diff --git a/octodns/provider/ns1.py b/octodns/provider/ns1.py
index f18646c..a3bd647 100644
--- a/octodns/provider/ns1.py
+++ b/octodns/provider/ns1.py
@@ -121,7 +121,9 @@ class Ns1Client(object):
 
     def monitors_update(self, job_id, **params):
         body = {}
-        return self._try(self._monitors.update, job_id, body, **params)
+        ret = self._try(self._monitors.update, job_id, body, **params)
+        self.monitors[ret['id']] = ret
+        return ret
 
     def notifylists_delete(self, nlid):
         return self._try(self._notifylists.delete, nlid)
@@ -238,8 +240,11 @@ class Ns1Provider(BaseProvider):
         data = {}
         if note:
             for piece in note.split(' '):
-                k, v = piece.split(':', 1)
-                data[k] = v
+                try:
+                    k, v = piece.split(':', 1)
+                    data[k] = v
+                except ValueError:
+                    pass
         return data
 
     def _data_for_geo_A(self, _type, record):
diff --git a/tests/test_octodns_provider_ns1.py b/tests/test_octodns_provider_ns1.py
index 539fcfb..4849684 100644
--- a/tests/test_octodns_provider_ns1.py
+++ b/tests/test_octodns_provider_ns1.py
@@ -517,6 +517,24 @@ class TestNs1Provider(TestCase):
                          provider._data_for_CNAME(b_record['type'], b_record))
 
 
+class TestNs1ProviderDynamic(TestCase):
+
+    def test_notes(self):
+        provider = Ns1Provider('test', 'api-key')
+
+        self.assertEquals({}, provider._parse_notes(None))
+        self.assertEquals({}, provider._parse_notes(''))
+        self.assertEquals({}, provider._parse_notes('blah-blah-blah'))
+
+        # Round tripping
+        data = {
+            'key': 'value',
+            'priority': '1',
+        }
+        notes = provider._encode_notes(data)
+        self.assertEquals(data, provider._parse_notes(notes))
+
+
 class TestNs1Client(TestCase):
 
     @patch('ns1.rest.zones.Zones.retrieve')
@@ -558,3 +576,171 @@ class TestNs1Client(TestCase):
         with self.assertRaises(RateLimitException) as ctx:
             client.zones_retrieve('unit.tests')
         self.assertEquals('last', text_type(ctx.exception))
+
+    @patch('ns1.rest.data.Source.list')
+    @patch('ns1.rest.data.Source.create')
+    def test_datasource_id(self, datasource_create_mock, datasource_list_mock):
+        client = Ns1Client('dummy-key')
+
+        # First invocation with an empty list create
+        datasource_list_mock.reset_mock()
+        datasource_create_mock.reset_mock()
+        datasource_list_mock.side_effect = [[]]
+        datasource_create_mock.side_effect = [{
+            'id': 'foo',
+        }]
+        self.assertEquals('foo', client.datasource_id)
+        name = 'octoDNS NS1 Data Source'
+        source_type = 'nsone_monitoring'
+        datasource_create_mock.assert_has_calls([call(name=name,
+                                                      sourcetype=source_type)])
+        datasource_list_mock.assert_called_once()
+
+        # 2nd invocation is cached
+        datasource_list_mock.reset_mock()
+        datasource_create_mock.reset_mock()
+        self.assertEquals('foo', client.datasource_id)
+        datasource_create_mock.assert_not_called()
+        datasource_list_mock.assert_not_called()
+
+        # Reset the client's cache
+        client._datasource_id = None
+
+        # First invocation with a match in the list finds it and doesn't call
+        # create
+        datasource_list_mock.reset_mock()
+        datasource_create_mock.reset_mock()
+        datasource_list_mock.side_effect = [[{
+            'id': 'other',
+            'name': 'not a match',
+        }, {
+            'id': 'bar',
+            'name': name,
+        }]]
+        self.assertEquals('bar', client.datasource_id)
+        datasource_create_mock.assert_not_called()
+        datasource_list_mock.assert_called_once()
+
+    @patch('ns1.rest.data.Feed.delete')
+    @patch('ns1.rest.data.Feed.create')
+    @patch('ns1.rest.data.Feed.list')
+    def test_feeds_for_monitors(self, datafeed_list_mock,
+                                datafeed_create_mock,
+                                datafeed_delete_mock):
+        client = Ns1Client('dummy-key')
+
+        # pre-cache datasource_id
+        client._datasource_id = 'foo'
+
+        # Populate the cache and check the results
+        datafeed_list_mock.reset_mock()
+        datafeed_list_mock.side_effect = [[{
+            'config': {
+                'jobid': 'the-job',
+            },
+            'id': 'the-feed',
+        }, {
+            'config': {
+                'jobid': 'the-other-job',
+            },
+            'id': 'the-other-feed',
+        }]]
+        expected = {
+            'the-job': 'the-feed',
+            'the-other-job': 'the-other-feed',
+        }
+        self.assertEquals(expected, client.feeds_for_monitors)
+        datafeed_list_mock.assert_called_once()
+
+        # 2nd call uses cache
+        datafeed_list_mock.reset_mock()
+        self.assertEquals(expected, client.feeds_for_monitors)
+        datafeed_list_mock.assert_not_called()
+
+        # create a feed and make sure it's in the cache/map
+        datafeed_create_mock.reset_mock()
+        datafeed_create_mock.side_effect = [{
+            'id': 'new-feed',
+        }]
+        client.datafeed_create(client.datasource_id, 'new-name', {
+            'jobid': 'new-job',
+        })
+        datafeed_create_mock.assert_has_calls([call('foo', 'new-name', {
+            'jobid': 'new-job',
+        })])
+        new_expected = expected.copy()
+        new_expected['new-job'] = 'new-feed'
+        self.assertEquals(new_expected, client.feeds_for_monitors)
+        datafeed_create_mock.assert_called_once()
+
+        # Delete a feed and make sure it's out of the cache/map
+        datafeed_delete_mock.reset_mock()
+        client.datafeed_delete(client.datasource_id, 'new-feed')
+        self.assertEquals(expected, client.feeds_for_monitors)
+        datafeed_delete_mock.assert_called_once()
+
+    @patch('ns1.rest.monitoring.Monitors.delete')
+    @patch('ns1.rest.monitoring.Monitors.update')
+    @patch('ns1.rest.monitoring.Monitors.create')
+    @patch('ns1.rest.monitoring.Monitors.list')
+    def test_monitors(self, monitors_list_mock, monitors_create_mock,
+                      monitors_update_mock, monitors_delete_mock):
+        client = Ns1Client('dummy-key')
+
+        one = {
+            'id': 'one',
+            'key': 'value',
+        }
+        two = {
+            'id': 'two',
+            'key': 'other-value',
+        }
+
+        # Populate the cache and check the results
+        monitors_list_mock.reset_mock()
+        monitors_list_mock.side_effect = [[one, two]]
+        expected = {
+            'one': one,
+            'two': two,
+        }
+        self.assertEquals(expected, client.monitors)
+        monitors_list_mock.assert_called_once()
+
+        # 2nd round pulls it from cache
+        monitors_list_mock.reset_mock()
+        self.assertEquals(expected, client.monitors)
+        monitors_list_mock.assert_not_called()
+
+        # Create a monitor, make sure it's in the list
+        monitors_create_mock.reset_mock()
+        monitor = {
+            'id': 'new-id',
+            'key': 'new-value',
+        }
+        monitors_create_mock.side_effect = [monitor]
+        self.assertEquals(monitor, client.monitors_create(param='eter'))
+        monitors_create_mock.assert_has_calls([call({}, param='eter')])
+        new_expected = expected.copy()
+        new_expected['new-id'] = monitor
+        self.assertEquals(new_expected, client.monitors)
+
+        # Update a monitor, make sure it's updated in the cache
+        monitors_update_mock.reset_mock()
+        monitor = {
+            'id': 'new-id',
+            'key': 'changed-value',
+        }
+        monitors_update_mock.side_effect = [monitor]
+        self.assertEquals(monitor, client.monitors_update('new-id',
+                                                          key='changed-value'))
+        monitors_update_mock \
+            .assert_has_calls([call('new-id', {}, key='changed-value')])
+        new_expected['new-id'] = monitor
+        self.assertEquals(new_expected, client.monitors)
+
+        # Delete a monitor, make sure it's out of the list
+        monitors_delete_mock.reset_mock()
+        monitors_delete_mock.side_effect = ['deleted']
+        self.assertEquals('deleted', client.monitors_delete('new-id'))
+        monitors_delete_mock.assert_has_calls([call('new-id')])
+        self.assertEquals(expected, client.monitors)

From 8ec84f49bb79d6b55a65ca50aec64f5a7b4bac11 Mon Sep 17 00:00:00 2001
From: Ross McFarland <ross@github.com>
Date: Fri, 13 Dec 2019 12:39:14 -0800
Subject: [PATCH 16/56] More ns1 code coverage, bug fix for monitor matching

---
 octodns/provider/ns1.py            |   7 +-
 tests/test_octodns_provider_ns1.py | 157 +++++++++++++++++++++++++++++
 2 files changed, 162 insertions(+), 2 deletions(-)

diff --git a/octodns/provider/ns1.py b/octodns/provider/ns1.py
index a3bd647..8f13ee5 100644
--- a/octodns/provider/ns1.py
+++ b/octodns/provider/ns1.py
@@ -602,7 +602,7 @@ class Ns1Provider(BaseProvider):
 
             for monitor in self._client.monitors.values():
                 data = self._parse_notes(monitor['notes'])
-                if expected_host == data['host'] or \
+                if expected_host == data['host'] and \
                    expected_type == data['type']:
                     # This monitor does not belong to this record
                     config = monitor['config']
@@ -611,11 +611,14 @@ class Ns1Provider(BaseProvider):
 
         return monitors
 
+    def _uuid(self):
+        return uuid4().hex
+
     def _create_feed(self, monitor):
         monitor_id = monitor['id']
         self.log.debug('_create_feed: monitor=%s', monitor_id)
         # TODO: looks like length limit is 64 char
-        name = '{} - {}'.format(monitor['name'], uuid4().hex[:6])
+        name = '{} - {}'.format(monitor['name'], self._uuid()[:6])
 
         # Create the data feed
         config = {
diff --git a/tests/test_octodns_provider_ns1.py b/tests/test_octodns_provider_ns1.py
index 4849684..ea7dcf2 100644
--- a/tests/test_octodns_provider_ns1.py
+++ b/tests/test_octodns_provider_ns1.py
@@ -518,6 +518,36 @@ class TestNs1Provider(TestCase):
 
 
 class TestNs1ProviderDynamic(TestCase):
+    zone = Zone('unit.tests.', [])
+
+    record = Record.new(zone, '', {
+        'dynamic': {
+            'pools': {
+                'iad': {
+                    'values': [{
+                        'value': '1.2.3.4',
+                    }, {
+                        'value': '2.3.4.5',
+                    }],
+                },
+            },
+            'rules': [{
+                'pool': 'iad',
+            }],
+        },
+        'octodns': {
+            'healthcheck': {
+                'host': 'send.me',
+                'path': '/_ping',
+                'port': 80,
+                'protocol': 'HTTP',
+            }
+        },
+        'ttl': 32,
+        'type': 'A',
+        'value': '1.2.3.4',
+        'meta': {},
+    })
 
     def test_notes(self):
         provider = Ns1Provider('test', 'api-key')
@@ -534,6 +564,133 @@ class TestNs1ProviderDynamic(TestCase):
         notes = provider._encode_notes(data)
         self.assertEquals(data, provider._parse_notes(notes))
 
+    def test_monitors_for(self):
+        provider = Ns1Provider('test', 'api-key')
+
+        # pre-populate the client's monitors cache
+        monitor_one = {
+            'config': {
+                'host': '1.2.3.4',
+            },
+            'notes': 'host:unit.tests type:A',
+        }
+        monitor_four = {
+            'config': {
+                'host': '2.3.4.5',
+            },
+            'notes': 'host:unit.tests type:A',
+        }
+        provider._client._monitors_cache = {
+            'one': monitor_one,
+            'two': {
+                'config': {
+                    'host': '8.8.8.8',
+                },
+                'notes': 'host:unit.tests type:AAAA',
+            },
+            'three': {
+                'config': {
+                    'host': '9.9.9.9',
+                },
+                'notes': 'host:other.unit.tests type:A',
+            },
+            'four': monitor_four,
+        }
+
+        # Would match, but won't get there b/c it's not dynamic
+        record = Record.new(self.zone, '', {
+            'ttl': 32,
+            'type': 'A',
+            'value': '1.2.3.4',
+            'meta': {},
+        })
+        self.assertEquals({}, provider._monitors_for(record))
+
+        # Will match some records
+        self.assertEquals({
+            '1.2.3.4': monitor_one,
+            '2.3.4.5': monitor_four,
+        }, provider._monitors_for(self.record))
+
+    def test_uuid(self):
+        # Just a smoke test/for coverage
+        provider = Ns1Provider('test', 'api-key')
+        self.assertTrue(provider._uuid())
+
+    @patch('octodns.provider.ns1.Ns1Provider._uuid')
+    @patch('ns1.rest.data.Feed.create')
+    def test_create_feed(self, datafeed_create_mock, uuid_mock):
+        provider = Ns1Provider('test', 'api-key')
+
+        # pre-fill caches to avoid extranious calls (things we're testing
+        # elsewhere)
+        provider._client._datasource_id = 'foo'
+        provider._client._feeds_for_monitors = {}
+
+        uuid_mock.reset_mock()
+        datafeed_create_mock.reset_mock()
+        uuid_mock.side_effect = ['xxxxxxxxxxxxxx']
+        feed = {
+            'id': 'feed',
+        }
+        datafeed_create_mock.side_effect = [feed]
+        monitor = {
+            'id': 'one',
+            'name': 'one name',
+            'config': {
+                'host': '1.2.3.4',
+            },
+            'notes': 'host:unit.tests type:A',
+        }
+        self.assertEquals('feed', provider._create_feed(monitor))
+        datafeed_create_mock.assert_has_calls([call('foo', 'one name - xxxxxx',
+                                                    {'jobid': 'one'})])
+
+    @patch('octodns.provider.ns1.Ns1Provider._create_feed')
+    @patch('octodns.provider.ns1.Ns1Client.monitors_create')
+    @patch('octodns.provider.ns1.Ns1Client.notifylists_create')
+    def test_create_monitor(self, notifylists_create_mock,
+                            monitors_create_mock, create_feed_mock):
+        provider = Ns1Provider('test', 'api-key')
+
+        # pre-fill caches to avoid extranious calls (things we're testing
+        # elsewhere)
+        provider._client._datasource_id = 'foo'
+        provider._client._feeds_for_monitors = {}
+
+        notifylists_create_mock.reset_mock()
+        monitors_create_mock.reset_mock()
+        create_feed_mock.reset_mock()
+        notifylists_create_mock.side_effect = [{
+            'id': 'nl-id',
+        }]
+        monitors_create_mock.side_effect = [{
+            'id': 'mon-id',
+        }]
+        create_feed_mock.side_effect = ['feed-id']
+        monitor = {
+            'name': 'test monitor',
+        }
+        monitor_id, feed_id = provider._create_monitor(monitor)
+        self.assertEquals('mon-id', monitor_id)
+        self.assertEquals('feed-id', feed_id)
+        monitors_create_mock.assert_has_calls([call(name='test monitor',
+                                                    notify_list='nl-id')])
+
+    def test_monitor_gen(self):
+        provider = Ns1Provider('test', 'api-key')
+
+        value = '3.4.5.6'
+        monitor = provider._monitor_gen(self.record, value)
+        self.assertEquals(value, monitor['config']['host'])
+        self.assertTrue('\\nHost: send.me\\r' in monitor['config']['send'])
+        self.assertFalse(monitor['config']['ssl'])
+        self.assertEquals('host:unit.tests type:A', monitor['notes'])
+
+        self.record._octodns['healthcheck']['protocol'] = 'HTTPS'
+        monitor = provider._monitor_gen(self.record, value)
+        self.assertTrue(monitor['config']['ssl'])
+
 
 class TestNs1Client(TestCase):
 

From 4022155b72f4f3526145e2c297fe00e2338ec3ff Mon Sep 17 00:00:00 2001
From: Ross McFarland <ross@github.com>
Date: Fri, 13 Dec 2019 13:07:32 -0800
Subject: [PATCH 17/56] Method naming consistency, test coverage for feeds and
 monitors

---
 octodns/provider/ns1.py            |  20 ++--
 tests/test_octodns_provider_ns1.py | 154 +++++++++++++++++++++++++++--
 2 files changed, 156 insertions(+), 18 deletions(-)

diff --git a/octodns/provider/ns1.py b/octodns/provider/ns1.py
index 8f13ee5..5cb7c2e 100644
--- a/octodns/provider/ns1.py
+++ b/octodns/provider/ns1.py
@@ -614,9 +614,9 @@ class Ns1Provider(BaseProvider):
     def _uuid(self):
         return uuid4().hex
 
-    def _create_feed(self, monitor):
+    def _feed_create(self, monitor):
         monitor_id = monitor['id']
-        self.log.debug('_create_feed: monitor=%s', monitor_id)
+        self.log.debug('_feed_create: monitor=%s', monitor_id)
         # TODO: looks like length limit is 64 char
         name = '{} - {}'.format(monitor['name'], self._uuid()[:6])
 
@@ -627,12 +627,12 @@ class Ns1Provider(BaseProvider):
         feed = self._client.datafeed_create(self._client.datasource_id, name,
                                             config)
         feed_id = feed['id']
-        self.log.debug('_create_feed:   feed=%s', feed_id)
+        self.log.debug('_feed_create:   feed=%s', feed_id)
 
         return feed_id
 
-    def _create_monitor(self, monitor):
-        self.log.debug('_create_monitor: monitor="%s"', monitor['name'])
+    def _monitor_create(self, monitor):
+        self.log.debug('_monitor_create: monitor="%s"', monitor['name'])
         # Create the notify list
         notify_list = [{
             'config': {
@@ -643,15 +643,15 @@ class Ns1Provider(BaseProvider):
         nl = self._client.notifylists_create(name=monitor['name'],
                                              notify_list=notify_list)
         nl_id = nl['id']
-        self.log.debug('_create_monitor:   notify_list=%s', nl_id)
+        self.log.debug('_monitor_create:   notify_list=%s', nl_id)
 
         # Create the monitor
         monitor['notify_list'] = nl_id
         monitor = self._client.monitors_create(**monitor)
         monitor_id = monitor['id']
-        self.log.debug('_create_monitor:   monitor=%s', monitor_id)
+        self.log.debug('_monitor_create:   monitor=%s', monitor_id)
 
-        return monitor_id, self._create_feed(monitor)
+        return monitor_id, self._feed_create(monitor)
 
     def _monitor_gen(self, record, value):
         host = record.fqdn[:-1]
@@ -718,11 +718,11 @@ class Ns1Provider(BaseProvider):
             if feed_id is None:
                 self.log.warn('_monitor_sync: %s (%s) missing feed, creating',
                               existing['name'], monitor_id)
-                feed_id = self._create_feed(existing)
+                feed_id = self._feed_create(existing)
         else:
             self.log.debug('_monitor_sync:   needs create')
             # We don't have an existing monitor create it (and related bits)
-            monitor_id, feed_id = self._create_monitor(expected)
+            monitor_id, feed_id = self._monitor_create(expected)
 
         return monitor_id, feed_id
 
diff --git a/tests/test_octodns_provider_ns1.py b/tests/test_octodns_provider_ns1.py
index ea7dcf2..994b84a 100644
--- a/tests/test_octodns_provider_ns1.py
+++ b/tests/test_octodns_provider_ns1.py
@@ -619,7 +619,7 @@ class TestNs1ProviderDynamic(TestCase):
 
     @patch('octodns.provider.ns1.Ns1Provider._uuid')
     @patch('ns1.rest.data.Feed.create')
-    def test_create_feed(self, datafeed_create_mock, uuid_mock):
+    def test_feed_create(self, datafeed_create_mock, uuid_mock):
         provider = Ns1Provider('test', 'api-key')
 
         # pre-fill caches to avoid extranious calls (things we're testing
@@ -642,15 +642,15 @@ class TestNs1ProviderDynamic(TestCase):
             },
             'notes': 'host:unit.tests type:A',
         }
-        self.assertEquals('feed', provider._create_feed(monitor))
+        self.assertEquals('feed', provider._feed_create(monitor))
         datafeed_create_mock.assert_has_calls([call('foo', 'one name - xxxxxx',
                                                     {'jobid': 'one'})])
 
-    @patch('octodns.provider.ns1.Ns1Provider._create_feed')
+    @patch('octodns.provider.ns1.Ns1Provider._feed_create')
     @patch('octodns.provider.ns1.Ns1Client.monitors_create')
     @patch('octodns.provider.ns1.Ns1Client.notifylists_create')
-    def test_create_monitor(self, notifylists_create_mock,
-                            monitors_create_mock, create_feed_mock):
+    def test_monitor_create(self, notifylists_create_mock,
+                            monitors_create_mock, feed_create_mock):
         provider = Ns1Provider('test', 'api-key')
 
         # pre-fill caches to avoid extranious calls (things we're testing
@@ -660,18 +660,18 @@ class TestNs1ProviderDynamic(TestCase):
 
         notifylists_create_mock.reset_mock()
         monitors_create_mock.reset_mock()
-        create_feed_mock.reset_mock()
+        feed_create_mock.reset_mock()
         notifylists_create_mock.side_effect = [{
             'id': 'nl-id',
         }]
         monitors_create_mock.side_effect = [{
             'id': 'mon-id',
         }]
-        create_feed_mock.side_effect = ['feed-id']
+        feed_create_mock.side_effect = ['feed-id']
         monitor = {
             'name': 'test monitor',
         }
-        monitor_id, feed_id = provider._create_monitor(monitor)
+        monitor_id, feed_id = provider._monitor_create(monitor)
         self.assertEquals('mon-id', monitor_id)
         self.assertEquals('feed-id', feed_id)
         monitors_create_mock.assert_has_calls([call(name='test monitor',
@@ -691,6 +691,144 @@ class TestNs1ProviderDynamic(TestCase):
         monitor = provider._monitor_gen(self.record, value)
         self.assertTrue(monitor['config']['ssl'])
 
+    def test_monitor_is_match(self):
+        provider = Ns1Provider('test', 'api-key')
+
+        # Empty matches empty
+        self.assertTrue(provider._monitor_is_match({}, {}))
+
+        # Anything matches empty
+        self.assertTrue(provider._monitor_is_match({}, {
+            'anything': 'goes'
+        }))
+
+        # Missing doesn't match
+        self.assertFalse(provider._monitor_is_match({
+            'exepct': 'this',
+        }, {
+            'anything': 'goes'
+        }))
+
+        # Identical matches
+        self.assertTrue(provider._monitor_is_match({
+            'exepct': 'this',
+        }, {
+            'exepct': 'this',
+        }))
+
+        # Different values don't match
+        self.assertFalse(provider._monitor_is_match({
+            'exepct': 'this',
+        }, {
+            'exepct': 'that',
+        }))
+
+        # Different sub-values don't match
+        self.assertFalse(provider._monitor_is_match({
+            'exepct': {
+                'this': 'to-be',
+            },
+        }, {
+            'exepct': {
+                'this': 'something-else',
+            },
+        }))
+
+    @patch('octodns.provider.ns1.Ns1Provider._feed_create')
+    @patch('octodns.provider.ns1.Ns1Client.monitors_update')
+    @patch('octodns.provider.ns1.Ns1Provider._monitor_create')
+    @patch('octodns.provider.ns1.Ns1Provider._monitor_gen')
+    def test_monitor_sync(self, monitor_gen_mock, monitor_create_mock,
+                          monitors_update_mock, feed_create_mock):
+        provider = Ns1Provider('test', 'api-key')
+
+        # pre-fill caches to avoid extranious calls (things we're testing
+        # elsewhere)
+        provider._client._datasource_id = 'foo'
+        provider._client._feeds_for_monitors = {
+            'mon-id': 'feed-id',
+        }
+
+        # No existing monitor
+        monitor_gen_mock.reset_mock()
+        monitor_create_mock.reset_mock()
+        monitors_update_mock.reset_mock()
+        feed_create_mock.reset_mock()
+        monitor_gen_mock.side_effect = [{'key': 'value'}]
+        monitor_create_mock.side_effect = [('mon-id', 'feed-id')]
+        value = '1.2.3.4'
+        monitor_id, feed_id = provider._monitor_sync(self.record, value, None)
+        self.assertEquals('mon-id', monitor_id)
+        self.assertEquals('feed-id', feed_id)
+        monitor_gen_mock.assert_has_calls([call(self.record, value)])
+        monitor_create_mock.assert_has_calls([call({'key': 'value'})])
+        monitors_update_mock.assert_not_called()
+        feed_create_mock.assert_not_called()
+
+        # Existing monitor that doesn't need updates
+        monitor_gen_mock.reset_mock()
+        monitor_create_mock.reset_mock()
+        monitors_update_mock.reset_mock()
+        feed_create_mock.reset_mock()
+        monitor = {
+            'id': 'mon-id',
+            'key': 'value',
+            'name': 'monitor name',
+        }
+        monitor_gen_mock.side_effect = [monitor]
+        monitor_id, feed_id = provider._monitor_sync(self.record, value,
+                                                     monitor)
+        self.assertEquals('mon-id', monitor_id)
+        self.assertEquals('feed-id', feed_id)
+        monitor_gen_mock.assert_called_once()
+        monitor_create_mock.assert_not_called()
+        monitors_update_mock.assert_not_called()
+        feed_create_mock.assert_not_called()
+
+        # Existing monitor that doesn't need updates, but is missing its feed
+        monitor_gen_mock.reset_mock()
+        monitor_create_mock.reset_mock()
+        monitors_update_mock.reset_mock()
+        feed_create_mock.reset_mock()
+        monitor = {
+            'id': 'mon-id2',
+            'key': 'value',
+            'name': 'monitor name',
+        }
+        monitor_gen_mock.side_effect = [monitor]
+        feed_create_mock.side_effect = ['feed-id2']
+        monitor_id, feed_id = provider._monitor_sync(self.record, value,
+                                                     monitor)
+        self.assertEquals('mon-id2', monitor_id)
+        self.assertEquals('feed-id2', feed_id)
+        monitor_gen_mock.assert_called_once()
+        monitor_create_mock.assert_not_called()
+        monitors_update_mock.assert_not_called()
+        feed_create_mock.assert_has_calls([call(monitor)])
+
+        # Existing monitor that needs updates
+        monitor_gen_mock.reset_mock()
+        monitor_create_mock.reset_mock()
+        monitors_update_mock.reset_mock()
+        feed_create_mock.reset_mock()
+        monitor = {
+            'id': 'mon-id',
+            'key': 'value',
+            'name': 'monitor name',
+        }
+        gened = {
+            'other': 'thing',
+        }
+        monitor_gen_mock.side_effect = [gened]
+        monitor_id, feed_id = provider._monitor_sync(self.record, value,
+                                                     monitor)
+        self.assertEquals('mon-id', monitor_id)
+        self.assertEquals('feed-id', feed_id)
+        monitor_gen_mock.assert_called_once()
+        monitor_create_mock.assert_not_called()
+        monitors_update_mock.assert_has_calls([call('mon-id', other='thing')])
+        feed_create_mock.assert_not_called()
+
 
 class TestNs1Client(TestCase):
 

From 0f298e51bef5b93f0722d335f8f9515964eb51db Mon Sep 17 00:00:00 2001
From: Ross McFarland <ross@github.com>
Date: Fri, 13 Dec 2019 13:22:54 -0800
Subject: [PATCH 18/56] Tests for ns1 _monitors_gc

---
 octodns/provider/ns1.py            | 12 ++---
 tests/test_octodns_provider_ns1.py | 83 ++++++++++++++++++++++++++++++
 2 files changed, 89 insertions(+), 6 deletions(-)

diff --git a/octodns/provider/ns1.py b/octodns/provider/ns1.py
index 5cb7c2e..e8b8d17 100644
--- a/octodns/provider/ns1.py
+++ b/octodns/provider/ns1.py
@@ -726,8 +726,8 @@ class Ns1Provider(BaseProvider):
 
         return monitor_id, feed_id
 
-    def _gc_monitors(self, record, active_monitor_ids=None):
-        self.log.debug('_gc_monitors: record=%s, active_monitor_ids=%s',
+    def _monitors_gc(self, record, active_monitor_ids=None):
+        self.log.debug('_monitors_gc: record=%s, active_monitor_ids=%s',
                        record.fqdn, active_monitor_ids)
 
         if active_monitor_ids is None:
@@ -738,7 +738,7 @@ class Ns1Provider(BaseProvider):
             if monitor_id in active_monitor_ids:
                 continue
 
-            self.log.debug('_gc_monitors:   deleting %s', monitor_id)
+            self.log.debug('_monitors_gc:   deleting %s', monitor_id)
 
             feed_id = self._client.feeds_for_monitors.get(monitor_id)
             if feed_id:
@@ -957,7 +957,7 @@ class Ns1Provider(BaseProvider):
         params, active_monitor_ids = \
             getattr(self, '_params_for_{}'.format(_type))(new)
         self._client.records_create(zone, domain, _type, **params)
-        self._gc_monitors(new, active_monitor_ids)
+        self._monitors_gc(new, active_monitor_ids)
 
     def _apply_Update(self, ns1_zone, change):
         new = change.new
@@ -967,7 +967,7 @@ class Ns1Provider(BaseProvider):
         params, active_monitor_ids = \
             getattr(self, '_params_for_{}'.format(_type))(new)
         self._client.records_update(zone, domain, _type, **params)
-        self._gc_monitors(new, active_monitor_ids)
+        self._monitors_gc(new, active_monitor_ids)
 
     def _apply_Delete(self, ns1_zone, change):
         existing = change.existing
@@ -975,7 +975,7 @@ class Ns1Provider(BaseProvider):
         domain = existing.fqdn[:-1]
         _type = existing._type
         self._client.records_delete(zone, domain, _type)
-        self._gc_monitors(existing)
+        self._monitors_gc(existing)
 
     def _apply(self, plan):
         desired = plan.desired
diff --git a/tests/test_octodns_provider_ns1.py b/tests/test_octodns_provider_ns1.py
index 994b84a..68e87c9 100644
--- a/tests/test_octodns_provider_ns1.py
+++ b/tests/test_octodns_provider_ns1.py
@@ -829,6 +829,89 @@ class TestNs1ProviderDynamic(TestCase):
         monitors_update_mock.assert_has_calls([call('mon-id', other='thing')])
         feed_create_mock.assert_not_called()
 
+    @patch('octodns.provider.ns1.Ns1Client.notifylists_delete')
+    @patch('octodns.provider.ns1.Ns1Client.monitors_delete')
+    @patch('octodns.provider.ns1.Ns1Client.datafeed_delete')
+    @patch('octodns.provider.ns1.Ns1Provider._monitors_for')
+    def test_monitors_gc(self, monitors_for_mock, datafeed_delete_mock,
+                         monitors_delete_mock, notifylists_delete_mock):
+        provider = Ns1Provider('test', 'api-key')
+
+        # pre-fill caches to avoid extranious calls (things we're testing
+        # elsewhere)
+        provider._client._datasource_id = 'foo'
+        provider._client._feeds_for_monitors = {
+            'mon-id': 'feed-id',
+        }
+
+        # No active monitors and no existing, nothing will happen
+        monitors_for_mock.reset_mock()
+        datafeed_delete_mock.reset_mock()
+        monitors_delete_mock.reset_mock()
+        notifylists_delete_mock.reset_mock()
+        monitors_for_mock.side_effect = [{}]
+        provider._monitors_gc(self.record)
+        monitors_for_mock.assert_has_calls([call(self.record)])
+        datafeed_delete_mock.assert_not_called()
+        monitors_delete_mock.assert_not_called()
+        notifylists_delete_mock.assert_not_called()
+
+        # No active monitors and one existing, delete all the things
+        monitors_for_mock.reset_mock()
+        datafeed_delete_mock.reset_mock()
+        monitors_delete_mock.reset_mock()
+        notifylists_delete_mock.reset_mock()
+        monitors_for_mock.side_effect = [{
+            'x': {
+                'id': 'mon-id',
+                'notify_list': 'nl-id',
+            }
+        }]
+        provider._monitors_gc(self.record)
+        monitors_for_mock.assert_has_calls([call(self.record)])
+        datafeed_delete_mock.assert_has_calls([call('foo', 'feed-id')])
+        monitors_delete_mock.assert_has_calls([call('mon-id')])
+        notifylists_delete_mock.assert_has_calls([call('nl-id')])
+
+        # Same existing, this time in active list, should be noop
+        monitors_for_mock.reset_mock()
+        datafeed_delete_mock.reset_mock()
+        monitors_delete_mock.reset_mock()
+        notifylists_delete_mock.reset_mock()
+        monitors_for_mock.side_effect = [{
+            'x': {
+                'id': 'mon-id',
+                'notify_list': 'nl-id',
+            }
+        }]
+        provider._monitors_gc(self.record, {'mon-id'})
+        monitors_for_mock.assert_has_calls([call(self.record)])
+        datafeed_delete_mock.assert_not_called()
+        monitors_delete_mock.assert_not_called()
+        notifylists_delete_mock.assert_not_called()
+
+        # Non-active monitor w/o a feed, and another monitor that's left alone
+        # b/c it's active
+        monitors_for_mock.reset_mock()
+        datafeed_delete_mock.reset_mock()
+        monitors_delete_mock.reset_mock()
+        notifylists_delete_mock.reset_mock()
+        monitors_for_mock.side_effect = [{
+            'x': {
+                'id': 'mon-id',
+                'notify_list': 'nl-id',
+            },
+            'y': {
+                'id': 'mon-id2',
+                'notify_list': 'nl-id2',
+            },
+        }]
+        provider._monitors_gc(self.record, {'mon-id'})
+        monitors_for_mock.assert_has_calls([call(self.record)])
+        datafeed_delete_mock.assert_not_called()
+        monitors_delete_mock.assert_has_calls([call('mon-id2')])
+        notifylists_delete_mock.assert_has_calls([call('nl-id2')])
+
 
 class TestNs1Client(TestCase):
 

From ed482c60ca8c1f1c3cb4131da6e1a4cd64651c65 Mon Sep 17 00:00:00 2001
From: Dan Hanks <dhanks@adobe.com>
Date: Thu, 19 Dec 2019 10:33:22 -0500
Subject: [PATCH 19/56] Document max_workers flag

---
 README.md | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/README.md b/README.md
index c3ddb0b..83e28ca 100644
--- a/README.md
+++ b/README.md
@@ -51,6 +51,9 @@ We start by creating a config file to tell OctoDNS about our providers and the z
 
 ```yaml
 ---
+manager:
+  max_workers: 2
+
 providers:
   config:
     class: octodns.provider.yaml.YamlProvider
@@ -80,6 +83,8 @@ zones:
 
 Further information can be found in the `docstring` of each source and provider class.
 
+The `max_workers` key in the `manager` section of the config enables threading to parallelize the planning portion of the sync.
+
 Now that we have something to tell OctoDNS about our providers & zones we need to tell it about or records. We'll keep it simple for now and just create a single `A` record at the top-level of the domain.
 
 `config/example.com.yaml`

From 561a6ca2d98633023a131e491f3b24212ce447fb Mon Sep 17 00:00:00 2001
From: Ross McFarland <ross@github.com>
Date: Mon, 6 Jan 2020 08:31:35 -0800
Subject: [PATCH 20/56] Test coverage for Ns1Provider _params_for_dynamic_A

---
 tests/test_octodns_provider_ns1.py | 45 ++++++++++++++++++++++++++++++
 1 file changed, 45 insertions(+)

diff --git a/tests/test_octodns_provider_ns1.py b/tests/test_octodns_provider_ns1.py
index 68e87c9..bc40f8a 100644
--- a/tests/test_octodns_provider_ns1.py
+++ b/tests/test_octodns_provider_ns1.py
@@ -523,6 +523,12 @@ class TestNs1ProviderDynamic(TestCase):
     record = Record.new(zone, '', {
         'dynamic': {
             'pools': {
+                'lhr': {
+                    'fallback': 'iad',
+                    'values': [{
+                        'value': '3.4.5.6',
+                    }],
+                },
                 'iad': {
                     'values': [{
                         'value': '1.2.3.4',
@@ -532,6 +538,13 @@ class TestNs1ProviderDynamic(TestCase):
                 },
             },
             'rules': [{
+                'geos': [
+                    'AF',
+                    'EU-GB',
+                    'NA-US-FL'
+                ],
+                'pool': 'lhr',
+            }, {
                 'pool': 'iad',
             }],
         },
@@ -912,6 +925,38 @@ class TestNs1ProviderDynamic(TestCase):
         monitors_delete_mock.assert_has_calls([call('mon-id2')])
         notifylists_delete_mock.assert_has_calls([call('nl-id2')])
 
+    @patch('octodns.provider.ns1.Ns1Provider._monitor_sync')
+    @patch('octodns.provider.ns1.Ns1Provider._monitors_for')
+    def test_params_for_dynamic(self, monitors_for_mock, monitors_sync_mock):
+        provider = Ns1Provider('test', 'api-key')
+
+        # pre-fill caches to avoid extranious calls (things we're testing
+        # elsewhere)
+        provider._client._datasource_id = 'foo'
+        provider._client._feeds_for_monitors = {
+            'mon-id': 'feed-id',
+        }
+
+        monitors_for_mock.reset_mock()
+        monitors_sync_mock.reset_mock()
+        monitors_for_mock.side_effect = [{
+            '3.4.5.6': 'mid-3',
+        }]
+        monitors_sync_mock.side_effect = [
+            ('mid-1', 'fid-1'),
+            ('mid-2', 'fid-2'),
+            ('mid-3', 'fid-3'),
+        ]
+        # This indirectly calls into _params_for_dynamic_A and tests the
+        # handling to get there
+        provider._params_for_A(self.record)
+        monitors_for_mock.assert_has_calls([call(self.record)])
+        monitors_sync_mock.assert_has_calls([
+            call(self.record, '1.2.3.4', None),
+            call(self.record, '2.3.4.5', None),
+            call(self.record, '3.4.5.6', 'mid-3'),
+        ])
+
 
 class TestNs1Client(TestCase):
 

From 69cd30a1832c30a998528c4c6549f460ad1ee128 Mon Sep 17 00:00:00 2001
From: Ross McFarland <ross@github.com>
Date: Mon, 6 Jan 2020 09:18:10 -0800
Subject: [PATCH 21/56] Coverage for Ns1Provider _data_for_dynamic_A

---
 tests/test_octodns_provider_ns1.py | 136 ++++++++++++++++++++++++++++-
 1 file changed, 135 insertions(+), 1 deletion(-)

diff --git a/tests/test_octodns_provider_ns1.py b/tests/test_octodns_provider_ns1.py
index bc40f8a..a985c13 100644
--- a/tests/test_octodns_provider_ns1.py
+++ b/tests/test_octodns_provider_ns1.py
@@ -13,7 +13,7 @@ from six import text_type
 from unittest import TestCase
 
 from octodns.record import Delete, Record, Update
-from octodns.provider.ns1 import Ns1Client, Ns1Provider
+from octodns.provider.ns1 import Ns1Client, Ns1Exception, Ns1Provider
 from octodns.zone import Zone
 
 
@@ -957,6 +957,140 @@ class TestNs1ProviderDynamic(TestCase):
             call(self.record, '3.4.5.6', 'mid-3'),
         ])
 
+    def test_data_for_dynamic_A(self):
+        provider = Ns1Provider('test', 'api-key')
+
+        # Unexpected filters throws an error
+        ns1_record = {
+            'domain': 'unit.tests',
+            'filters': [],
+        }
+        with self.assertRaises(Ns1Exception) as ctx:
+            provider._data_for_dynamic_A('A', ns1_record)
+        self.assertEquals('Unrecognized advanced record',
+                          text_type(ctx.exception))
+
+        # empty record turns into empty data
+        ns1_record = {
+            'answers': [],
+            'domain': 'unit.tests',
+            'filters': Ns1Provider._DYNAMIC_FILTERS,
+            'regions': {},
+            'ttl': 42,
+        }
+        data = provider._data_for_dynamic_A('A', ns1_record)
+        self.assertEquals({
+            'dynamic': {
+                'pools': {},
+                'rules': [],
+            },
+            'ttl': 42,
+            'type': 'A',
+            'values': [],
+        }, data)
+
+        # Test out a small, but realistic setup that covers all the options
+        ns1_record = {
+            'answers': [{
+                'answer': ['3.4.5.6'],
+                'meta': {
+                    'priority': 1,
+                    'note': 'from:lhr',
+                },
+                'region': 'lhr',
+            }, {
+                'answer': ['2.3.4.5'],
+                'meta': {
+                    'priority': 2,
+                    'weight': 12,
+                    'note': 'from:iad',
+                },
+                'region': 'lhr',
+            }, {
+                'answer': ['1.2.3.4'],
+                'meta': {
+                    'priority': 3,
+                    'note': 'from:--default--',
+                },
+                'region': 'lhr',
+            }, {
+                'answer': ['2.3.4.5'],
+                'meta': {
+                    'priority': 1,
+                    'weight': 12,
+                    'note': 'from:iad',
+                },
+                'region': 'iad',
+            }, {
+                'answer': ['1.2.3.4'],
+                'meta': {
+                    'priority': 2,
+                    'note': 'from:--default--',
+                },
+                'region': 'iad',
+            }],
+            'domain': 'unit.tests',
+            'filters': Ns1Provider._DYNAMIC_FILTERS,
+            'regions': {
+                'lhr': {
+                    'meta': {
+                        'note': 'rule-order:1 fallback:iad',
+                        'country': ['CA'],
+                        'georegion': ['AFRICA'],
+                        'us_state': ['OR'],
+                    },
+                },
+                'iad': {
+                    'meta': {
+                        'note': 'rule-order:2',
+                    },
+                }
+            },
+            'tier': 3,
+            'ttl': 42,
+        }
+        data = provider._data_for_dynamic_A('A', ns1_record)
+        self.assertEquals({
+            'dynamic': {
+                'pools': {
+                    'iad': {
+                        'fallback': None,
+                        'values': [{
+                            'value': '2.3.4.5',
+                            'weight': 12,
+                        }],
+                    },
+                    'lhr': {
+                        'fallback': 'iad',
+                        'values': [{
+                            'weight': 1,
+                            'value': '3.4.5.6',
+                        }],
+                    },
+                },
+                'rules': [{
+                    '_order': '1',
+                    'geos': [
+                        'AF',
+                        'NA-CA',
+                        'NA-US-OR',
+                    ],
+                    'pool': 'lhr',
+                }, {
+                    '_order': '2',
+                    'pool': 'iad',
+                }],
+            },
+            'ttl': 42,
+            'type': 'A',
+            'values': ['1.2.3.4'],
+        }, data)
+
+        # Same answer if we go through _data_for_A which out sources the job to
+        # _data_for_dynamic_A
+        data2 = provider._data_for_A('A', ns1_record)
+        self.assertEquals(data, data2)
+
 
 class TestNs1Client(TestCase):
 

From eefd83de80de20ed4937f892f7da235c63d1a0e2 Mon Sep 17 00:00:00 2001
From: Ross McFarland <ross@github.com>
Date: Mon, 6 Jan 2020 10:04:07 -0800
Subject: [PATCH 22/56] Coverage for Ns1Provider _extra_changes

---
 tests/test_octodns_provider_ns1.py | 107 +++++++++++++++++++++++++++++
 1 file changed, 107 insertions(+)

diff --git a/tests/test_octodns_provider_ns1.py b/tests/test_octodns_provider_ns1.py
index a985c13..4c505e4 100644
--- a/tests/test_octodns_provider_ns1.py
+++ b/tests/test_octodns_provider_ns1.py
@@ -1091,6 +1091,113 @@ class TestNs1ProviderDynamic(TestCase):
         data2 = provider._data_for_A('A', ns1_record)
         self.assertEquals(data, data2)
 
+    @patch('octodns.provider.ns1.Ns1Provider._monitors_for')
+    def test_extra_changes(self, monitors_for_mock):
+        provider = Ns1Provider('test', 'api-key')
+
+        desired = Zone('unit.tests.', [])
+
+        # Empty zone and no changes
+        monitors_for_mock.reset_mock()
+        extra = provider._extra_changes(desired, [])
+        self.assertFalse(extra)
+        monitors_for_mock.assert_not_called()
+
+        # Simple record, ignored
+        monitors_for_mock.reset_mock()
+        simple = Record.new(desired, '', {
+            'ttl': 32,
+            'type': 'A',
+            'value': '1.2.3.4',
+            'meta': {},
+        })
+        desired.add_record(simple)
+        extra = provider._extra_changes(desired, [])
+        self.assertFalse(extra)
+        monitors_for_mock.assert_not_called()
+
+        # Dynamic record, inspectable
+        dynamic = Record.new(desired, 'dyn', {
+            'dynamic': {
+                'pools': {
+                    'iad': {
+                        'values': [{
+                            'value': '1.2.3.4',
+                        }],
+                    },
+                },
+                'rules': [{
+                    'pool': 'iad',
+                }],
+            },
+            'octodns': {
+                'healthcheck': {
+                    'host': 'send.me',
+                    'path': '/_ping',
+                    'port': 80,
+                    'protocol': 'HTTP',
+                }
+            },
+            'ttl': 32,
+            'type': 'A',
+            'value': '1.2.3.4',
+            'meta': {},
+        })
+        desired.add_record(dynamic)
+
+        # untouched, but everything in sync so no change needed
+        monitors_for_mock.reset_mock()
+        # Generate what we expect to have
+        gend = provider._monitor_gen(dynamic, '1.2.3.4')
+        gend.update({
+            'id': 'mid',  # need to add an id
+            'notify_list': 'xyz',  # need to add a notify list (for now)
+        })
+        monitors_for_mock.side_effect = [{
+            '1.2.3.4': gend,
+        }]
+        extra = provider._extra_changes(desired, [])
+        self.assertFalse(extra)
+        monitors_for_mock.assert_has_calls([call(dynamic)])
+
+        update = Update(dynamic, dynamic)
+
+        # If we don't have a notify list we're broken and we'll expect to see
+        # an Update
+        monitors_for_mock.reset_mock()
+        del gend['notify_list']
+        monitors_for_mock.side_effect = [{
+            '1.2.3.4': gend,
+        }]
+        extra = provider._extra_changes(desired, [])
+        self.assertEquals(1, len(extra))
+        extra = list(extra)[0]
+        self.assertIsInstance(extra, Update)
+        self.assertEquals(dynamic, extra.new)
+        monitors_for_mock.assert_has_calls([call(dynamic)])
+
+        # Add notify_list back and change the healthcheck protocol, we'll still
+        # expect to see an update
+        monitors_for_mock.reset_mock()
+        gend['notify_list'] = 'xyz'
+        dynamic._octodns['healthcheck']['protocol'] = 'HTTPS'
+        del gend['notify_list']
+        monitors_for_mock.side_effect = [{
+            '1.2.3.4': gend,
+        }]
+        extra = provider._extra_changes(desired, [])
+        self.assertEquals(1, len(extra))
+        extra = list(extra)[0]
+        self.assertIsInstance(extra, Update)
+        self.assertEquals(dynamic, extra.new)
+        monitors_for_mock.assert_has_calls([call(dynamic)])
+
+        # If it's in the changed list, it'll be ignored
+        monitors_for_mock.reset_mock()
+        extra = provider._extra_changes(desired, [update])
+        self.assertFalse(extra)
+        monitors_for_mock.assert_not_called()
+
 
 class TestNs1Client(TestCase):
 

From f91cac3ef47595f40cb53a96e88c27a59bdf27c4 Mon Sep 17 00:00:00 2001
From: Ross McFarland <ross@github.com>
Date: Mon, 6 Jan 2020 10:13:58 -0800
Subject: [PATCH 23/56] coverage for Ns1Client notifylist methods

---
 tests/test_octodns_provider_ns1.py | 45 ++++++++++++++++++++++++++++++
 1 file changed, 45 insertions(+)

diff --git a/tests/test_octodns_provider_ns1.py b/tests/test_octodns_provider_ns1.py
index 4c505e4..0d456fe 100644
--- a/tests/test_octodns_provider_ns1.py
+++ b/tests/test_octodns_provider_ns1.py
@@ -1408,3 +1408,48 @@ class TestNs1Client(TestCase):
         self.assertEquals('deleted', client.monitors_delete('new-id'))
         monitors_delete_mock.assert_has_calls([call('new-id')])
         self.assertEquals(expected, client.monitors)
+
+    @patch('ns1.rest.monitoring.NotifyLists.delete')
+    @patch('ns1.rest.monitoring.NotifyLists.create')
+    @patch('ns1.rest.monitoring.NotifyLists.list')
+    def test_notifylists(self, notifylists_list_mock, notifylists_create_mock,
+                         notifylists_delete_mock):
+        client = Ns1Client('dummy-key')
+
+        notifylists_list_mock.reset_mock()
+        notifylists_create_mock.reset_mock()
+        notifylists_delete_mock.reset_mock()
+        notifylists_create_mock.side_effect = ['bar']
+        notify_list = [{
+            'config': {
+                'sourceid': 'foo',
+            },
+            'type': 'datafeed',
+        }]
+        nl = client.notifylists_create(name='some name',
+                                       notify_list=notify_list)
+        self.assertEquals('bar', nl)
+        notifylists_list_mock.assert_not_called()
+        notifylists_create_mock.assert_has_calls([
+            call({'name': 'some name', 'notify_list': notify_list})
+        ])
+        notifylists_delete_mock.assert_not_called()
+
+        notifylists_list_mock.reset_mock()
+        notifylists_create_mock.reset_mock()
+        notifylists_delete_mock.reset_mock()
+        client.notifylists_delete('nlid')
+        notifylists_list_mock.assert_not_called()
+        notifylists_create_mock.assert_not_called()
+        notifylists_delete_mock.assert_has_calls([call('nlid')])
+
+        notifylists_list_mock.reset_mock()
+        notifylists_create_mock.reset_mock()
+        notifylists_delete_mock.reset_mock()
+        expected = ['one', 'two', 'three']
+        notifylists_list_mock.side_effect = [expected]
+        nls = client.notifylists_list()
+        self.assertEquals(expected, nls)
+        notifylists_list_mock.assert_has_calls([call()])
+        notifylists_create_mock.assert_not_called()
+        notifylists_delete_mock.assert_not_called()

From 95f51114871ba552d3e42d408237a8424df30820 Mon Sep 17 00:00:00 2001
From: Ross McFarland <ross@github.com>
Date: Mon, 6 Jan 2020 10:18:56 -0800
Subject: [PATCH 24/56] NS1 geo records will always use 'answers'

---
 octodns/provider/ns1.py | 2 --
 1 file changed, 2 deletions(-)

diff --git a/octodns/provider/ns1.py b/octodns/provider/ns1.py
index e8b8d17..e9efd3e 100644
--- a/octodns/provider/ns1.py
+++ b/octodns/provider/ns1.py
@@ -256,8 +256,6 @@ class Ns1Provider(BaseProvider):
             'type': _type,
         }
         values, codes = [], []
-        if 'answers' not in record:
-            values = record['short_answers']
         for answer in record.get('answers', []):
             meta = answer.get('meta', {})
             if meta:

From e0debc963e2b120d599cff5537a39db053ca8f92 Mon Sep 17 00:00:00 2001
From: Ross McFarland <ross@github.com>
Date: Mon, 6 Jan 2020 10:22:18 -0800
Subject: [PATCH 25/56] Update Dynamic support to include NS1, remove Geo
 mentions

---
 README.md | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/README.md b/README.md
index c3ddb0b..f884557 100644
--- a/README.md
+++ b/README.md
@@ -169,7 +169,7 @@ The above command pulled the existing data out of Route53 and placed the results
 
 ## Supported providers
 
-| Provider | Requirements | Record Support | Dynamic/Geo Support | Notes |
+| Provider | Requirements | Record Support | Dynamic | Notes |
 |--|--|--|--|--|
 | [AzureProvider](/octodns/provider/azuredns.py) | azure-mgmt-dns | A, AAAA, CAA, CNAME, MX, NS, PTR, SRV, TXT | No | |
 | [Akamai](/octodns/provider/fastdns.py) | edgegrid-python | A, AAAA, CNAME, MX, NAPTR, NS, PTR, SPF, SRV, SSHFP, TXT | No | |
@@ -182,7 +182,7 @@ The above command pulled the existing data out of Route53 and placed the results
 | [EtcHostsProvider](/octodns/provider/etc_hosts.py) | | A, AAAA, ALIAS, CNAME | No | |
 | [GoogleCloudProvider](/octodns/provider/googlecloud.py) | google-cloud-dns | A, AAAA, CAA, CNAME, MX, NAPTR, NS, PTR, SPF, SRV, TXT  | No | |
 | [MythicBeastsProvider](/octodns/provider/mythicbeasts.py) | Mythic Beasts | A, AAAA, ALIAS, CNAME, MX, NS, SRV, SSHFP, CAA, TXT | No | |
-| [Ns1Provider](/octodns/provider/ns1.py) | ns1-python | All | Partial Geo | No health checking for GeoDNS |
+| [Ns1Provider](/octodns/provider/ns1.py) | ns1-python | All | Yes | No CNAME support, missing `NA` geo target |
 | [OVH](/octodns/provider/ovh.py) | ovh | A, AAAA, CNAME, MX, NAPTR, NS, PTR, SPF, SRV, SSHFP, TXT, DKIM | No | |
 | [PowerDnsProvider](/octodns/provider/powerdns.py) | | All | No | |
 | [Rackspace](/octodns/provider/rackspace.py) | | A, AAAA, ALIAS, CNAME, MX, NS, PTR, SPF, TXT | No |  |

From 391ef583ae3f3a63ddda82f84e00490a7fc66061 Mon Sep 17 00:00:00 2001
From: Ross McFarland <ross@github.com>
Date: Mon, 6 Jan 2020 10:22:41 -0800
Subject: [PATCH 26/56] Ns1 should use geofence_regional to avoid nearest
 matching

---
 octodns/provider/ns1.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/octodns/provider/ns1.py b/octodns/provider/ns1.py
index e9efd3e..b442271 100644
--- a/octodns/provider/ns1.py
+++ b/octodns/provider/ns1.py
@@ -188,7 +188,7 @@ class Ns1Provider(BaseProvider):
         'filter': 'up'
     }, {
         'config': {},
-        'filter': u'geotarget_regional'
+        'filter': u'geofence_regional'
     }, {
         'config': {},
         'filter': u'select_first_region'

From e22a7d2738789f4d30934f9b388c2e60fec25bf6 Mon Sep 17 00:00:00 2001
From: Charles Durieux <charles@oply.com>
Date: Wed, 8 Jan 2020 17:45:02 +0100
Subject: [PATCH 27/56] Fix trailing semicolon in dkim for ovh provider

Pulling dns records from ovh to a yaml file puts a semicolon at the end.
Pushing from yaml to ovh will fail the "dkim-compliant" verification if
there is an empty field (and there is one in case of a trailing
semicolon). With the current logic, pulling dkim record created with ovh
then pushing it back will NOT work.

This small patch ignores all empty fields in a dkim records during dkim
validation.
---
 octodns/provider/ovh.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/octodns/provider/ovh.py b/octodns/provider/ovh.py
index 17aff8d..6bed788 100644
--- a/octodns/provider/ovh.py
+++ b/octodns/provider/ovh.py
@@ -323,7 +323,7 @@ class OvhProvider(BaseProvider):
                           'n': lambda _: True,
                           'g': lambda _: True}
 
-        splitted = value.split('\\;')
+        splitted = list(filter(None, value.split('\\;')))
         found_key = False
         for splitted_value in splitted:
             sub_split = [x.strip() for x in splitted_value.split("=", 1)]

From 4b625eba64f89c37b5ef5866de4d714f8134318b Mon Sep 17 00:00:00 2001
From: Kaari <durieux.charles01@gmail.com>
Date: Wed, 8 Jan 2020 19:52:13 +0100
Subject: [PATCH 28/56] Use comprehension for clarity and best practice

Co-Authored-By: Ross McFarland <ross@github.com>
---
 octodns/provider/ovh.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/octodns/provider/ovh.py b/octodns/provider/ovh.py
index 6bed788..8a3d492 100644
--- a/octodns/provider/ovh.py
+++ b/octodns/provider/ovh.py
@@ -323,7 +323,7 @@ class OvhProvider(BaseProvider):
                           'n': lambda _: True,
                           'g': lambda _: True}
 
-        splitted = list(filter(None, value.split('\\;')))
+        splitted = [v for v in value.split('\\;') if v]
         found_key = False
         for splitted_value in splitted:
             sub_split = [x.strip() for x in splitted_value.split("=", 1)]

From ae9e465d8ddf009762cc1ff1d413fae3816aa991 Mon Sep 17 00:00:00 2001
From: Christian Clauss <cclauss@me.com>
Date: Sat, 11 Jan 2020 04:52:11 +0100
Subject: [PATCH 29/56] Add dependabot to periodically refresh dependancies

As recommended by @ross at https://github.com/github/octodns/pull/441#discussion_r363515321
---
 .dependabot/config.yml | 6 ++++++
 1 file changed, 6 insertions(+)
 create mode 100644 .dependabot/config.yml

diff --git a/.dependabot/config.yml b/.dependabot/config.yml
new file mode 100644
index 0000000..165af5d
--- /dev/null
+++ b/.dependabot/config.yml
@@ -0,0 +1,6 @@
+version: 1
+
+update_configs:
+  - package_manager: "python"
+    directory: "/"
+    update_schedule: "weekly"

From fe58c67133d24bb728fc41d46079ffa17434e3d8 Mon Sep 17 00:00:00 2001
From: "dependabot-preview[bot]"
 <27856297+dependabot-preview[bot]@users.noreply.github.com>
Date: Sat, 11 Jan 2020 16:28:23 +0000
Subject: [PATCH 30/56] Bump python-dateutil from 2.6.1 to 2.8.1

Bumps [python-dateutil](https://github.com/dateutil/dateutil) from 2.6.1 to 2.8.1.
- [Release notes](https://github.com/dateutil/dateutil/releases)
- [Changelog](https://github.com/dateutil/dateutil/blob/master/NEWS)
- [Commits](https://github.com/dateutil/dateutil/compare/2.6.1...2.8.1)

Signed-off-by: dependabot-preview[bot] <support@dependabot.com>
---
 requirements.txt | 2 +-
 setup.py         | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/requirements.txt b/requirements.txt
index 6a26ad3..e227fa4 100644
--- a/requirements.txt
+++ b/requirements.txt
@@ -18,7 +18,7 @@ ns1-python==0.13.0
 ovh==0.4.8
 pycountry-convert==0.7.2
 pycountry==19.8.18
-python-dateutil==2.6.1
+python-dateutil==2.8.1
 requests==2.22.0
 s3transfer==0.1.13
 setuptools==40.3.0
diff --git a/setup.py b/setup.py
index 4f28232..4858e2b 100644
--- a/setup.py
+++ b/setup.py
@@ -74,7 +74,7 @@ setup(
         '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',
+        'python-dateutil>=2.6.0,<2.9.0',
         'requests>=2.20.0'
     ],
     license='MIT',

From c98ba64e8e4037101a6d5c5e5e75aee32f88d6a6 Mon Sep 17 00:00:00 2001
From: "dependabot-preview[bot]"
 <27856297+dependabot-preview[bot]@users.noreply.github.com>
Date: Sat, 11 Jan 2020 16:29:48 +0000
Subject: [PATCH 31/56] Bump six from 1.12.0 to 1.13.0

Bumps [six](https://github.com/benjaminp/six) from 1.12.0 to 1.13.0.
- [Release notes](https://github.com/benjaminp/six/releases)
- [Changelog](https://github.com/benjaminp/six/blob/master/CHANGES)
- [Commits](https://github.com/benjaminp/six/compare/1.12.0...1.13.0)

Signed-off-by: dependabot-preview[bot] <support@dependabot.com>
---
 requirements.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/requirements.txt b/requirements.txt
index 6a26ad3..88ba2b9 100644
--- a/requirements.txt
+++ b/requirements.txt
@@ -22,5 +22,5 @@ python-dateutil==2.6.1
 requests==2.22.0
 s3transfer==0.1.13
 setuptools==40.3.0
-six==1.12.0
+six==1.13.0
 transip==2.0.0

From 826df247b525137c85ddc8238eaff6ba28909094 Mon Sep 17 00:00:00 2001
From: Ross McFarland <ross@github.com>
Date: Sat, 11 Jan 2020 08:57:05 -0800
Subject: [PATCH 32/56] python-dateutil>=2.8.1

---
 setup.py | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/setup.py b/setup.py
index 4858e2b..c56aa82 100644
--- a/setup.py
+++ b/setup.py
@@ -73,8 +73,7 @@ setup(
         '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.9.0',
+        'python-dateutil>=2.8.1',
         'requests>=2.20.0'
     ],
     license='MIT',

From 5316dddbc6b63e70b8c31bbdfda53a13cbf50e78 Mon Sep 17 00:00:00 2001
From: "dependabot-preview[bot]"
 <27856297+dependabot-preview[bot]@users.noreply.github.com>
Date: Sat, 11 Jan 2020 17:05:32 +0000
Subject: [PATCH 33/56] Bump google-cloud-core from 0.28.1 to 1.1.0

Bumps [google-cloud-core](https://github.com/GoogleCloudPlatform/google-cloud-python) from 0.28.1 to 1.1.0.
- [Release notes](https://github.com/GoogleCloudPlatform/google-cloud-python/releases)
- [Changelog](https://github.com/googleapis/google-cloud-python/blob/master/CHANGELOG.md)
- [Commits](https://github.com/GoogleCloudPlatform/google-cloud-python/compare/core-0.28.1...kms-1.1.0)

Signed-off-by: dependabot-preview[bot] <support@dependabot.com>
---
 requirements.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/requirements.txt b/requirements.txt
index 985b044..d06cb3f 100644
--- a/requirements.txt
+++ b/requirements.txt
@@ -8,7 +8,7 @@ docutils==0.14
 dyn==1.8.1
 edgegrid-python==1.1.1
 futures==3.2.0; python_version < '3.0'
-google-cloud-core==0.28.1
+google-cloud-core==1.1.0
 google-cloud-dns==0.29.0
 ipaddress==1.0.22
 jmespath==0.9.3

From d4be1036a4f3ffb2e19bcc8e64b9dd036ce7c1ff Mon Sep 17 00:00:00 2001
From: "dependabot-preview[bot]"
 <27856297+dependabot-preview[bot]@users.noreply.github.com>
Date: Sat, 11 Jan 2020 17:13:57 +0000
Subject: [PATCH 34/56] Bump s3transfer from 0.1.13 to 0.3.0

Bumps [s3transfer](https://github.com/boto/s3transfer) from 0.1.13 to 0.3.0.
- [Release notes](https://github.com/boto/s3transfer/releases)
- [Changelog](https://github.com/boto/s3transfer/blob/develop/CHANGELOG.rst)
- [Commits](https://github.com/boto/s3transfer/compare/0.1.13...0.3.0)

Signed-off-by: dependabot-preview[bot] <support@dependabot.com>
---
 requirements.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/requirements.txt b/requirements.txt
index d06cb3f..8769223 100644
--- a/requirements.txt
+++ b/requirements.txt
@@ -20,7 +20,7 @@ pycountry-convert==0.7.2
 pycountry==19.8.18
 python-dateutil==2.8.1
 requests==2.22.0
-s3transfer==0.1.13
+s3transfer==0.3.0
 setuptools==40.3.0
 six==1.13.0
 transip==2.0.0

From c28b1a7d7389ff4363229c22d922d212c3881a21 Mon Sep 17 00:00:00 2001
From: "dependabot-preview[bot]"
 <27856297+dependabot-preview[bot]@users.noreply.github.com>
Date: Sat, 11 Jan 2020 17:17:37 +0000
Subject: [PATCH 35/56] Bump jmespath from 0.9.3 to 0.9.4

Bumps [jmespath](https://github.com/jmespath/jmespath.py) from 0.9.3 to 0.9.4.
- [Release notes](https://github.com/jmespath/jmespath.py/releases)
- [Changelog](https://github.com/jmespath/jmespath.py/blob/develop/CHANGELOG.rst)
- [Commits](https://github.com/jmespath/jmespath.py/compare/0.9.3...0.9.4)

Signed-off-by: dependabot-preview[bot] <support@dependabot.com>
---
 requirements.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/requirements.txt b/requirements.txt
index 8769223..5ba5ac2 100644
--- a/requirements.txt
+++ b/requirements.txt
@@ -11,7 +11,7 @@ futures==3.2.0; python_version < '3.0'
 google-cloud-core==1.1.0
 google-cloud-dns==0.29.0
 ipaddress==1.0.22
-jmespath==0.9.3
+jmespath==0.9.4
 msrestazure==0.6.2
 natsort==5.5.0
 ns1-python==0.13.0

From 04759a51a02de6ce59829c732b73d1e7055cb490 Mon Sep 17 00:00:00 2001
From: "dependabot-preview[bot]"
 <27856297+dependabot-preview[bot]@users.noreply.github.com>
Date: Sat, 11 Jan 2020 17:21:33 +0000
Subject: [PATCH 36/56] Bump ovh from 0.4.8 to 0.5.0

Bumps [ovh](https://github.com/ovh/python-ovh) from 0.4.8 to 0.5.0.
- [Release notes](https://github.com/ovh/python-ovh/releases)
- [Changelog](https://github.com/ovh/python-ovh/blob/master/CHANGELOG.md)
- [Commits](https://github.com/ovh/python-ovh/compare/v0.4.8...v0.5.0)

Signed-off-by: dependabot-preview[bot] <support@dependabot.com>
---
 requirements.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/requirements.txt b/requirements.txt
index 5ba5ac2..14a52f0 100644
--- a/requirements.txt
+++ b/requirements.txt
@@ -15,7 +15,7 @@ jmespath==0.9.4
 msrestazure==0.6.2
 natsort==5.5.0
 ns1-python==0.13.0
-ovh==0.4.8
+ovh==0.5.0
 pycountry-convert==0.7.2
 pycountry==19.8.18
 python-dateutil==2.8.1

From 09000540a012dc19bc499234dbd17d8054d00b9d Mon Sep 17 00:00:00 2001
From: "dependabot-preview[bot]"
 <27856297+dependabot-preview[bot]@users.noreply.github.com>
Date: Sat, 11 Jan 2020 17:26:30 +0000
Subject: [PATCH 37/56] Bump google-cloud-dns from 0.29.0 to 0.31.0

Bumps [google-cloud-dns](https://github.com/GoogleCloudPlatform/google-cloud-python) from 0.29.0 to 0.31.0.
- [Release notes](https://github.com/GoogleCloudPlatform/google-cloud-python/releases)
- [Changelog](https://github.com/googleapis/google-cloud-python/blob/master/CHANGELOG.md)
- [Commits](https://github.com/GoogleCloudPlatform/google-cloud-python/compare/0.29.0...0.31.0)

Signed-off-by: dependabot-preview[bot] <support@dependabot.com>
---
 requirements.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/requirements.txt b/requirements.txt
index 14a52f0..f063d26 100644
--- a/requirements.txt
+++ b/requirements.txt
@@ -9,7 +9,7 @@ dyn==1.8.1
 edgegrid-python==1.1.1
 futures==3.2.0; python_version < '3.0'
 google-cloud-core==1.1.0
-google-cloud-dns==0.29.0
+google-cloud-dns==0.31.0
 ipaddress==1.0.22
 jmespath==0.9.4
 msrestazure==0.6.2

From a33b75911d6a24952cafe586061f16805941771c Mon Sep 17 00:00:00 2001
From: "dependabot-preview[bot]"
 <27856297+dependabot-preview[bot]@users.noreply.github.com>
Date: Sat, 11 Jan 2020 17:30:12 +0000
Subject: [PATCH 38/56] Bump pycodestyle from 2.4.0 to 2.5.0

Bumps [pycodestyle](https://github.com/PyCQA/pycodestyle) from 2.4.0 to 2.5.0.
- [Release notes](https://github.com/PyCQA/pycodestyle/releases)
- [Changelog](https://github.com/PyCQA/pycodestyle/blob/master/CHANGES.txt)
- [Commits](https://github.com/PyCQA/pycodestyle/compare/2.4.0...2.5.0)

Signed-off-by: dependabot-preview[bot] <support@dependabot.com>
---
 requirements-dev.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/requirements-dev.txt b/requirements-dev.txt
index d9888b8..5fb2233 100644
--- a/requirements-dev.txt
+++ b/requirements-dev.txt
@@ -1,7 +1,7 @@
 coverage
 mock
 nose
-pycodestyle==2.4.0
+pycodestyle==2.5.0
 pyflakes==1.6.0
 readme_renderer[md]==24.0
 requests_mock

From 79cb88ef2566fc7863d3f71ad078bea081c1c04e Mon Sep 17 00:00:00 2001
From: Ross McFarland <ross@github.com>
Date: Sat, 11 Jan 2020 09:37:00 -0800
Subject: [PATCH 39/56] Fix pycodestyle overindent warnings w/2.5.0

---
 octodns/provider/constellix.py          | 4 ++--
 octodns/provider/dnsmadeeasy.py         | 2 +-
 tests/test_octodns_provider_azuredns.py | 2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/octodns/provider/constellix.py b/octodns/provider/constellix.py
index 2ca49e3..0600f80 100644
--- a/octodns/provider/constellix.py
+++ b/octodns/provider/constellix.py
@@ -429,8 +429,8 @@ class ConstellixProvider(BaseProvider):
         for record in self.zone_records(zone):
             if existing.name == record['name'] and \
                existing._type == record['type']:
-                    self._client.record_delete(zone.name, record['type'],
-                                               record['id'])
+                self._client.record_delete(zone.name, record['type'],
+                                           record['id'])
 
     def _apply(self, plan):
         desired = plan.desired
diff --git a/octodns/provider/dnsmadeeasy.py b/octodns/provider/dnsmadeeasy.py
index cc10c9a..0bf05a0 100644
--- a/octodns/provider/dnsmadeeasy.py
+++ b/octodns/provider/dnsmadeeasy.py
@@ -374,7 +374,7 @@ class DnsMadeEasyProvider(BaseProvider):
         for record in self.zone_records(zone):
             if existing.name == record['name'] and \
                existing._type == record['type']:
-                    self._client.record_delete(zone.name, record['id'])
+                self._client.record_delete(zone.name, record['id'])
 
     def _apply(self, plan):
         desired = plan.desired
diff --git a/tests/test_octodns_provider_azuredns.py b/tests/test_octodns_provider_azuredns.py
index 920c502..1769cef 100644
--- a/tests/test_octodns_provider_azuredns.py
+++ b/tests/test_octodns_provider_azuredns.py
@@ -321,7 +321,7 @@ class Test_ParseAzureType(TestCase):
                                ['AAAA', 'Microsoft.Network/dnszones/AAAA'],
                                ['NS', 'Microsoft.Network/dnszones/NS'],
                                ['MX', 'Microsoft.Network/dnszones/MX']]:
-                self.assertEquals(expected, _parse_azure_type(test))
+            self.assertEquals(expected, _parse_azure_type(test))
 
 
 class Test_CheckEndswithDot(TestCase):

From d56bf28d8d00f56a9aaa4521e4e63b7a24b7071c Mon Sep 17 00:00:00 2001
From: "dependabot-preview[bot]"
 <27856297+dependabot-preview[bot]@users.noreply.github.com>
Date: Sat, 11 Jan 2020 17:42:18 +0000
Subject: [PATCH 40/56] Bump botocore from 1.10.5 to 1.14.0

Bumps [botocore](https://github.com/boto/botocore) from 1.10.5 to 1.14.0.
- [Release notes](https://github.com/boto/botocore/releases)
- [Changelog](https://github.com/boto/botocore/blob/develop/CHANGELOG.rst)
- [Commits](https://github.com/boto/botocore/compare/1.10.5...1.14.0)

Signed-off-by: dependabot-preview[bot] <support@dependabot.com>
---
 requirements.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/requirements.txt b/requirements.txt
index f063d26..e627537 100644
--- a/requirements.txt
+++ b/requirements.txt
@@ -2,7 +2,7 @@ PyYaml==4.2b1
 azure-common==1.1.23
 azure-mgmt-dns==3.0.0
 boto3==1.7.5
-botocore==1.10.5
+botocore==1.14.0
 dnspython==1.15.0
 docutils==0.14
 dyn==1.8.1

From eb890f02148aba0593bb5b7e8d57be6f616cc3b9 Mon Sep 17 00:00:00 2001
From: Ross McFarland <ross@github.com>
Date: Sat, 11 Jan 2020 10:16:22 -0800
Subject: [PATCH 41/56] Bump boto3 to 1.11.0

---
 requirements.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/requirements.txt b/requirements.txt
index e627537..20db7f8 100644
--- a/requirements.txt
+++ b/requirements.txt
@@ -1,7 +1,7 @@
 PyYaml==4.2b1
 azure-common==1.1.23
 azure-mgmt-dns==3.0.0
-boto3==1.7.5
+boto3==1.11.0
 botocore==1.14.0
 dnspython==1.15.0
 docutils==0.14

From 60ec6e9a288921c8140c3c06d7706e33b322eb73 Mon Sep 17 00:00:00 2001
From: Ross McFarland <ross@github.com>
Date: Sat, 11 Jan 2020 10:16:31 -0800
Subject: [PATCH 42/56] Update Route53 test that pokes at boto internals

---
 tests/test_octodns_provider_route53.py | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/tests/test_octodns_provider_route53.py b/tests/test_octodns_provider_route53.py
index 7691804..60da3b8 100644
--- a/tests/test_octodns_provider_route53.py
+++ b/tests/test_octodns_provider_route53.py
@@ -1930,9 +1930,8 @@ class TestRoute53Provider(TestCase):
         provider = Route53Provider('test', 'abc', '123',
                                    client_max_attempts=42)
         # NOTE: this will break if boto ever changes the impl details...
-        self.assertEquals(43, provider._conn.meta.events
-                          ._unique_id_handlers['retry-config-route53']
-                          ['handler']._checker.__dict__['_max_attempts'])
+        self.assertEquals(42, provider._conn._client_config
+                          .retries['max_attempts'])
 
     def test_data_for_dynamic(self):
         provider = Route53Provider('test', 'abc', '123')

From 1573172c36f8316a85fe0322d40370af69d944b8 Mon Sep 17 00:00:00 2001
From: "dependabot-preview[bot]"
 <27856297+dependabot-preview[bot]@users.noreply.github.com>
Date: Sat, 11 Jan 2020 18:20:20 +0000
Subject: [PATCH 43/56] Bump azure-common from 1.1.23 to 1.1.24

Bumps [azure-common](https://github.com/Azure/azure-sdk-for-python) from 1.1.23 to 1.1.24.
- [Release notes](https://github.com/Azure/azure-sdk-for-python/releases)
- [Commits](https://github.com/Azure/azure-sdk-for-python/compare/azure-common_1.1.23...azure-common_1.1.24)

Signed-off-by: dependabot-preview[bot] <support@dependabot.com>
---
 requirements.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/requirements.txt b/requirements.txt
index 20db7f8..1dcec72 100644
--- a/requirements.txt
+++ b/requirements.txt
@@ -1,5 +1,5 @@
 PyYaml==4.2b1
-azure-common==1.1.23
+azure-common==1.1.24
 azure-mgmt-dns==3.0.0
 boto3==1.11.0
 botocore==1.14.0

From e19cc270607c45a54b1fef2d448fa881bd2c0224 Mon Sep 17 00:00:00 2001
From: "dependabot-preview[bot]"
 <27856297+dependabot-preview[bot]@users.noreply.github.com>
Date: Sat, 11 Jan 2020 18:28:37 +0000
Subject: [PATCH 44/56] Bump ipaddress from 1.0.22 to 1.0.23

Bumps [ipaddress](https://github.com/phihag/ipaddress) from 1.0.22 to 1.0.23.
- [Release notes](https://github.com/phihag/ipaddress/releases)
- [Commits](https://github.com/phihag/ipaddress/compare/v1.0.22...v1.0.23)

Signed-off-by: dependabot-preview[bot] <support@dependabot.com>
---
 requirements.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/requirements.txt b/requirements.txt
index 1dcec72..95577ce 100644
--- a/requirements.txt
+++ b/requirements.txt
@@ -10,7 +10,7 @@ edgegrid-python==1.1.1
 futures==3.2.0; python_version < '3.0'
 google-cloud-core==1.1.0
 google-cloud-dns==0.31.0
-ipaddress==1.0.22
+ipaddress==1.0.23
 jmespath==0.9.4
 msrestazure==0.6.2
 natsort==5.5.0

From 8598214edb2db0f1f93bc3445db652c550fd224b Mon Sep 17 00:00:00 2001
From: "dependabot-preview[bot]"
 <27856297+dependabot-preview[bot]@users.noreply.github.com>
Date: Sat, 11 Jan 2020 20:31:59 +0000
Subject: [PATCH 45/56] Bump docutils from 0.14 to 0.15.2

Bumps [docutils](http://docutils.sourceforge.net/) from 0.14 to 0.15.2.

Signed-off-by: dependabot-preview[bot] <support@dependabot.com>
---
 requirements.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/requirements.txt b/requirements.txt
index 95577ce..12e413d 100644
--- a/requirements.txt
+++ b/requirements.txt
@@ -4,7 +4,7 @@ azure-mgmt-dns==3.0.0
 boto3==1.11.0
 botocore==1.14.0
 dnspython==1.15.0
-docutils==0.14
+docutils==0.15.2
 dyn==1.8.1
 edgegrid-python==1.1.1
 futures==3.2.0; python_version < '3.0'

From 96dad58a48f2a600a1a5e6fa4e78ef5cd75bacc5 Mon Sep 17 00:00:00 2001
From: "dependabot-preview[bot]"
 <27856297+dependabot-preview[bot]@users.noreply.github.com>
Date: Sat, 11 Jan 2020 20:35:09 +0000
Subject: [PATCH 46/56] Bump pyyaml from 4.2b1 to 5.3

Bumps [pyyaml](https://github.com/yaml/pyyaml) from 4.2b1 to 5.3.
- [Release notes](https://github.com/yaml/pyyaml/releases)
- [Changelog](https://github.com/yaml/pyyaml/blob/master/CHANGES)
- [Commits](https://github.com/yaml/pyyaml/commits/5.3)

Signed-off-by: dependabot-preview[bot] <support@dependabot.com>
---
 requirements.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/requirements.txt b/requirements.txt
index 12e413d..281a06e 100644
--- a/requirements.txt
+++ b/requirements.txt
@@ -1,4 +1,4 @@
-PyYaml==4.2b1
+PyYaml==5.3
 azure-common==1.1.24
 azure-mgmt-dns==3.0.0
 boto3==1.11.0

From f15358535028a315c89427e0a5a11519f5a12636 Mon Sep 17 00:00:00 2001
From: "dependabot-preview[bot]"
 <27856297+dependabot-preview[bot]@users.noreply.github.com>
Date: Sat, 11 Jan 2020 20:35:09 +0000
Subject: [PATCH 47/56] Bump dnspython from 1.15.0 to 1.16.0

Bumps [dnspython](https://github.com/rthalley/dnspython) from 1.15.0 to 1.16.0.
- [Release notes](https://github.com/rthalley/dnspython/releases)
- [Changelog](https://github.com/rthalley/dnspython/blob/master/doc/whatsnew.rst)
- [Commits](https://github.com/rthalley/dnspython/compare/v1.15.0...v1.16.0)

Signed-off-by: dependabot-preview[bot] <support@dependabot.com>
---
 requirements.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/requirements.txt b/requirements.txt
index 12e413d..d926f6a 100644
--- a/requirements.txt
+++ b/requirements.txt
@@ -3,7 +3,7 @@ azure-common==1.1.24
 azure-mgmt-dns==3.0.0
 boto3==1.11.0
 botocore==1.14.0
-dnspython==1.15.0
+dnspython==1.16.0
 docutils==0.15.2
 dyn==1.8.1
 edgegrid-python==1.1.1

From 0e7b986c157339d91a4621af5266c48cce983689 Mon Sep 17 00:00:00 2001
From: "dependabot-preview[bot]"
 <27856297+dependabot-preview[bot]@users.noreply.github.com>
Date: Sat, 11 Jan 2020 20:38:21 +0000
Subject: [PATCH 48/56] Bump twine from 1.13.0 to 1.15.0

Bumps [twine](https://github.com/pypa/twine) from 1.13.0 to 1.15.0.
- [Release notes](https://github.com/pypa/twine/releases)
- [Changelog](https://github.com/pypa/twine/blob/master/docs/changelog.rst)
- [Commits](https://github.com/pypa/twine/compare/1.13.0...1.15.0)

Signed-off-by: dependabot-preview[bot] <support@dependabot.com>
---
 requirements-dev.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/requirements-dev.txt b/requirements-dev.txt
index 5fb2233..0de9ceb 100644
--- a/requirements-dev.txt
+++ b/requirements-dev.txt
@@ -5,4 +5,4 @@ pycodestyle==2.5.0
 pyflakes==1.6.0
 readme_renderer[md]==24.0
 requests_mock
-twine==1.13.0
+twine==1.15.0

From 743516575fd3c9162dac4ad782605abc8a114acb Mon Sep 17 00:00:00 2001
From: "dependabot-preview[bot]"
 <27856297+dependabot-preview[bot]@users.noreply.github.com>
Date: Sat, 11 Jan 2020 20:43:20 +0000
Subject: [PATCH 49/56] Bump dnspython from 1.15.0 to 1.16.0

Bumps [dnspython](https://github.com/rthalley/dnspython) from 1.15.0 to 1.16.0.
- [Release notes](https://github.com/rthalley/dnspython/releases)
- [Changelog](https://github.com/rthalley/dnspython/blob/master/doc/whatsnew.rst)
- [Commits](https://github.com/rthalley/dnspython/compare/v1.15.0...v1.16.0)

Signed-off-by: dependabot-preview[bot] <support@dependabot.com>
---
 requirements.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/requirements.txt b/requirements.txt
index 281a06e..fb46bed 100644
--- a/requirements.txt
+++ b/requirements.txt
@@ -3,7 +3,7 @@ azure-common==1.1.24
 azure-mgmt-dns==3.0.0
 boto3==1.11.0
 botocore==1.14.0
-dnspython==1.15.0
+dnspython==1.16.0
 docutils==0.15.2
 dyn==1.8.1
 edgegrid-python==1.1.1

From f18c702c59dcf1bd925790e9cd00722b9b6eef64 Mon Sep 17 00:00:00 2001
From: "dependabot-preview[bot]"
 <27856297+dependabot-preview[bot]@users.noreply.github.com>
Date: Sat, 11 Jan 2020 20:43:23 +0000
Subject: [PATCH 50/56] Bump natsort from 5.5.0 to 6.2.0

Bumps [natsort](https://github.com/SethMMorton/natsort) from 5.5.0 to 6.2.0.
- [Release notes](https://github.com/SethMMorton/natsort/releases)
- [Changelog](https://github.com/SethMMorton/natsort/blob/master/CHANGELOG.md)
- [Commits](https://github.com/SethMMorton/natsort/compare/5.5.0...6.2.0)

Signed-off-by: dependabot-preview[bot] <support@dependabot.com>
---
 requirements.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/requirements.txt b/requirements.txt
index 281a06e..d44fef8 100644
--- a/requirements.txt
+++ b/requirements.txt
@@ -13,7 +13,7 @@ google-cloud-dns==0.31.0
 ipaddress==1.0.23
 jmespath==0.9.4
 msrestazure==0.6.2
-natsort==5.5.0
+natsort==6.2.0
 ns1-python==0.13.0
 ovh==0.5.0
 pycountry-convert==0.7.2

From e56051ba55026ba1e372af1970fce1e6c2c3c785 Mon Sep 17 00:00:00 2001
From: Ross McFarland <ross@github.com>
Date: Sat, 11 Jan 2020 13:45:48 -0800
Subject: [PATCH 51/56] Add TTL's to the axfr test files

---
 tests/zones/invalid.zone. | 2 +-
 tests/zones/unit.tests.   | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/zones/invalid.zone. b/tests/zones/invalid.zone.
index c814af6..04748a1 100644
--- a/tests/zones/invalid.zone.
+++ b/tests/zones/invalid.zone.
@@ -1,5 +1,5 @@
 $ORIGIN invalid.zone.
-@           IN	SOA	ns1.invalid.zone. root.invalid.zone. (
+@           3600 IN	SOA	ns1.invalid.zone. root.invalid.zone. (
                         2018071501		; Serial
                         3600    ; Refresh (1 hour)
                         600     ; Retry (10 minutes)
diff --git a/tests/zones/unit.tests. b/tests/zones/unit.tests.
index 95828ad..0305e05 100644
--- a/tests/zones/unit.tests.
+++ b/tests/zones/unit.tests.
@@ -1,5 +1,5 @@
 $ORIGIN unit.tests.
-@           IN	SOA	ns1.unit.tests. root.unit.tests. (
+@           3600 IN	SOA	ns1.unit.tests. root.unit.tests. (
                         2018071501		; Serial
                         3600    ; Refresh (1 hour)
                         600     ; Retry (10 minutes)

From a564da68bb04b50730ce87bebbfd2b15990bc484 Mon Sep 17 00:00:00 2001
From: "dependabot-preview[bot]"
 <27856297+dependabot-preview[bot]@users.noreply.github.com>
Date: Sat, 11 Jan 2020 21:51:52 +0000
Subject: [PATCH 52/56] Bump pyflakes from 1.6.0 to 2.1.1

Bumps [pyflakes](https://github.com/PyCQA/pyflakes) from 1.6.0 to 2.1.1.
- [Release notes](https://github.com/PyCQA/pyflakes/releases)
- [Changelog](https://github.com/PyCQA/pyflakes/blob/master/NEWS.rst)
- [Commits](https://github.com/PyCQA/pyflakes/compare/1.6.0...2.1.1)

Signed-off-by: dependabot-preview[bot] <support@dependabot.com>
---
 requirements-dev.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/requirements-dev.txt b/requirements-dev.txt
index 0de9ceb..3ad1b04 100644
--- a/requirements-dev.txt
+++ b/requirements-dev.txt
@@ -2,7 +2,7 @@ coverage
 mock
 nose
 pycodestyle==2.5.0
-pyflakes==1.6.0
+pyflakes==2.1.1
 readme_renderer[md]==24.0
 requests_mock
 twine==1.15.0

From 2ea63959da5ef7f8b6cad0aa5d14de1a11b2943e Mon Sep 17 00:00:00 2001
From: "dependabot-preview[bot]"
 <27856297+dependabot-preview[bot]@users.noreply.github.com>
Date: Sat, 11 Jan 2020 21:55:24 +0000
Subject: [PATCH 53/56] Bump setuptools from 40.3.0 to 44.0.0

Bumps [setuptools](https://github.com/pypa/setuptools) from 40.3.0 to 44.0.0.
- [Release notes](https://github.com/pypa/setuptools/releases)
- [Changelog](https://github.com/pypa/setuptools/blob/master/CHANGES.rst)
- [Commits](https://github.com/pypa/setuptools/compare/v40.3.0...v44.0.0)

Signed-off-by: dependabot-preview[bot] <support@dependabot.com>
---
 requirements.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/requirements.txt b/requirements.txt
index d44fef8..9794364 100644
--- a/requirements.txt
+++ b/requirements.txt
@@ -21,6 +21,6 @@ pycountry==19.8.18
 python-dateutil==2.8.1
 requests==2.22.0
 s3transfer==0.3.0
-setuptools==40.3.0
+setuptools==44.0.0
 six==1.13.0
 transip==2.0.0

From 2d093c479692c8eeca7c425e76ad785385a77ef9 Mon Sep 17 00:00:00 2001
From: "dependabot-preview[bot]"
 <27856297+dependabot-preview[bot]@users.noreply.github.com>
Date: Sun, 12 Jan 2020 19:50:47 +0000
Subject: [PATCH 54/56] Bump docutils from 0.15.2 to 0.16

Bumps [docutils](http://docutils.sourceforge.net/) from 0.15.2 to 0.16.

Signed-off-by: dependabot-preview[bot] <support@dependabot.com>
---
 requirements.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/requirements.txt b/requirements.txt
index 59b8864..93d8567 100644
--- a/requirements.txt
+++ b/requirements.txt
@@ -4,7 +4,7 @@ azure-mgmt-dns==3.0.0
 boto3==1.11.0
 botocore==1.14.0
 dnspython==1.16.0
-docutils==0.15.2
+docutils==0.16
 dyn==1.8.1
 edgegrid-python==1.1.1
 futures==3.2.0; python_version < '3.0'

From 01a9fa87b190128c40f6ff6a96a6eb015be4119d Mon Sep 17 00:00:00 2001
From: Ross McFarland <ross@github.com>
Date: Mon, 13 Jan 2020 07:29:38 -0800
Subject: [PATCH 55/56] Address Ns1Provider review feedback

---
 octodns/provider/ns1.py | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/octodns/provider/ns1.py b/octodns/provider/ns1.py
index b442271..0694627 100644
--- a/octodns/provider/ns1.py
+++ b/octodns/provider/ns1.py
@@ -655,9 +655,9 @@ class Ns1Provider(BaseProvider):
         host = record.fqdn[:-1]
         _type = record._type
 
-        request = 'GET {path} HTTP/1.0\\r\\nHost: {host}\\r\\n' \
-            'User-agent: NS1\\r\\n\\r\\n'.format(path=record.healthcheck_path,
-                                                 host=record.healthcheck_host)
+        request = r'GET {path} HTTP/1.0\r\nHost: {host}\r\n' \
+            r'User-agent: NS1\r\n\r\n'.format(path=record.healthcheck_path,
+                                              host=record.healthcheck_host)
 
         return {
             'active': True,
@@ -771,13 +771,13 @@ class Ns1Provider(BaseProvider):
             for geo in rule.data.get('geos', []):
                 n = len(geo)
                 if n == 8:
-                    # US state
+                    # US state, e.g. NA-US-KY
                     us_state.add(geo[-2:])
                 elif n == 5:
-                    # Country
+                    # Country, e.g. EU-FR
                     country.add(geo[-2:])
                 else:
-                    # Continent
+                    # Continent, e.g. AS
                     georegion.update(self._CONTINENT_TO_REGIONS[geo])
 
             meta = {
@@ -826,7 +826,9 @@ class Ns1Provider(BaseProvider):
 
             # Dynamic/health checked
             current_pool_name = pool_name
-            while current_pool_name:
+            seen = set()
+            while current_pool_name and current_pool_name not in seen:
+                seen.add(current_pool_name)
                 pool = pools[current_pool_name]
                 for answer in pool_answers[current_pool_name]:
                     answer = {

From c950503868ded420ee3d2a2e5df5d2e1d64fba8b Mon Sep 17 00:00:00 2001
From: Ross McFarland <ross@github.com>
Date: Mon, 13 Jan 2020 07:29:56 -0800
Subject: [PATCH 56/56] Pass through the CHANGELOG

---
 CHANGELOG.md | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 1cb7b1b..491370f 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,3 +1,11 @@
+## v0.9.10 - ????-??-?? - ???
+
+* Added support for dynamic records to Ns1Provider, updated client and rate
+  limiting implementation
+* Moved CI to use GitHub Actions
+* Set up dependabot to automatically PR requirements updates
+* Pass at bumping all of the requirements
+
 ## v0.9.9 - 2019-11-04 - Python 3.7 Support
 
 * Extensive pass through the whole codebase to support Python 3