From 6ab6124d729a20fcf9844f16f96be35cbc2c96ff Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Fri, 25 Mar 2022 18:16:50 -0700 Subject: [PATCH 1/4] Log the octoDNS version as part of Manager.__init__ info line --- CHANGELOG.md | 4 ++++ octodns/manager.py | 4 +++- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ed95a32..82a91ef 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,10 @@ to do so, but weren't. There was an ordering before, but it was essentially arbitrarily picked. +#### Stuff + +* Manager includes the octoDNS version in its init log line + ## v0.9.16 - 2022-03-04 - Manage the root of the problem #### Noteworthy changes diff --git a/octodns/manager.py b/octodns/manager.py index c8a4263..d69fd60 100644 --- a/octodns/manager.py +++ b/octodns/manager.py @@ -11,6 +11,7 @@ from os import environ from sys import stdout import logging +from . import __VERSION__ from .provider.base import BaseProvider from .provider.plan import Plan from .provider.yaml import SplitYamlProvider, YamlProvider @@ -84,7 +85,8 @@ class Manager(object): return len(plan.changes[0].record.zone.name) if plan.changes else 0 def __init__(self, config_file, max_workers=None, include_meta=False): - self.log.info('__init__: config_file=%s', config_file) + self.log.info('__init__: config_file=%s (octoDNS %s)', config_file, + __VERSION__) # Read our config file with open(config_file, 'r') as fh: From 633aef5845a7c48f1bc1e85b95af7464dfe89966 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Fri, 25 Mar 2022 20:03:42 -0700 Subject: [PATCH 2/4] Manager prints provider, processor, and plan_output versions for non-core modules when available --- octodns/manager.py | 20 ++++++++++++++++---- tests/config/processors.yaml | 6 +++++- tests/helpers.py | 9 +++++++++ tests/test_octodns_manager.py | 4 ++-- 4 files changed, 32 insertions(+), 7 deletions(-) diff --git a/octodns/manager.py b/octodns/manager.py index d69fd60..8365bc8 100644 --- a/octodns/manager.py +++ b/octodns/manager.py @@ -115,10 +115,13 @@ class Manager(object): self.log.exception('Invalid provider class') raise ManagerException(f'Provider {provider_name} is missing ' 'class') - _class = self._get_named_class('provider', _class) + _class, module, version = self._get_named_class('provider', _class) kwargs = self._build_kwargs(provider_config) try: self.providers[provider_name] = _class(provider_name, **kwargs) + if not module.startswith('octodns.'): + self.log.info('__init__: provider=%s (%s %s)', + provider_name, module, version) except TypeError: self.log.exception('Invalid provider config') raise ManagerException('Incorrect provider config for ' + @@ -133,11 +136,15 @@ class Manager(object): self.log.exception('Invalid processor class') raise ManagerException(f'Processor {processor_name} is ' 'missing class') - _class = self._get_named_class('processor', _class) + _class, module, version = self._get_named_class('processor', + _class) kwargs = self._build_kwargs(processor_config) try: self.processors[processor_name] = _class(processor_name, **kwargs) + if not module.startswith('octodns.'): + self.log.info('__init__: processor=%s (%s %s)', + processor_name, module, version) except TypeError: self.log.exception('Invalid processor config') raise ManagerException('Incorrect processor config for ' + @@ -177,11 +184,15 @@ class Manager(object): self.log.exception('Invalid plan_output class') raise ManagerException(f'plan_output {plan_output_name} is ' 'missing class') - _class = self._get_named_class('plan_output', _class) + _class, module, version = self._get_named_class('plan_output', + _class) kwargs = self._build_kwargs(plan_output_config) try: self.plan_outputs[plan_output_name] = \ _class(plan_output_name, **kwargs) + if not module.startswith('octodns.'): + self.log.info('__init__: plan_output=%s (%s %s)', + plan_output_name, module, version) except TypeError: self.log.exception('Invalid plan_output config') raise ManagerException('Incorrect plan_output config for ' + @@ -195,8 +206,9 @@ class Manager(object): self.log.exception('_get_{}_class: Unable to import ' 'module %s', _class) raise ManagerException(f'Unknown {_type} class: {_class}') + version = getattr(module, '__VERSION__', 'n/a') try: - return getattr(module, class_name) + return getattr(module, class_name), module_name, version except AttributeError: self.log.exception('_get_{}_class: Unable to get class %s ' 'from module %s', class_name, module) diff --git a/tests/config/processors.yaml b/tests/config/processors.yaml index 4229707..ec50fb3 100644 --- a/tests/config/processors.yaml +++ b/tests/config/processors.yaml @@ -1,6 +1,7 @@ providers: config: - class: octodns.provider.yaml.YamlProvider + # This helps us get coverage when printing out provider versions + class: helpers.TestYamlProvider directory: tests/config dump: class: octodns.provider.yaml.YamlProvider @@ -15,6 +16,9 @@ processors: # Just testing config so any processor will do noop: class: octodns.processor.base.BaseProcessor + test: + # This helps us get coverage when printing out processor versions + class: helpers.TestBaseProcessor zones: unit.tests.: diff --git a/tests/helpers.py b/tests/helpers.py index 0d0f54d..4f0ddb2 100644 --- a/tests/helpers.py +++ b/tests/helpers.py @@ -11,6 +11,7 @@ from logging import getLogger from octodns.processor.base import BaseProcessor from octodns.provider.base import BaseProvider +from octodns.provider.yaml import YamlProvider class SimpleSource(object): @@ -122,3 +123,11 @@ class PlannableProvider(BaseProvider): def __repr__(self): return self.__class__.__name__ + + +class TestYamlProvider(YamlProvider): + pass + + +class TestBaseProcessor(BaseProcessor): + pass diff --git a/tests/test_octodns_manager.py b/tests/test_octodns_manager.py index 9e68030..51c47eb 100644 --- a/tests/test_octodns_manager.py +++ b/tests/test_octodns_manager.py @@ -425,7 +425,7 @@ class TestManager(TestCase): 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 + mock.return_value = (plan_output_class_mock, 'ignored', 'ignored') fh_mock = MagicMock() Manager(get_config_filename('plan-output-filehandle.yaml') @@ -441,7 +441,7 @@ class TestManager(TestCase): def test_processor_config(self): # Smoke test loading a valid config manager = Manager(get_config_filename('processors.yaml')) - self.assertEqual(['noop'], list(manager.processors.keys())) + self.assertEqual(['noop', 'test'], list(manager.processors.keys())) # This zone specifies a valid processor manager.sync(['unit.tests.']) From b458fe0dc8533d6c1f214883638c8e7126cb2ded Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Fri, 25 Mar 2022 20:43:48 -0700 Subject: [PATCH 3/4] Walk module hierarchy looking for __VERSION__ --- octodns/manager.py | 33 +++++++++++++++++++++++---------- 1 file changed, 23 insertions(+), 10 deletions(-) diff --git a/octodns/manager.py b/octodns/manager.py index 8365bc8..f3da6a3 100644 --- a/octodns/manager.py +++ b/octodns/manager.py @@ -119,9 +119,8 @@ class Manager(object): kwargs = self._build_kwargs(provider_config) try: self.providers[provider_name] = _class(provider_name, **kwargs) - if not module.startswith('octodns.'): - self.log.info('__init__: provider=%s (%s %s)', - provider_name, module, version) + self.log.info('__init__: provider=%s (%s %s)', provider_name, + module, version) except TypeError: self.log.exception('Invalid provider config') raise ManagerException('Incorrect provider config for ' + @@ -142,9 +141,8 @@ class Manager(object): try: self.processors[processor_name] = _class(processor_name, **kwargs) - if not module.startswith('octodns.'): - self.log.info('__init__: processor=%s (%s %s)', - processor_name, module, version) + self.log.info('__init__: processor=%s (%s %s)', processor_name, + module, version) except TypeError: self.log.exception('Invalid processor config') raise ManagerException('Incorrect processor config for ' + @@ -172,7 +170,7 @@ class Manager(object): self.plan_outputs = {} plan_outputs = manager_config.get('plan_outputs', { - 'logger': { + '_logger': { 'class': 'octodns.provider.plan.PlanLogger', 'level': 'info' } @@ -190,7 +188,8 @@ class Manager(object): try: self.plan_outputs[plan_output_name] = \ _class(plan_output_name, **kwargs) - if not module.startswith('octodns.'): + # Don't print out version info for the default output + if plan_output_name != '_logger': self.log.info('__init__: plan_output=%s (%s %s)', plan_output_name, module, version) except TypeError: @@ -198,15 +197,29 @@ class Manager(object): raise ManagerException('Incorrect plan_output config for ' + plan_output_name) + def _import_module(self, module_name): + current = module_name + _next = current.rsplit('.', 1)[0] + module = import_module(current) + version = getattr(module, '__VERSION__', None) + # If we didn't find a version in the specific module we're importing, + # we'll try walking up the hierarchy, as long as there is one (`.`), + # looking for it. + while version is None and current != _next: + current = _next + _next = current.rsplit('.', 1)[0] + version = getattr(import_module(current), '__VERSION__', None) + return module, version or 'n/a' + def _get_named_class(self, _type, _class): try: module_name, class_name = _class.rsplit('.', 1) - module = import_module(module_name) + module, version = self._import_module(module_name) except (ImportError, ValueError): self.log.exception('_get_{}_class: Unable to import ' 'module %s', _class) raise ManagerException(f'Unknown {_type} class: {_class}') - version = getattr(module, '__VERSION__', 'n/a') + try: return getattr(module, class_name), module_name, version except AttributeError: From 66958f7c21c5124ce2eac056a3e52199213278d9 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Mon, 28 Mar 2022 13:33:32 -0700 Subject: [PATCH 4/4] Manager._try_version with 3.7 noop (for now) --- octodns/manager.py | 33 ++++++++++++++++++++++++++++++--- tests/test_octodns_manager.py | 24 ++++++++++++++++++++++++ 2 files changed, 54 insertions(+), 3 deletions(-) diff --git a/octodns/manager.py b/octodns/manager.py index f3da6a3..e640a8d 100644 --- a/octodns/manager.py +++ b/octodns/manager.py @@ -20,6 +20,18 @@ from .yaml import safe_load from .zone import Zone +# TODO: this can go away once we no longer support python 3.7 +try: + from importlib.metadata import PackageNotFoundError, \ + version as module_version +except ModuleNotFoundError: # pragma: no cover + class PackageNotFoundError(Exception): + pass + + def module_version(*args, **kargs): + raise PackageNotFoundError('placeholder') + + class _AggregateTarget(object): id = 'aggregate' @@ -85,8 +97,9 @@ class Manager(object): return len(plan.changes[0].record.zone.name) if plan.changes else 0 def __init__(self, config_file, max_workers=None, include_meta=False): + version = self._try_version('octodns', version=__VERSION__) self.log.info('__init__: config_file=%s (octoDNS %s)', config_file, - __VERSION__) + version) # Read our config file with open(config_file, 'r') as fh: @@ -197,18 +210,32 @@ class Manager(object): raise ManagerException('Incorrect plan_output config for ' + plan_output_name) + def _try_version(self, module_name, module=None, version=None): + try: + # Always try and use the official lookup first + return module_version(module_name) + except PackageNotFoundError: + pass + # If we were passed a version that's next in line + if version is not None: + return version + # finally try and import the module and see if it has a __VERSION__ + if module is None: + module = import_module(module_name) + return getattr(module, '__VERSION__', None) + def _import_module(self, module_name): current = module_name _next = current.rsplit('.', 1)[0] module = import_module(current) - version = getattr(module, '__VERSION__', None) + version = self._try_version(current, module=module) # If we didn't find a version in the specific module we're importing, # we'll try walking up the hierarchy, as long as there is one (`.`), # looking for it. while version is None and current != _next: current = _next _next = current.rsplit('.', 1)[0] - version = getattr(import_module(current), '__VERSION__', None) + version = self._try_version(current) return module, version or 'n/a' def _get_named_class(self, _type, _class): diff --git a/tests/test_octodns_manager.py b/tests/test_octodns_manager.py index 51c47eb..6d85924 100644 --- a/tests/test_octodns_manager.py +++ b/tests/test_octodns_manager.py @@ -8,6 +8,7 @@ from __future__ import absolute_import, division, print_function, \ from os import environ from os.path import dirname, join +from octodns import __VERSION__ from octodns.manager import _AggregateTarget, MainThreadExecutor, Manager, \ ManagerException from octodns.processor.base import BaseProcessor @@ -524,6 +525,29 @@ class TestManager(TestCase): # no plans self.assertFalse(plans) + def test_try_version(self): + manager = Manager(get_config_filename('simple.yaml')) + + class DummyModule(object): + __VERSION__ = '2.3.4' + + dummy_module = DummyModule() + + # use importlib.metadata.version + self.assertTrue(__VERSION__, + manager._try_version('octodns', + module=dummy_module, + version='1.2.3')) + + # use module + self.assertTrue(manager._try_version('doesnt-exist', + module=dummy_module)) + + # fall back to version, preferred over module + self.assertEqual('1.2.3', manager._try_version('doesnt-exist', + module=dummy_module, + version='1.2.3', )) + class TestMainThreadExecutor(TestCase):