From 38a86adcdb78c1944c26a5ab8deddff619b33bcf Mon Sep 17 00:00:00 2001 From: J08nY Date: Wed, 31 May 2017 02:09:09 +0200 Subject: Add pluggable components. - Adds the notion of a 'plugin'. - A plugin has a package path and a flag specifying whether it's enabled or not. - Adds a find_pluggable_components function similar to the find_components one. This one dynamically searches not only the mailman package but all of plugins. - e.g. find_pluggable_components('rules', IRule) finds all IRule components in mailman.rules but also in example_plugin.rules for plugin names example_plugin. - Uses the find_pluggable_components function in place of find_components when searching for Rules, Handlers, Chains, EmailCommands, and Styles. --- src/mailman/utilities/plugins.py | 76 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 76 insertions(+) create mode 100644 src/mailman/utilities/plugins.py (limited to 'src/mailman/utilities/plugins.py') diff --git a/src/mailman/utilities/plugins.py b/src/mailman/utilities/plugins.py new file mode 100644 index 000000000..a8449af31 --- /dev/null +++ b/src/mailman/utilities/plugins.py @@ -0,0 +1,76 @@ +# Copyright (C) 2009-2017 by the Free Software Foundation, Inc. +# +# This file is part of GNU Mailman. +# +# GNU Mailman is free software: you can redistribute it and/or modify it under +# the terms of the GNU General Public License as published by the Free +# Software Foundation, either version 3 of the License, or (at your option) +# any later version. +# +# GNU Mailman is distributed in the hope that it will be useful, but WITHOUT +# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or +# FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for +# more details. +# +# You should have received a copy of the GNU General Public License along with +# GNU Mailman. If not, see . + +"""Plugin utilities.""" + +from lazr.config import as_boolean +from mailman.config import config +from mailman.utilities.modules import find_components +from pkg_resources import resource_isdir +from public import public + + +@public +def find_pluggable_components(subpackage, interface): + """Find components which conform to a given interface, in subpackage. + + :param subpackage: Mailman subpackage to search. The search is also + done inside the corresponding plugins subpackages. + :type subpackage: string + :param interface: The interface that returned objects must conform to. + :type interface: `Interface` + :return: The sequence of matching components. + :rtype: objects implementing `interface` + """ + yield from find_components('mailman.' + subpackage, interface) + package_roots = [plugin.path + for plugin in config.plugin_configs + if as_boolean(plugin.enable) and plugin.path] + for package_root in package_roots: + package = package_root + '.' + subpackage + if resource_isdir(package_root, subpackage): + yield from find_components(package, interface) + + +@public +def add_pluggable_components(subpackage, interface, mapping): + """Add components to a given mapping. + + Similarly to `find_pluggable_components()` this inspects all modules in a + given mailman and plugin's subpackage looking for objects that conform to + a given interface. All such found objects (unless decorated with + `@abstract_component`) are added to the given mapping, keyed by the + object's `.name` attribute, which is required. It is a fatal error if + that key already exists in the mapping. + + :param package: The subpackage path to search. + :type package: string + :param interface: The interface that returned objects must conform to. + Objects found must have a `.name` attribute containing a unique + string. + :type interface: `Interface` + :param mapping: The mapping to add the found components to. + :type mapping: A dict-like mapping. This only needs to support + containment tests (e.g. `in` and `not in`) and `__setitem__()`. + :raises RuntimeError: when a duplicate key is found. + """ + for component in find_pluggable_components(subpackage, interface): + if component.name in mapping: + raise RuntimeError( + 'Duplicate key "{}" found in {}; previously {}'.format( + component.name, component, mapping[component.name])) + mapping[component.name] = component -- cgit v1.2.3-70-g09d2 From a2668307017e8b3a0c705e5cff49a8fa7dbeac7d Mon Sep 17 00:00:00 2001 From: J08nY Date: Tue, 6 Jun 2017 15:51:10 +0200 Subject: Make config.plugin_configs yield a dict with plugin names. - Allows to better loop over pluging configs and their names. --- src/mailman/config/config.py | 6 ++++-- src/mailman/core/plugins.py | 3 +-- src/mailman/rest/plugins.py | 13 +++++++------ src/mailman/utilities/plugins.py | 2 +- 4 files changed, 13 insertions(+), 11 deletions(-) (limited to 'src/mailman/utilities/plugins.py') diff --git a/src/mailman/config/config.py b/src/mailman/config/config.py index 405b689ef..a5e3ed3a7 100644 --- a/src/mailman/config/config.py +++ b/src/mailman/config/config.py @@ -250,8 +250,10 @@ class Configuration: @property def plugin_configs(self): - """Iterate over all the plugin configuration sections.""" - return self._config.getByCategory('plugin', []) + """Return all the plugin configuration sections.""" + plugin_sections = self._config.getByCategory('plugin', []) + for section in plugin_sections: + yield section.category_and_section_names[1], section @property def language_configs(self): diff --git a/src/mailman/core/plugins.py b/src/mailman/core/plugins.py index 565c590b5..e891f380d 100644 --- a/src/mailman/core/plugins.py +++ b/src/mailman/core/plugins.py @@ -28,13 +28,12 @@ from zope.interface.verify import verifyObject @public def initialize(): """Initialize all enabled plugins.""" - for plugin_config in config.plugin_configs: + for name, plugin_config in config.plugin_configs: plugin_class_path = plugin_config['class'] if as_boolean(plugin_config.enable) and plugin_class_path: plugin_class = find_name(plugin_class_path) plugin = plugin_class() verifyObject(IPlugin, plugin) - name = plugin_config.name.split('.')[-1] plugin.name = name assert plugin.name not in config.plugins, ( 'Duplicate plugin "{}" found in {}'.format( diff --git a/src/mailman/rest/plugins.py b/src/mailman/rest/plugins.py index 2209e634e..fa2605f24 100644 --- a/src/mailman/rest/plugins.py +++ b/src/mailman/rest/plugins.py @@ -29,18 +29,19 @@ class AllPlugins(CollectionMixin): def _resource_as_dict(self, plugin_config): """See `CollectionMixin`.""" + name, plugin_section = plugin_config resource = { - 'name': plugin_config.name.split('.')[-1], - 'class': plugin_config['class'], - 'enable': as_boolean(plugin_config['enable']), - 'path': plugin_config['path'], - 'configuration': plugin_config['configuration'] + 'name': name, + 'class': plugin_section['class'], + 'enable': as_boolean(plugin_section['enable']), + 'path': plugin_section['path'], + 'configuration': plugin_section['configuration'] } return resource def _get_collection(self, request): """See `CollectionMixin`.""" - return config.plugin_configs + return sorted(config.plugin_configs) def on_get(self, request, response): """/plugins""" diff --git a/src/mailman/utilities/plugins.py b/src/mailman/utilities/plugins.py index a8449af31..98a34fbca 100644 --- a/src/mailman/utilities/plugins.py +++ b/src/mailman/utilities/plugins.py @@ -38,7 +38,7 @@ def find_pluggable_components(subpackage, interface): """ yield from find_components('mailman.' + subpackage, interface) package_roots = [plugin.path - for plugin in config.plugin_configs + for name, plugin in config.plugin_configs if as_boolean(plugin.enable) and plugin.path] for package_root in package_roots: package = package_root + '.' + subpackage -- cgit v1.2.3-70-g09d2 From 0923455ba337fa734f89b3be72bb7fde061dcc91 Mon Sep 17 00:00:00 2001 From: J08nY Date: Mon, 19 Jun 2017 15:04:12 +0200 Subject: Don't fail when plugins pre_hook or post_hook fails. - Catches all exceptions raised when running a plugins (pre|post) _hooks. If its pre_hook raises an exception, the plugin is disabled and removed, so its post_hook will not be run and its components will not be loaded. If its post_hook raises an exception we just continue and hope for the best. --- src/mailman/core/initialize.py | 43 ++++++++++++++++++++++++++++++---------- src/mailman/utilities/plugins.py | 6 +++--- 2 files changed, 36 insertions(+), 13 deletions(-) (limited to 'src/mailman/utilities/plugins.py') diff --git a/src/mailman/core/initialize.py b/src/mailman/core/initialize.py index 18e874552..a1314b51e 100644 --- a/src/mailman/core/initialize.py +++ b/src/mailman/core/initialize.py @@ -27,6 +27,7 @@ by the command line arguments. import os import sys import warnings +import traceback import mailman.config.config import mailman.core.logging @@ -149,14 +150,30 @@ def initialize_2(debug=False, propagate_logs=None, testing=False): # Initialize plugins from mailman.core.plugins import initialize as initialize_plugins initialize_plugins() - # Run the plugin pre_hooks + # Check for deprecated features in config. config = mailman.config.config - if "pre_hook" in config.mailman: # pragma: no cover + if 'pre_hook' in config.mailman: # pragma: no cover warnings.warn( - "The pre_hook configuration value has been replaced by the " - "plugins infrastructure.", DeprecationWarning) - for plugin in config.plugins.values(): - plugin.pre_hook() + 'The pre_hook configuration value has been replaced by the ' + 'plugins infrastructure.', DeprecationWarning) + # Run the plugin pre_hooks, if one fails, disable the offending plugin. + for name in list(config.plugins.keys()): + plugin = config.plugins[name] + try: + plugin.pre_hook() + except BaseException: # pragma: no cover + traceback.print_exc() + warnings.warn('Plugin {} failed to run its pre_hook,' + 'it will be disabled and its components' + 'wont be loaded.'.format(name), RuntimeWarning) + # It failed, push disabling overlay to config. This will stop + # components from being loaded. + config.push(name + '_error', """\ +[plugin.{}] +enable: no + """.format(name)) + # And forget about it. This will stop running its pot_hook. + del config.plugins[name] # Instantiate the database class, ensure that it's of the right type, and # initialize it. Then stash the object on our configuration object. utility_name = ('testing' if testing else 'production') @@ -182,12 +199,18 @@ def initialize_3(): """ # Run the plugin post_hooks config = mailman.config.config - if "post_hook" in config.mailman: # pragma: no cover + if 'post_hook' in config.mailman: # pragma: no cover warnings.warn( - "The post_hook configuration value has been replaced by the " - "plugins infrastructure.", DeprecationWarning) + 'The post_hook configuration value has been replaced by the ' + 'plugins infrastructure.', DeprecationWarning) for plugin in config.plugins.values(): - plugin.post_hook() + try: + plugin.post_hook() + except: # pragma: no cover + # A post_hook may fail, here we just hope for the best that the + # plugin can work even if it post_hook failed as it's components + # are already loaded. + pass @public diff --git a/src/mailman/utilities/plugins.py b/src/mailman/utilities/plugins.py index 98a34fbca..3fb5e49bc 100644 --- a/src/mailman/utilities/plugins.py +++ b/src/mailman/utilities/plugins.py @@ -57,8 +57,8 @@ def add_pluggable_components(subpackage, interface, mapping): object's `.name` attribute, which is required. It is a fatal error if that key already exists in the mapping. - :param package: The subpackage path to search. - :type package: string + :param subpackage: The subpackage path to search. + :type subpackage: string :param interface: The interface that returned objects must conform to. Objects found must have a `.name` attribute containing a unique string. @@ -70,7 +70,7 @@ def add_pluggable_components(subpackage, interface, mapping): """ for component in find_pluggable_components(subpackage, interface): if component.name in mapping: - raise RuntimeError( + raise RuntimeError( # pragma: no cover 'Duplicate key "{}" found in {}; previously {}'.format( component.name, component, mapping[component.name])) mapping[component.name] = component -- cgit v1.2.3-70-g09d2 From 8addebbf9802e911c06f6a27b7ffff1e0f1d2e57 Mon Sep 17 00:00:00 2001 From: J08nY Date: Tue, 25 Jul 2017 21:44:54 +0200 Subject: Fix coverage, deprecate, but run non-plugin (post|pre)_hooks. --- src/mailman/app/docs/plugins.rst | 50 +++++++++++++++++++++++++++---- src/mailman/bin/mailman.py | 6 ++-- src/mailman/bin/master.py | 2 +- src/mailman/bin/tests/test_mailman.py | 18 +++++++++-- src/mailman/config/schema.cfg | 9 +++++- src/mailman/core/initialize.py | 28 +++++++++-------- src/mailman/rest/docs/systemconf.rst | 2 ++ src/mailman/rest/tests/test_systemconf.py | 2 ++ src/mailman/utilities/plugins.py | 2 +- 9 files changed, 94 insertions(+), 25 deletions(-) (limited to 'src/mailman/utilities/plugins.py') diff --git a/src/mailman/app/docs/plugins.rst b/src/mailman/app/docs/plugins.rst index a66e4413d..4430179ae 100644 --- a/src/mailman/app/docs/plugins.rst +++ b/src/mailman/app/docs/plugins.rst @@ -117,7 +117,6 @@ initialization process. ... def rest_object(self): ... pass ... """, file=fp) - >>> fp.close() Running the hooks ----------------- @@ -143,12 +142,12 @@ script that will produce no output to force the hooks to run. >>> import subprocess >>> from mailman.testing.layers import ConfigLayer - >>> def call(): + >>> def call(cfg_path, python_path): ... exe = os.path.join(os.path.dirname(sys.executable), 'mailman') ... env = os.environ.copy() ... env.update( - ... MAILMAN_CONFIG_FILE=config_path, - ... PYTHONPATH=config_directory, + ... MAILMAN_CONFIG_FILE=cfg_path, + ... PYTHONPATH=python_path, ... ) ... test_cfg = os.environ.get('MAILMAN_EXTRA_TESTING_CFG') ... if test_cfg is not None: @@ -161,10 +160,51 @@ script that will produce no output to force the hooks to run. ... stdout, stderr = proc.communicate() ... assert proc.returncode == 0, stderr ... print(stdout) + ... print(stderr) - >>> call() + >>> call(config_path, config_directory) pre-hook: 1 post-hook: 2 + >>> os.remove(config_path) + +Deprecated hooks +---------------- + +The old-style `pre_hook` and `post_hook` callables are deprecated and are no +longer called upon startup. +:: + + >>> deprecated_hook_path = os.path.join(config_directory, 'deprecated_hooks.py') + >>> with open(deprecated_hook_path, 'w') as fp: + ... print("""\ + ... def do_something(): + ... print("does something") + ... + ... def do_something_else(): + ... print("does something else") + ... + ... """, file=fp) + + >>> deprecated_config_path = os.path.join(config_directory, 'deprecated.cfg') + >>> with open(deprecated_config_path, 'w') as fp: + ... print("""\ + ... [meta] + ... extends: test.cfg + ... + ... [mailman] + ... pre_hook: deprecated_hooks.do_something + ... post_hook: deprecated_hooks.do_something_else + ... """, file=fp) + + >>> call(deprecated_config_path, config_directory) + does something + does something else + + ... UserWarning: The pre_hook configuration value has been replaced by the plugins infrastructure. ... + ... UserWarning: The post_hook configuration value has been replaced by the plugins infrastructure. ... + + + >>> os.remove(deprecated_config_path) diff --git a/src/mailman/bin/mailman.py b/src/mailman/bin/mailman.py index ece223180..f8006218d 100644 --- a/src/mailman/bin/mailman.py +++ b/src/mailman/bin/mailman.py @@ -44,9 +44,9 @@ class Subcommands(click.MultiCommand): self._commands) self._loaded = True - def list_commands(self, ctx): + def list_commands(self, ctx): # pragma: nocover self._load() - return sorted(self._commands) # pragma: nocover + return sorted(self._commands) def get_command(self, ctx, name): self._load() @@ -88,7 +88,7 @@ class Subcommands(click.MultiCommand): def initialize_config(ctx, param, value): - if ctx.resilient_parsing: + if ctx.resilient_parsing: # pragma: nocover return initialize(value) diff --git a/src/mailman/bin/master.py b/src/mailman/bin/master.py index a23fb8e0b..ccaa16398 100644 --- a/src/mailman/bin/master.py +++ b/src/mailman/bin/master.py @@ -405,7 +405,7 @@ class Loop: except InterruptedError: # pragma: nocover # If the system call got interrupted, just restart it. continue - if pid not in self._kids: + if pid not in self._kids: # pragma: nocover # Not a runner subprocess, maybe a plugin started one # ignore it continue diff --git a/src/mailman/bin/tests/test_mailman.py b/src/mailman/bin/tests/test_mailman.py index 54ee54bce..196437291 100644 --- a/src/mailman/bin/tests/test_mailman.py +++ b/src/mailman/bin/tests/test_mailman.py @@ -27,6 +27,7 @@ from mailman.config import config from mailman.database.transaction import transaction from mailman.testing.layers import ConfigLayer from mailman.utilities.datetime import now +from pkg_resources import resource_filename from unittest.mock import patch @@ -36,7 +37,19 @@ class TestMailmanCommand(unittest.TestCase): def setUp(self): self._command = CliRunner() - def test_mailman_command_without_subcommand_prints_help(self): + def test_mailman_command_config(self): + config_path = resource_filename('mailman.testing', 'testing.cfg') + with patch('mailman.bin.mailman.initialize') as init: + self._command.invoke(main, ('-C', config_path, 'info')) + init.assert_called_once_with(config_path) + + def test_mailman_command_no_config(self): + with patch('mailman.bin.mailman.initialize') as init: + self._command.invoke(main, ('info',)) + init.assert_called_once_with(None) + + @patch('mailman.bin.mailman.initialize') + def test_mailman_command_without_subcommand_prints_help(self, mock): # Issue #137: Running `mailman` without a subcommand raises an # AttributeError. result = self._command.invoke(main) @@ -46,7 +59,8 @@ class TestMailmanCommand(unittest.TestCase): # command line. self.assertEqual(lines[0], 'Usage: main [OPTIONS] COMMAND [ARGS]...') - def test_mailman_command_with_bad_subcommand_prints_help(self): + @patch('mailman.bin.mailman.initialize') + def test_mailman_command_with_bad_subcommand_prints_help(self, mock): # Issue #137: Running `mailman` without a subcommand raises an # AttributeError. result = self._command.invoke(main, ('not-a-subcommand',)) diff --git a/src/mailman/config/schema.cfg b/src/mailman/config/schema.cfg index a55c37ff4..742bceae1 100644 --- a/src/mailman/config/schema.cfg +++ b/src/mailman/config/schema.cfg @@ -74,10 +74,17 @@ html_to_plain_text_command: /usr/bin/lynx -dump $filename # unpredictable. listname_chars: [-_.0-9a-z] +# Deprecated, callable run before DB initialization. +pre_hook: + +# Deprecated, callable run after DB initialization. +post_hook: + + [plugin.master] # Plugin package # It's sub packages will be searched for components. -# - commands for IEmailCommand +# - commands for IEmailCommand and ICliSubCommand # - chains for IChain # - rules for IRule # - pipelines for IPipeline diff --git a/src/mailman/core/initialize.py b/src/mailman/core/initialize.py index a1314b51e..830bd9b84 100644 --- a/src/mailman/core/initialize.py +++ b/src/mailman/core/initialize.py @@ -31,7 +31,9 @@ import traceback import mailman.config.config import mailman.core.logging +from mailman.core.i18n import _ from mailman.interfaces.database import IDatabaseFactory +from mailman.utilities.modules import call_name from pkg_resources import resource_string as resource_bytes from public import public from zope.component import getUtility @@ -152,27 +154,28 @@ def initialize_2(debug=False, propagate_logs=None, testing=False): initialize_plugins() # Check for deprecated features in config. config = mailman.config.config - if 'pre_hook' in config.mailman: # pragma: no cover + if config.mailman.pre_hook: # pragma: nocover warnings.warn( - 'The pre_hook configuration value has been replaced by the ' - 'plugins infrastructure.', DeprecationWarning) + _('The pre_hook configuration value has been replaced by the ' + 'plugins infrastructure.'), UserWarning) + call_name(config.mailman.pre_hook) # Run the plugin pre_hooks, if one fails, disable the offending plugin. for name in list(config.plugins.keys()): plugin = config.plugins[name] try: plugin.pre_hook() - except BaseException: # pragma: no cover + except: # pragma: nocover traceback.print_exc() - warnings.warn('Plugin {} failed to run its pre_hook,' - 'it will be disabled and its components' - 'wont be loaded.'.format(name), RuntimeWarning) + warnings.warn(_('Plugin $name failed to run its pre_hook,' + 'it will be disabled and its components' + 'wont be loaded.'), RuntimeWarning) # It failed, push disabling overlay to config. This will stop # components from being loaded. config.push(name + '_error', """\ [plugin.{}] enable: no """.format(name)) - # And forget about it. This will stop running its pot_hook. + # And forget about it. This will stop running its post_hook. del config.plugins[name] # Instantiate the database class, ensure that it's of the right type, and # initialize it. Then stash the object on our configuration object. @@ -199,14 +202,15 @@ def initialize_3(): """ # Run the plugin post_hooks config = mailman.config.config - if 'post_hook' in config.mailman: # pragma: no cover + if config.mailman.post_hook: # pragma: nocover warnings.warn( - 'The post_hook configuration value has been replaced by the ' - 'plugins infrastructure.', DeprecationWarning) + _('The post_hook configuration value has been replaced by the ' + 'plugins infrastructure.'), UserWarning) + call_name(config.mailman.post_hook) for plugin in config.plugins.values(): try: plugin.post_hook() - except: # pragma: no cover + except: # pragma: nocover # A post_hook may fail, here we just hope for the best that the # plugin can work even if it post_hook failed as it's components # are already loaded. diff --git a/src/mailman/rest/docs/systemconf.rst b/src/mailman/rest/docs/systemconf.rst index f5c9b691a..a65b81ab8 100644 --- a/src/mailman/rest/docs/systemconf.rst +++ b/src/mailman/rest/docs/systemconf.rst @@ -24,6 +24,8 @@ You can also get all the values for a particular section, such as the listname_chars: [-_.0-9a-z] noreply_address: noreply pending_request_life: 3d + post_hook: + pre_hook: self_link: http://localhost:9001/3.0/system/configuration/mailman sender_headers: from from_ reply-to sender site_owner: noreply@example.com diff --git a/src/mailman/rest/tests/test_systemconf.py b/src/mailman/rest/tests/test_systemconf.py index fe5d951a8..59f0619a3 100644 --- a/src/mailman/rest/tests/test_systemconf.py +++ b/src/mailman/rest/tests/test_systemconf.py @@ -46,6 +46,8 @@ class TestSystemConfiguration(unittest.TestCase): listname_chars='[-_.0-9a-z]', noreply_address='noreply', pending_request_life='3d', + post_hook='', + pre_hook='', self_link='http://localhost:9001/3.0/system/configuration/mailman', sender_headers='from from_ reply-to sender', site_owner='noreply@example.com', diff --git a/src/mailman/utilities/plugins.py b/src/mailman/utilities/plugins.py index 3fb5e49bc..aad909056 100644 --- a/src/mailman/utilities/plugins.py +++ b/src/mailman/utilities/plugins.py @@ -70,7 +70,7 @@ def add_pluggable_components(subpackage, interface, mapping): """ for component in find_pluggable_components(subpackage, interface): if component.name in mapping: - raise RuntimeError( # pragma: no cover + raise RuntimeError( # pragma: nocover 'Duplicate key "{}" found in {}; previously {}'.format( component.name, component, mapping[component.name])) mapping[component.name] = component -- cgit v1.2.3-70-g09d2