remove namespace plugin mechanism (#4188)

Author: epwalsh

allennlp/common/plugins.py

@@ -1,52 +1,23 @@
 """
 Plugin management.
 
-AllenNLP supports loading "plugins" dynamically. A plugin is just a Python package that is found
-by AllenNLP with the methods provided in this module.
-
-There are two ways of declaring plugins for discovery:
-
-- Writing the package name to import in a file (typically ".allennlp_plugins", in the current
-  directory from which the command "allennlp" is run). This is the simplest approach.
-
-- Creating a folder called "allennlp_plugins" that's in the Python path when you run the
-  "allennlp" command (typically under your project's root directory), then creating a subfolder
-  with the name you want and creating an "__init__.py" file that imports the code you want (e.g.,
-  your Python package). This option is preferred when you want to create a pip-installable
-  package and you want to make your AllenNLP plugin available when users install your package.
-  See [allennlp-server](https://github.com/allenai/allennlp-server) for an example.
+AllenNLP supports loading "plugins" dynamically. A plugin is just a Python package that
+can be found and imported by AllenNLP. This is done by creating a file named `.allennlp_plugins`
+in the directory where the `allennlp` command is run that lists the modules that should be loaded,
+one per line.
 """
+
 import importlib
 import logging
 import os
-import pkgutil
-import sys
 from typing import Iterable
 
-from allennlp.common.util import push_python_path
+from allennlp.common.util import push_python_path, import_module_and_submodules
 
 logger = logging.getLogger(__name__)
 
 
-def discover_namespace_plugins(
-    namespace_name: str = "allennlp_plugins",
-) -> Iterable[pkgutil.ModuleInfo]:
-    """
-    Returns an iterable of the plugins found, declared within the namespace package `namespace_name`.
-    """
-    try:
-        reload = namespace_name in sys.modules
-
-        namespace_module = importlib.import_module(namespace_name)
-
-        if reload:
-            importlib.reload(namespace_module)
-
-        return pkgutil.iter_modules(
-            namespace_module.__path__, namespace_module.__name__ + "."  # type: ignore
-        )
-    except ModuleNotFoundError:
-        return []
+DEFAULT_PLUGINS = ("allennlp_models",)
 
 
 def discover_file_plugins(plugins_filename: str = ".allennlp_plugins") -> Iterable[str]:
@@ -68,15 +39,19 @@ def discover_plugins() -> Iterable[str]:
     Returns an iterable of the plugins found.
     """
     with push_python_path("."):
-        for module_info in discover_namespace_plugins():
-            yield module_info.name
         yield from discover_file_plugins()
 
 
 def import_plugins() -> None:
     """
     Imports the plugins found with `discover_plugins()`.
     """
+    for module in DEFAULT_PLUGINS:
+        try:
+            # For default plugins we recursively import everything.
+            import_module_and_submodules(module)
+        except ModuleNotFoundError:
+            pass
     for module_name in discover_plugins():
         try:
             importlib.import_module(module_name)

allennlp/tests/commands/main_test.py

@@ -1,7 +1,5 @@
-import io
 import shutil
 import sys
-from contextlib import redirect_stdout
 
 import pytest
 from overrides import overrides
@@ -12,7 +10,6 @@
 from allennlp.common.plugins import discover_plugins
 from allennlp.common.testing import AllenNlpTestCase
 from allennlp.common.util import push_python_path, pushd
-from allennlp.tests.common.plugins_util import pip_install
 
 
 class TestMain(AllenNlpTestCase):
@@ -133,46 +130,13 @@ def test_other_modules(self):
 
     def test_file_plugin_loaded(self):
         plugins_root = self.FIXTURES_ROOT / "plugins"
-        # "d" sets a "local" file plugin, because it's supposed to be run from that directory
-        # and has a ".allennlp_plugins" file in it.
-        project_d_fixtures_root = plugins_root / "project_d"
 
         sys.argv = ["allennlp"]
 
         available_plugins = set(discover_plugins())
         self.assertSetEqual(set(), available_plugins)
 
-        with pushd(project_d_fixtures_root):
+        with pushd(plugins_root):
             main()
             subcommands_available = Subcommand.list_available()
             self.assertIn("d", subcommands_available)
-
-    def test_namespace_plugin_loaded(self):
-        plugins_root = self.FIXTURES_ROOT / "plugins"
-        # "a" sets a "global" namespace plugin, because it's gonna be installed with pip.
-        project_a_fixtures_root = plugins_root / "project_a"
-
-        sys.argv = ["allennlp"]
-
-        available_plugins = set(discover_plugins())
-        self.assertSetEqual(set(), available_plugins)
-
-        with pip_install(project_a_fixtures_root, "a"):
-            main()
-
-        subcommands_available = Subcommand.list_available()
-        self.assertIn("a", subcommands_available)
-
-    def test_subcommand_plugin_is_available(self):
-        plugins_root = self.FIXTURES_ROOT / "plugins"
-        allennlp_server_fixtures_root = plugins_root / "allennlp_server"
-
-        sys.argv = ["allennlp"]
-
-        with pip_install(
-            allennlp_server_fixtures_root, "allennlp_server"
-        ), io.StringIO() as buf, redirect_stdout(buf):
-            main()
-            output = buf.getvalue()
-
-        self.assertIn("    serve", output)

allennlp/tests/common/plugins_test.py

@@ -1,222 +1,32 @@
-import distutils.dir_util
-import tempfile
-
-import pytest
 from overrides import overrides
 
 from allennlp.commands import Subcommand
 from allennlp.common.plugins import (
-    discover_file_plugins,
-    discover_namespace_plugins,
     discover_plugins,
     import_plugins,
 )
 from allennlp.common.testing import AllenNlpTestCase
 from allennlp.common.util import pushd
-from allennlp.tests.common.plugins_util import pip_install
 
 
 class TestPlugins(AllenNlpTestCase):
     @overrides
     def setUp(self):
         super().setUp()
         self.plugins_root = self.FIXTURES_ROOT / "plugins"
-        # "a" sets a "global" namespace plugin, because it's gonna be installed with pip.
-        self.project_a_fixtures_root = self.plugins_root / "project_a"
-        # "b" sets a "local" namespace plugin, because it's supposed to be run from that directory.
-        self.project_b_fixtures_root = self.plugins_root / "project_b"
-        # "c" sets a "global" namespace plugin, because it's gonna be installed with pip.
-        self.project_c_fixtures_root = self.plugins_root / "project_c"
-        # "d" sets a "local" file plugin, because it's supposed to be run from that directory
-        # and has a ".allennlp_plugins" file in it.
-        self.project_d_fixtures_root = self.plugins_root / "project_d"
 
     def test_no_plugins(self):
         available_plugins = set(discover_plugins())
         self.assertSetEqual(set(), available_plugins)
 
-    def test_namespace_package_does_not_exist(self):
-        available_plugins = set(discover_namespace_plugins("dummy_namespace"))
-        self.assertSetEqual(set(), available_plugins)
-
-    def test_file_plugins_does_not_exist(self):
-        available_plugins = set(discover_file_plugins("dummy_file"))
-        self.assertSetEqual(set(), available_plugins)
-
-    def test_global_namespace_plugin(self):
-        available_plugins = set(discover_plugins())
-        self.assertSetEqual(set(), available_plugins)
-
-        with pip_install(self.project_a_fixtures_root, "a"):
-            available_plugins = set(discover_plugins())
-            self.assertSetEqual({"allennlp_plugins.a"}, available_plugins)
-
-            import_plugins()
-            subcommands_available = Subcommand.list_available()
-            self.assertIn("a", subcommands_available)
-
-    def test_two_global_namespace_plugins(self):
-        available_plugins = set(discover_plugins())
-        self.assertSetEqual(set(), available_plugins)
-
-        with pip_install(self.project_a_fixtures_root, "a"), pip_install(
-            self.project_c_fixtures_root, "c"
-        ):
-            available_plugins = set(discover_plugins())
-            self.assertSetEqual({"allennlp_plugins.a", "allennlp_plugins.c"}, available_plugins)
-
-            import_plugins()
-            subcommands_available = Subcommand.list_available()
-            self.assertIn("a", subcommands_available)
-            self.assertIn("c", subcommands_available)
-
-    def test_local_namespace_plugin(self):
-        available_plugins = set(discover_plugins())
-        self.assertSetEqual(set(), available_plugins)
-
-        with pushd(self.project_b_fixtures_root):
-            available_plugins = set(discover_plugins())
-            self.assertSetEqual({"allennlp_plugins.b"}, available_plugins)
-
-            import_plugins()
-            subcommands_available = Subcommand.list_available()
-            self.assertIn("b", subcommands_available)
-
     def test_file_plugin(self):
         available_plugins = set(discover_plugins())
         self.assertSetEqual(set(), available_plugins)
 
-        with pushd(self.project_d_fixtures_root):
+        with pushd(self.plugins_root):
             available_plugins = set(discover_plugins())
             self.assertSetEqual({"d"}, available_plugins)
 
             import_plugins()
             subcommands_available = Subcommand.list_available()
             self.assertIn("d", subcommands_available)
-
-    def test_local_namespace_plugin_different_path(self):
-        available_plugins = set(discover_plugins())
-        self.assertSetEqual(set(), available_plugins)
-
-        with tempfile.TemporaryDirectory() as temp_dir_b:
-            distutils.dir_util.copy_tree(self.project_b_fixtures_root, temp_dir_b)
-
-            # We move to another directory with a different plugin "b", as if it were another
-            # separate project which is not installed ("local" usage of the plugin declared in
-            # the namespace).
-            with pushd(temp_dir_b):
-                available_plugins = set(discover_plugins())
-                self.assertSetEqual({"allennlp_plugins.b"}, available_plugins)
-
-                import_plugins()
-                subcommands_available = Subcommand.list_available()
-                self.assertIn("b", subcommands_available)
-
-    def test_local_and_two_global_namespace_plugins(self):
-        available_plugins = set(discover_plugins())
-        self.assertSetEqual(set(), available_plugins)
-
-        # We make plugins "a" and "c" available as packages, each from other directories,
-        # as if they were separate installed projects ("global" usage of the plugins).
-        # We move to another directory with a different plugin "b", as if it were another separate
-        # project which is not installed ("local" usage of the plugin declared in the namespace).
-        with pip_install(self.project_a_fixtures_root, "a"), pip_install(
-            self.project_c_fixtures_root, "c"
-        ), pushd(self.project_b_fixtures_root):
-            available_plugins = set(discover_plugins())
-            self.assertSetEqual(
-                {"allennlp_plugins.a", "allennlp_plugins.b", "allennlp_plugins.c"},
-                available_plugins,
-            )
-
-            import_plugins()
-            subcommands_available = Subcommand.list_available()
-            self.assertIn("a", subcommands_available)
-            self.assertIn("b", subcommands_available)
-            self.assertIn("c", subcommands_available)
-
-    def test_file_and_two_global_namespace_plugins(self):
-        available_plugins = set(discover_plugins())
-        self.assertSetEqual(set(), available_plugins)
-
-        # We make plugins "a" and "c" available as packages, each from other directories,
-        # as if they were separate installed projects ("global" usage of the plugins).
-        # We move to another directory with a different plugin "b", as if it were another separate
-        # project which is not installed ("local" usage of the plugin declared in a file).
-        with pip_install(self.project_a_fixtures_root, "a"), pip_install(
-            self.project_c_fixtures_root, "c"
-        ), pushd(self.project_d_fixtures_root):
-            available_plugins = set(discover_plugins())
-            self.assertSetEqual(
-                {"allennlp_plugins.a", "allennlp_plugins.c", "d"}, available_plugins
-            )
-
-            import_plugins()
-            subcommands_available = Subcommand.list_available()
-            self.assertIn("a", subcommands_available)
-            self.assertIn("c", subcommands_available)
-            self.assertIn("d", subcommands_available)
-
-    def test_reload_plugins_adds_new(self):
-        available_plugins = set(discover_plugins())
-        self.assertSetEqual(set(), available_plugins)
-
-        with pip_install(self.project_a_fixtures_root, "a"):
-            available_plugins = set(discover_plugins())
-            self.assertSetEqual({"allennlp_plugins.a"}, available_plugins)
-
-            import_plugins()
-            subcommands_available = Subcommand.list_available()
-            self.assertIn("a", subcommands_available)
-
-            with pip_install(self.project_c_fixtures_root, "c"):
-                available_plugins = set(discover_plugins())
-                self.assertSetEqual({"allennlp_plugins.a", "allennlp_plugins.c"}, available_plugins)
-
-                import_plugins()
-                subcommands_available = Subcommand.list_available()
-                self.assertIn("a", subcommands_available)
-                self.assertIn("c", subcommands_available)
-
-    @pytest.mark.skip("Plugin unloading is not supported.")
-    def test_unload_plugin(self):
-        available_plugins = set(discover_plugins())
-        self.assertSetEqual(set(), available_plugins)
-
-        with pip_install(self.project_a_fixtures_root, "a"):
-            available_plugins = set(discover_plugins())
-            self.assertSetEqual({"allennlp_plugins.a"}, available_plugins)
-
-            import_plugins()
-            subcommands_available = Subcommand.list_available()
-            self.assertIn("a", subcommands_available)
-
-        available_plugins = set(discover_plugins())
-        self.assertSetEqual(set(), available_plugins)
-
-        import_plugins()
-        subcommands_available = Subcommand.list_available()
-        self.assertNotIn("a", subcommands_available)
-
-    @pytest.mark.skip("Plugin unloading is not supported.")
-    def test_reload_plugins_removes_one_adds_one(self):
-        available_plugins = set(discover_plugins())
-        self.assertSetEqual(set(), available_plugins)
-
-        with pip_install(self.project_a_fixtures_root, "a"):
-            available_plugins = set(discover_plugins())
-            self.assertSetEqual({"allennlp_plugins.a"}, available_plugins)
-
-            import_plugins()
-            subcommands_available = Subcommand.list_available()
-            self.assertIn("a", subcommands_available)
-            self.assertNotIn("c", subcommands_available)
-
-        with pip_install(self.project_c_fixtures_root, "c"):
-            available_plugins = set(discover_plugins())
-            self.assertSetEqual({"allennlp_plugins.c"}, available_plugins)
-
-            import_plugins()
-            subcommands_available = Subcommand.list_available()
-            self.assertNotIn("a", subcommands_available)
-            self.assertIn("c", subcommands_available)