From 090dbe351554ca3e929f099fa9e127aceedccd79 Mon Sep 17 00:00:00 2001 From: Christian Funkhouser Date: Wed, 7 Apr 2021 16:00:05 -0400 Subject: [PATCH 1/5] sync accepts file handle for plan output --- octodns/manager.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/octodns/manager.py b/octodns/manager.py index 9ce10ff..f4f1491 100644 --- a/octodns/manager.py +++ b/octodns/manager.py @@ -9,6 +9,7 @@ from concurrent.futures import ThreadPoolExecutor from importlib import import_module from os import environ from six import text_type +from sys import stdout import logging from .provider.base import BaseProvider @@ -267,7 +268,7 @@ class Manager(object): return plans, zone def sync(self, eligible_zones=[], eligible_sources=[], eligible_targets=[], - dry_run=True, force=False): + dry_run=True, force=False, plan_output_fh=stdout): self.log.info('sync: eligible_zones=%s, eligible_targets=%s, ' 'dry_run=%s, force=%s', eligible_zones, eligible_targets, dry_run, force) @@ -276,7 +277,7 @@ class Manager(object): if eligible_zones: zones = [z for z in zones if z[0] in eligible_zones] - aliased_zones = {} + aliased_zones = {} futures = [] for zone_name, config in zones: self.log.info('sync: zone=%s', zone_name) @@ -402,7 +403,7 @@ class Manager(object): plans.sort(key=self._plan_keyer, reverse=True) for output in self.plan_outputs.values(): - output.run(plans=plans, log=self.log) + output.run(plans=plans, log=self.log, fh=plan_output_fh) if not force: self.log.debug('sync: checking safety') From 2075550f078942d642c873bd2ec62343eb1a0f16 Mon Sep 17 00:00:00 2001 From: Christian Funkhouser Date: Wed, 7 Apr 2021 18:21:34 -0400 Subject: [PATCH 2/5] Test that Manager passes fh to _PlanOutputs --- octodns/manager.py | 5 +++-- tests/config/plan-output-filehandle.yaml | 6 ++++++ tests/test_octodns_manager.py | 23 ++++++++++++++++++++++- 3 files changed, 31 insertions(+), 3 deletions(-) create mode 100644 tests/config/plan-output-filehandle.yaml diff --git a/octodns/manager.py b/octodns/manager.py index f4f1491..e94e41e 100644 --- a/octodns/manager.py +++ b/octodns/manager.py @@ -270,8 +270,9 @@ class Manager(object): def sync(self, eligible_zones=[], eligible_sources=[], eligible_targets=[], dry_run=True, force=False, plan_output_fh=stdout): self.log.info('sync: eligible_zones=%s, eligible_targets=%s, ' - 'dry_run=%s, force=%s', eligible_zones, eligible_targets, - dry_run, force) + 'dry_run=%s, force=%s, plan_output_fh=%s', + eligible_zones, eligible_targets, dry_run, force, + plan_output_fh) zones = self.config['zones'].items() if eligible_zones: diff --git a/tests/config/plan-output-filehandle.yaml b/tests/config/plan-output-filehandle.yaml new file mode 100644 index 0000000..9c9bb87 --- /dev/null +++ b/tests/config/plan-output-filehandle.yaml @@ -0,0 +1,6 @@ +manager: + plan_outputs: + "doesntexist": + class: octodns.provider.plan.DoesntExist +providers: {} +zones: {} diff --git a/tests/test_octodns_manager.py b/tests/test_octodns_manager.py index 442ed49..003d414 100644 --- a/tests/test_octodns_manager.py +++ b/tests/test_octodns_manager.py @@ -8,7 +8,6 @@ from __future__ import absolute_import, division, print_function, \ from os import environ from os.path import dirname, join from six import text_type -from unittest import TestCase from octodns.record import Record from octodns.manager import _AggregateTarget, MainThreadExecutor, Manager, \ @@ -16,6 +15,10 @@ from octodns.manager import _AggregateTarget, MainThreadExecutor, Manager, \ from octodns.yaml import safe_load from octodns.zone import Zone +from mock import MagicMock, patch +from unittest import TestCase + + from helpers import DynamicProvider, GeoProvider, NoSshFpProvider, \ SimpleProvider, TemporaryDirectory @@ -371,6 +374,24 @@ class TestManager(TestCase): with self.assertRaises(TypeError): manager._populate_and_plan('unit.tests.', [NoZone()], []) + @patch('octodns.manager.Manager._get_named_class') + def test_sync_passes_file_handle(self, mock): + plan_output_mock = MagicMock() + plan_output_class_mock = MagicMock() + plan_output_class_mock.return_value = plan_output_mock + mock.return_value = plan_output_class_mock + fh_mock = MagicMock() + + Manager(get_config_filename('plan-output-filehandle.yaml') + ).sync(plan_output_fh=fh_mock) + + # Since we only care about the fh kwarg, and different _PlanOutputs are + # are free to require arbitrary kwargs anyway, we concern ourselves + # with checking the value of fh only. + plan_output_mock.run.assert_called() + _, kwargs = plan_output_mock.run.call_args + self.assertEqual(fh_mock, kwargs.get('fh')) + class TestMainThreadExecutor(TestCase): From 55c194c2032fd65b1d61b0309d3d9c55f8e22489 Mon Sep 17 00:00:00 2001 From: Christian Funkhouser Date: Thu, 8 Apr 2021 09:40:04 -0400 Subject: [PATCH 3/5] Update tests/test_octodns_manager.py Co-authored-by: Ross McFarland --- tests/test_octodns_manager.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/test_octodns_manager.py b/tests/test_octodns_manager.py index 003d414..91ee374 100644 --- a/tests/test_octodns_manager.py +++ b/tests/test_octodns_manager.py @@ -18,7 +18,6 @@ from octodns.zone import Zone from mock import MagicMock, patch from unittest import TestCase - from helpers import DynamicProvider, GeoProvider, NoSshFpProvider, \ SimpleProvider, TemporaryDirectory From aa93e20f2e4f7fa597e61f11a5096a34fea32f20 Mon Sep 17 00:00:00 2001 From: Christian Funkhouser Date: Thu, 8 Apr 2021 11:03:30 -0400 Subject: [PATCH 4/5] Represent plan_output_fh less verbosely. Co-authored-by: Ross McFarland --- octodns/manager.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/octodns/manager.py b/octodns/manager.py index e94e41e..7326207 100644 --- a/octodns/manager.py +++ b/octodns/manager.py @@ -272,7 +272,7 @@ class Manager(object): self.log.info('sync: eligible_zones=%s, eligible_targets=%s, ' 'dry_run=%s, force=%s, plan_output_fh=%s', eligible_zones, eligible_targets, dry_run, force, - plan_output_fh) + getattr(plan_output_fh, 'name', plan_output_fh.__class__.__name__)) zones = self.config['zones'].items() if eligible_zones: From ada61f8d764cf4175f83376d989eaec3984e716a Mon Sep 17 00:00:00 2001 From: Christian Funkhouser Date: Thu, 8 Apr 2021 11:56:17 -0400 Subject: [PATCH 5/5] De-lint an aggressively-long log line --- octodns/manager.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/octodns/manager.py b/octodns/manager.py index 7326207..9b10196 100644 --- a/octodns/manager.py +++ b/octodns/manager.py @@ -269,10 +269,12 @@ class Manager(object): def sync(self, eligible_zones=[], eligible_sources=[], eligible_targets=[], dry_run=True, force=False, plan_output_fh=stdout): - self.log.info('sync: eligible_zones=%s, eligible_targets=%s, ' - 'dry_run=%s, force=%s, plan_output_fh=%s', - eligible_zones, eligible_targets, dry_run, force, - getattr(plan_output_fh, 'name', plan_output_fh.__class__.__name__)) + + self.log.info( + 'sync: eligible_zones=%s, eligible_targets=%s, dry_run=%s, ' + 'force=%s, plan_output_fh=%s', + eligible_zones, eligible_targets, dry_run, force, + getattr(plan_output_fh, 'name', plan_output_fh.__class__.__name__)) zones = self.config['zones'].items() if eligible_zones: