From 159e24a95d52826f3072261d3b96d3f4247e931a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?R=C3=A9gis=20Behmo?= <regis@behmo.com>
Date: Tue, 14 Jan 2020 13:32:45 +0100
Subject: [PATCH] Refactor plugin internals
This is for supporting json-based plugins. The great thing about this
change is that it allows us to easily print plugin version numbers in
`plugins list`.
---
CHANGELOG.md | 4 +
Makefile | 2 +
bin/main.py | 18 +++--
tests/test_plugins.py | 73 ++++++++++--------
tutor/commands/plugins.py | 18 +++--
tutor/config.py | 15 ++--
tutor/plugins.py | 155 +++++++++++++++++++++++++++-----------
7 files changed, 190 insertions(+), 95 deletions(-)
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 9deb306..8279a60 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -2,6 +2,10 @@
Note: Breaking changes between versions are indicated by "💥".
+## Unreleased
+
+- 💥[Improvement] The output of `plugins list` now includes plugin version numbers.
+
## 3.10.1 (2020-01-13)
- [Improvement] Explicitely point to docker.io images, when necessary, for [podman](https://podman.io/) compatibility.
diff --git a/Makefile b/Makefile
index eb63d58..bf7bf05 100644
--- a/Makefile
+++ b/Makefile
@@ -87,7 +87,9 @@ ci-bundle: bundle ## Create bundle and run basic tests
./dist/tutor config printroot
yes "" | ./dist/tutor config save --interactive
./dist/tutor config save
+ ./dist/tutor plugins list
./dist/tutor plugins enable discovery ecommerce figures lts minio notes xqueue
+ ./dist/tutor plugins list
./dist/tutor lts --help
./releases/github-release: ## Download github-release binary
diff --git a/bin/main.py b/bin/main.py
index 7ec9145..81ed7e9 100755
--- a/bin/main.py
+++ b/bin/main.py
@@ -1,16 +1,20 @@
#!/usr/bin/env python3
-import importlib
-
-from tutor.plugins import Plugins
+from tutor.plugins import OfficialPlugin
# Manually install plugins (this is for creating the bundle)
-for plugin in ["discovery", "ecommerce", "figures", "lts", "minio", "notes", "xqueue"]:
+for plugin_name in [
+ "discovery",
+ "ecommerce",
+ "figures",
+ "lts",
+ "minio",
+ "notes",
+ "xqueue",
+]:
try:
- module = importlib.import_module("tutor{}.plugin".format(plugin))
+ OfficialPlugin.INSTALLED.append(OfficialPlugin(plugin_name))
except ImportError:
pass
- else:
- Plugins.EXTRA_INSTALLED[plugin] = module
from tutor.commands.cli import main
diff --git a/tests/test_plugins.py b/tests/test_plugins.py
index ba1157f..447431e 100644
--- a/tests/test_plugins.py
+++ b/tests/test_plugins.py
@@ -20,22 +20,16 @@ class PluginsTests(unittest.TestCase):
self.assertFalse(plugins.is_installed("dummy"))
def test_extra_installed(self):
- class plugin1:
- pass
-
- class plugin2:
- pass
+ plugin1 = plugins.BasePlugin("plugin1", None)
+ plugin2 = plugins.BasePlugin("plugin2", None)
- plugins.Plugins.EXTRA_INSTALLED["plugin1"] = plugin1
- plugins.Plugins.EXTRA_INSTALLED["plugin2"] = plugin2
+ plugins.OfficialPlugin.INSTALLED.append(plugin1)
+ plugins.OfficialPlugin.INSTALLED.append(plugin2)
with unittest.mock.patch.object(
- plugins.Plugins,
- "iter_installed_entrypoints",
- return_value=[("plugin1", plugin1)],
+ plugins.EntrypointPlugin, "iter_installed", return_value=[plugin1],
):
self.assertEqual(
- [("plugin1", plugin1), ("plugin2", plugin2)],
- list(plugins.iter_installed()),
+ [plugin1, plugin2], list(plugins.iter_installed()),
)
def test_enable(self):
@@ -67,17 +61,18 @@ class PluginsTests(unittest.TestCase):
patches = {"patch1": "Hello {{ ID }}"}
with unittest.mock.patch.object(
- plugins.Plugins, "iter_enabled", return_value=[("plugin1", plugin1)]
+ plugins.Plugins,
+ "iter_enabled",
+ return_value=[plugins.BasePlugin("plugin1", plugin1)],
):
patches = list(plugins.iter_patches({}, "patch1"))
self.assertEqual([("plugin1", "Hello {{ ID }}")], patches)
def test_plugin_without_patches(self):
- class plugin1:
- pass
-
with unittest.mock.patch.object(
- plugins.Plugins, "iter_enabled", return_value=[("plugin1", plugin1)]
+ plugins.Plugins,
+ "iter_enabled",
+ return_value=[plugins.BasePlugin("plugin1", None)],
):
patches = list(plugins.iter_patches({}, "patch1"))
self.assertEqual([], patches)
@@ -94,7 +89,9 @@ class PluginsTests(unittest.TestCase):
}
with unittest.mock.patch.object(
- plugins.Plugins, "iter_enabled", return_value=[("plugin1", plugin1)]
+ plugins.Plugins,
+ "iter_enabled",
+ return_value=[plugins.BasePlugin("plugin1", plugin1)],
):
tutor_config.load_plugins(config, defaults)
@@ -116,7 +113,9 @@ class PluginsTests(unittest.TestCase):
config = {"set": {"ID": "newid"}}
with unittest.mock.patch.object(
- plugins.Plugins, "iter_enabled", return_value=[("plugin1", plugin1)]
+ plugins.Plugins,
+ "iter_enabled",
+ return_value=[plugins.BasePlugin("plugin1", plugin1)],
):
tutor_config.load_plugins(config, {})
@@ -129,7 +128,9 @@ class PluginsTests(unittest.TestCase):
config = {"set": {"PARAM1": "{{ 128|random_string }}"}}
with unittest.mock.patch.object(
- plugins.Plugins, "iter_enabled", return_value=[("plugin1", plugin1)]
+ plugins.Plugins,
+ "iter_enabled",
+ return_value=[plugins.BasePlugin("plugin1", plugin1)],
):
tutor_config.load_plugins(config, {})
self.assertEqual(128, len(config["PARAM1"]))
@@ -142,7 +143,9 @@ class PluginsTests(unittest.TestCase):
config = {"defaults": {"PARAM2": "{{ PARAM1 }}"}}
with unittest.mock.patch.object(
- plugins.Plugins, "iter_enabled", return_value=[("plugin1", plugin1)]
+ plugins.Plugins,
+ "iter_enabled",
+ return_value=[plugins.BasePlugin("plugin1", plugin1)],
):
tutor_config.load_plugins(config, defaults)
self.assertEqual("{{ PARAM1 }}", defaults["PLUGIN1_PARAM2"])
@@ -154,12 +157,16 @@ class PluginsTests(unittest.TestCase):
config = {"add": {"PARAM1": "{{ 10|random_string }}"}}
with unittest.mock.patch.object(
- plugins.Plugins, "iter_enabled", return_value=[("plugin1", plugin1)]
+ plugins.Plugins,
+ "iter_enabled",
+ return_value=[plugins.BasePlugin("plugin1", plugin1)],
):
tutor_config.load_plugins(config, {})
value1 = config["PLUGIN1_PARAM1"]
with unittest.mock.patch.object(
- plugins.Plugins, "iter_enabled", return_value=[("plugin1", plugin1)]
+ plugins.Plugins,
+ "iter_enabled",
+ return_value=[plugins.BasePlugin("plugin1", plugin1)],
):
tutor_config.load_plugins(config, {})
value2 = config["PLUGIN1_PARAM1"]
@@ -173,7 +180,9 @@ class PluginsTests(unittest.TestCase):
hooks = {"init": ["myclient"]}
with unittest.mock.patch.object(
- plugins.Plugins, "iter_enabled", return_value=[("plugin1", plugin1)]
+ plugins.Plugins,
+ "iter_enabled",
+ return_value=[plugins.BasePlugin("plugin1", plugin1)],
):
self.assertEqual(
[("plugin1", ["myclient"])], list(plugins.iter_hooks({}, "init"))
@@ -184,7 +193,9 @@ class PluginsTests(unittest.TestCase):
templates = "/tmp/templates"
with unittest.mock.patch.object(
- plugins.Plugins, "iter_enabled", return_value=[("plugin1", plugin1)]
+ plugins.Plugins,
+ "iter_enabled",
+ return_value=[plugins.BasePlugin("plugin1", plugin1)],
):
self.assertEqual(
[("plugin1", "/tmp/templates")], list(plugins.iter_template_roots({}))
@@ -192,11 +203,13 @@ class PluginsTests(unittest.TestCase):
def test_plugins_are_updated_on_config_change(self):
config = {"PLUGINS": []}
- instance1 = plugins.Plugins(config)
- self.assertEqual(0, len(list(instance1.iter_enabled())))
+ plugins1 = plugins.Plugins(config)
+ self.assertEqual(0, len(list(plugins1.iter_enabled())))
config["PLUGINS"].append("plugin1")
with unittest.mock.patch.object(
- plugins.Plugins, "iter_installed", return_value=[("plugin1", None)]
+ plugins.Plugins,
+ "iter_installed",
+ return_value=[plugins.BasePlugin("plugin1", None)],
):
- instance2 = plugins.Plugins(config)
- self.assertEqual(1, len(list(instance2.iter_enabled())))
+ plugins2 = plugins.Plugins(config)
+ self.assertEqual(1, len(list(plugins2.iter_enabled())))
diff --git a/tutor/commands/plugins.py b/tutor/commands/plugins.py
index 7874180..1078369 100644
--- a/tutor/commands/plugins.py
+++ b/tutor/commands/plugins.py
@@ -24,9 +24,13 @@ def plugins_command():
@click.pass_obj
def list_command(context):
config = tutor_config.load_user(context.root)
- for name, _ in plugins.iter_installed():
- status = "" if plugins.is_enabled(config, name) else " (disabled)"
- print("{plugin}{status}".format(plugin=name, status=status))
+ for plugin in plugins.iter_installed():
+ status = "" if plugins.is_enabled(config, plugin.name) else " (disabled)"
+ print(
+ "{plugin}@{version}{status}".format(
+ plugin=plugin.name, status=status, version=plugin.version
+ )
+ )
@click.command(help="Enable a plugin")
@@ -67,10 +71,10 @@ def add_plugin_commands(command_group):
Add commands provided by all plugins to the given command group. Each command is
added with a name that is equal to the plugin name.
"""
- for plugin_name, command in plugins.iter_commands():
- if isinstance(command, click.Command):
- command.name = plugin_name
- command_group.add_command(command)
+ for plugin in plugins.iter_installed():
+ if isinstance(plugin.command, click.Command):
+ plugin.command.name = plugin.name
+ command_group.add_command(plugin.command)
plugins_command.add_command(list_command)
diff --git a/tutor/config.py b/tutor/config.py
index 04688d7..4b46ac1 100644
--- a/tutor/config.py
+++ b/tutor/config.py
@@ -107,24 +107,21 @@ def load_plugins(config, defaults):
"""
Add, override and set new defaults from plugins.
"""
- for plugin_name, plugin in plugins.iter_enabled(config):
- plugin_prefix = plugin_name.upper() + "_"
- plugin_config = plugins.get_callable_attr(plugin, "config", {})
-
+ for plugin in plugins.iter_enabled(config):
# Add new config key/values
- for key, value in plugin_config.get("add", {}).items():
- new_key = plugin_prefix + key
+ for key, value in plugin.config_add.items():
+ new_key = plugin.config_key(key)
if new_key not in config:
config[new_key] = env.render_unknown(config, value)
# Set existing config key/values: here, we do not override existing values
- for key, value in plugin_config.get("set", {}).items():
+ for key, value in plugin.config_set.items():
if key not in config:
config[key] = env.render_unknown(config, value)
# Create new defaults
- for key, value in plugin_config.get("defaults", {}).items():
- defaults[plugin_prefix + key] = value
+ for key, value in plugin.config_defaults.items():
+ defaults[plugin.config_key(key)] = value
def upgrade_obsolete(config):
diff --git a/tutor/plugins.py b/tutor/plugins.py
index a428c7d..9f60cfb 100644
--- a/tutor/plugins.py
+++ b/tutor/plugins.py
@@ -1,4 +1,5 @@
from copy import deepcopy
+import importlib
import pkg_resources
from . import exceptions
@@ -7,16 +8,15 @@ from . import exceptions
CONFIG_KEY = "PLUGINS"
-class Plugins:
+class BasePlugin:
"""
- Tutor plugins are regular python packages that have a 'tutor.plugin.v0' entrypoint.
-
- The API for Tutor plugins is currently in development. The entrypoint will switch to
- 'tutor.plugin.v1' once it is stabilised.
-
- This entrypoint must point to a module or a class that implements one or more of the
+ Tutor plugins are defined by a name and an object that implements one or more of the
following properties:
+ `config` (dict str->dict(str->str)): contains "add", "set", "default" keys. Entries
+ in these dicts will be added or override the global configuration. Keys in "add" and
+ "set" will be prefixed by the plugin name in uppercase.
+
`patches` (dict str->str): entries in this dict will be used to patch the rendered
Tutor templates. For instance, to add "somecontent" to a template that includes '{{
patch("mypatch") }}', set: `patches["mypatch"] = "somecontent"`. It is recommended
@@ -36,11 +36,94 @@ class Plugins:
It is then assumed that there are `myplugin/hooks/service1/init` and
`myplugin/hooks/service2/init` templates in the plugin `templates` directory.
+
+ `command` (click.Command): if a plugin exposes a `command` attribute, users will be able to run it from the command line as `tutor pluginname`.
+ """
+
+ def __init__(self, name, obj):
+ self.name = name
+ self.config = get_callable_attr(obj, "config", {})
+ self.patches = get_callable_attr(obj, "patches", default={})
+ self.hooks = get_callable_attr(obj, "hooks", default={})
+ self.templates_root = get_callable_attr(obj, "templates", default=None)
+ self.command = getattr(obj, "command", None)
+
+ def config_key(self, key):
+ """
+ Config keys in the "add" and "defaults" dicts should be prefixed by the plugin name, in uppercase.
+ """
+ return self.name.upper() + "_" + key
+
+ @property
+ def config_add(self):
+ return self.config.get("add", {})
+
+ @property
+ def config_set(self):
+ return self.config.get("set", {})
+
+ @property
+ def config_defaults(self):
+ return self.config.get("defaults", {})
+
+ @property
+ def version(self):
+ raise NotImplementedError
+
+ @classmethod
+ def iter_installed(cls):
+ raise NotImplementedError
+
+
+class EntrypointPlugin(BasePlugin):
+ """
+ Entrypoint plugins are regular python packages that have a 'tutor.plugin.v0' entrypoint.
+
+ The API for Tutor plugins is currently in development. The entrypoint will switch to
+ 'tutor.plugin.v1' once it is stabilised.
"""
ENTRYPOINT = "tutor.plugin.v0"
+
+ def __init__(self, entrypoint):
+ super().__init__(entrypoint.name, entrypoint.load())
+ self.entrypoint = entrypoint
+
+ @property
+ def version(self):
+ return self.entrypoint.dist.version
+
+ @classmethod
+ def iter_installed(cls):
+ for entrypoint in pkg_resources.iter_entry_points(cls.ENTRYPOINT):
+ yield cls(entrypoint)
+
+
+class OfficialPlugin(BasePlugin):
+ """
+ Official plugins have a "plugin" module which exposes a __version__
+ attribute.
+ Official plugins should be manually added to INSTALLED.
+ """
+
+ INSTALLED = []
+
+ def __init__(self, name):
+ self.module = importlib.import_module("tutor{}.plugin".format(name))
+ super().__init__(name, self.module)
+
+ @property
+ def version(self):
+ return self.module.__version__
+
+ @classmethod
+ def iter_installed(cls):
+ yield from cls.INSTALLED
+
+
+class Plugins:
+
INSTANCE = None
- EXTRA_INSTALLED = {}
def __init__(self, config):
self.config = deepcopy(config)
@@ -48,27 +131,24 @@ class Plugins:
self.hooks = {}
self.template_roots = {}
- for plugin_name, plugin in self.iter_enabled():
- patches = get_callable_attr(plugin, "patches", {})
- for patch_name, content in patches.items():
+ for plugin in self.iter_enabled():
+ for patch_name, content in plugin.patches.items():
if patch_name not in self.patches:
self.patches[patch_name] = {}
- self.patches[patch_name][plugin_name] = content
+ self.patches[patch_name][plugin.name] = content
- hooks = get_callable_attr(plugin, "hooks", {})
- for hook_name, services in hooks.items():
+ for hook_name, services in plugin.hooks.items():
if hook_name not in self.hooks:
self.hooks[hook_name] = {}
- self.hooks[hook_name][plugin_name] = services
+ self.hooks[hook_name][plugin.name] = services
- templates_root = get_callable_attr(plugin, "templates")
- if templates_root:
- self.template_roots[plugin_name] = templates_root
+ if plugin.templates_root:
+ self.template_roots[plugin.name] = plugin.templates_root
@classmethod
def clear(cls):
cls.INSTANCE = None
- cls.EXTRA_INSTALLED.clear()
+ OfficialPlugin.INSTALLED.clear()
@classmethod
def instance(cls, config):
@@ -78,20 +158,21 @@ class Plugins:
@classmethod
def iter_installed(cls):
- yield from cls.EXTRA_INSTALLED.items()
- for name, module in cls.iter_installed_entrypoints():
- if name not in cls.EXTRA_INSTALLED:
- yield name, module
-
- @classmethod
- def iter_installed_entrypoints(cls):
- for entrypoint in pkg_resources.iter_entry_points(cls.ENTRYPOINT):
- yield (entrypoint.name, entrypoint.load())
+ """
+ Iterate on all installed plugins. Plugins are deduplicated by name.
+ """
+ classes = [OfficialPlugin, EntrypointPlugin]
+ installed_plugin_names = set()
+ for PluginClass in classes:
+ for plugin in PluginClass.iter_installed():
+ if plugin.name not in installed_plugin_names:
+ installed_plugin_names.add(plugin.name)
+ yield plugin
def iter_enabled(self):
- for name, plugin in self.iter_installed():
- if is_enabled(self.config, name):
- yield name, plugin
+ for plugin in self.iter_installed():
+ if is_enabled(self.config, plugin.name):
+ yield plugin
def iter_patches(self, name):
plugin_patches = self.patches.get(name, {})
@@ -114,7 +195,7 @@ def get_callable_attr(plugin, attr_name, default=None):
def is_installed(name):
- plugin_names = [name for name, _ in iter_installed()]
+ plugin_names = [plugin.name for plugin in iter_installed()]
return name in plugin_names
@@ -156,13 +237,3 @@ def iter_hooks(config, hook_name):
def iter_template_roots(config):
yield from Plugins.instance(config).iter_template_roots()
-
-
-def iter_commands():
- """
- Iterate over all plugins that provide a `command` attribute.
- """
- for plugin_name, plugin in iter_installed():
- command = getattr(plugin, "command", None)
- if command:
- yield plugin_name, command
--
GitLab