Use pluggy for plugin management (#146)

* add XpublishFactory base class

* use factories for base and zarr routers

* tests: drop py36 support, add py39

* Add entry point based plugins

Builds on top of @benbovy 's work in building router factories in #89 to build a plugin system.

The plugin system uses entry points, which are most commonly used for console or GUI scripts. The entry_point group is `xpublish.plugin` Right now plugins can provide dataset specific and general (app) routes, with default prefixes and tags for both.

Xpublish will by default load plugins via the entry point. Additionally, plugins can also be loaded directly via the init, as well as being disabled, or configured. The existing dataset router pattern also still works, so that folks aren't forced into using plugins

Entry point reference:
- https://setuptools.pypa.io/en/latest/userguide/entry_point.html
- https://packaging.python.org/en/latest/specifications/entry-points/
- https://amir.rachum.com/amp/blog/2017/07/28/python-entry-points.html

* Test plugin system against existing test suite

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Clean up unused imports

* Extendable plugins

Another variation on #140 with a few of the ideas from the discussion there and #139.

Plugin routers are now nested under a parent `Plugin` class which now acts as a way to combine multiple related pieces of functionality together (say db management routes and a CLI). This allows new plugin functionality to be added in other plugins or Xpublish related libraries without requiring the parent `Plugin` class to define everything.

Plugins are loaded from the `xpublish.plugin` entrypoint group. Plugins can be manually configured via the `plugins` argument to `xpublish.Rest`. The specifics of plugin loading can be changed by overriding the `.setup_plugins()` method.

Some other `xpublish.Rest` functionality has been refactored out into separate methods to allow easier overriding for instance making a `SingleDatasetRest` class that will allow simplifying `xpublish.Rest`.

The `ds.rest` accessor has been move out into it's own file.

* Use `typing.Dict` for Python 3.8 compatibility

* More typing fixes for 3.8

* Refactor single dataset support into it's own class

* Use pluggy for plugin management

Pluggy is the core of py.test's ability to be extended, and it also is the plugin manager for Tox, Datasette, Jupyter-FPS, and Conda, among others.

In Xpublish a set of hooks is defined that plugins can implement, and a `pluggy.PluginManager` (as `xpublish.Rest.pm`) proxies requests to the plugins that have implemented the hooks and aggregates the results. Hooks define a set of possible kwargs that can be passed to downstream implementations, but not all implementations need to implement all of them (which makes it easy to add new kwargs without disrupting existing implementations). Hooks can also be defined as only returning the first response, or wrapping other hooks (dataset middleware?).

So far I've defined a handful of hooks in `xpublish.plugin.hooks:PluginSpec`:
- app_router()
- dataset_router()
- get_datasets()
- get_dataset(dataset_id: str)
- register_hookspec() - Which allows plugins to register new hook types

`get_datasets` and `get_dataset` allow plugins to provide datasets without loading them on launch, or overriding  `Rest._get_dataset_fn` or `Rest.setup_datasets()`.

I've kept the kwargs relatively minimal right now on the hooks as it's easier to expand the kwargs later, than it is to reduce them.

I've additionally refactored the single dataset usage into it's own class `xpublish.SingleDatasetRest` to simplify some of the conditional logic which the accessor uses.

Pluggy references:
- https://pluggy.readthedocs.io/en/stable/
- https://docs.pytest.org/en/latest/how-to/writing_plugins.html
- https://docs.datasette.io/en/latest/writing_plugins.html
- https://docs.conda.io/projects/conda/en/latest/dev-guide/plugins/index.html#

* Move included plugins to plugins.included

* Remove commented code, clarify a few methods

* Allow late registered plugins to add new hooks and routers

* Refactor dependency injection into plugin routers

Refactored dependency injection into plugin routers so that dependencies can be overridden when routers are called, rather than when plugins are instantiated. This makes it so that routers can be reused and adapted by other plugins.

* Clean up and tighten plugin typing

* Test plugins and plugin management

---------

Co-authored-by: Benoit Bovy <[email protected]>
Co-authored-by: Joe Hamman <[email protected]>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

requirements.txt

@@ -3,6 +3,7 @@ dask
 fastapi>=0.59
 numcodecs
 numpy>=1.17
+pluggy
 toolz
 uvicorn
 xarray>=0.16

setup.cfg

@@ -9,7 +9,7 @@ select = B,C,E,F,W,T4,B9
 
 [isort]
 known_first_party=xpublish
-known_third_party=cachey,dask,fastapi,numcodecs,numpy,pandas,pkg_resources,pytest,setuptools,sphinx_autosummary_accessors,starlette,uvicorn,xarray,zarr
+known_third_party=cachey,dask,fastapi,numcodecs,numpy,pandas,pkg_resources,pluggy,pydantic,pytest,setuptools,sphinx_autosummary_accessors,starlette,uvicorn,xarray,zarr
 multi_line_output=3
 include_trailing_comma=True
 force_grid_wrap=0

setup.py

@@ -46,4 +46,12 @@
     keywords=['xarray', 'zarr', 'api'],
     use_scm_version={'version_scheme': 'post-release', 'local_scheme': 'dirty-tag'},
     setup_requires=['setuptools_scm>=3.4', 'setuptools>=42'],
+    entry_points={
+        'xpublish.plugin': [
+            'info = xpublish.plugins.included.dataset_info:DatasetInfoPlugin',
+            'zarr = xpublish.plugins.included.zarr:ZarrPlugin',
+            'module_version = xpublish.plugins.included.module_version:ModuleVersionPlugin',
+            'plugin_info = xpublish.plugins.included.plugin_info:PluginInfoPlugin',
+        ]
+    },
 )

tests/test_fsspec_compat.py

@@ -2,20 +2,20 @@
 
 import pytest
 
-from xpublish import Rest
+from xpublish import SingleDatasetRest
 from xpublish.utils.zarr import create_zmetadata, jsonify_zmetadata
 
 from .utils import TestMapper
 
 
 def test_get_zmetadata_key(airtemp_ds):
-    mapper = TestMapper(Rest(airtemp_ds).app)
+    mapper = TestMapper(SingleDatasetRest(airtemp_ds).app)
     actual = json.loads(mapper['.zmetadata'].decode())
     expected = jsonify_zmetadata(airtemp_ds, create_zmetadata(airtemp_ds))
     assert actual == expected
 
 
 def test_missing_key_raises_keyerror(airtemp_ds):
-    mapper = TestMapper(Rest(airtemp_ds).app)
+    mapper = TestMapper(SingleDatasetRest(airtemp_ds).app)
     with pytest.raises(KeyError):
         _ = mapper['notakey']

tests/test_plugin_management.py

@@ -0,0 +1,35 @@
+from starlette.testclient import TestClient
+
+from xpublish import Rest
+from xpublish.plugins import manage
+
+
+def test_exclude_plugins():
+    found_plugins = manage.find_default_plugins(exclude_plugins=['zarr'])
+
+    assert 'zarr' not in found_plugins
+    assert 'info' in found_plugins
+
+
+def test_configure_plugins(airtemp_ds):
+    info_prefix = '/meta'
+    zarr_prefix = '/zarr'
+    config = {
+        'info': {'dataset_router_prefix': info_prefix},
+        'zarr': {'dataset_router_prefix': zarr_prefix},
+    }
+    found_plugins = manage.find_default_plugins()
+
+    configured_plugins = manage.configure_plugins(found_plugins, config)
+
+    assert configured_plugins['info'].dataset_router_prefix == info_prefix
+    assert configured_plugins['zarr'].dataset_router_prefix == zarr_prefix
+
+    rest = Rest({'airtemp': airtemp_ds}, plugins=configured_plugins)
+    app = rest.app
+    client = TestClient(app)
+
+    info_response = client.get('/datasets/airtemp/meta/info')
+    json_response = info_response.json()
+    assert json_response['dimensions'] == airtemp_ds.dims
+    assert list(json_response['variables'].keys()) == list(airtemp_ds.variables.keys())

tests/test_rest_api.py

@@ -5,7 +5,7 @@
 from starlette.testclient import TestClient
 
 import xpublish  # noqa: F401
-from xpublish import Rest
+from xpublish import Plugin, Rest, SingleDatasetRest, hookimpl, hookspec
 from xpublish.dependencies import get_dataset
 from xpublish.utils.zarr import create_zmetadata, jsonify_zmetadata
 
@@ -20,7 +20,7 @@ def airtemp_rest(airtemp_ds):
         docs_url='/data-docs',
     )
 
-    return Rest(airtemp_ds, app_kws=app_kws)
+    return SingleDatasetRest(airtemp_ds, app_kws=app_kws)
 
 
 @pytest.fixture(scope='function')
@@ -59,6 +59,52 @@ def get_dims(dataset: xr.Dataset = Depends(get_dataset)):
     return router
 
 
+@pytest.fixture(scope='function')
+def dataset_plugin(airtemp_ds):
+    class AirtempPlugin(Plugin):
+        name = 'airtemp'
+
+        @hookimpl
+        def get_dataset(self, dataset_id: str):
+            if dataset_id == 'airtemp':
+                return airtemp_ds
+
+        @hookimpl
+        def get_datasets(self):
+            return ['airtemp']
+
+    return AirtempPlugin()
+
+
+@pytest.fixture(scope='function')
+def hook_spec_plugin():
+    class TestHookSpec:
+        @hookspec(firstresult=True)
+        def hello(self):
+            pass
+
+    class HookSpecPlugin(Plugin):
+        name = 'hook_spec'
+
+        @hookimpl
+        def register_hookspec(self):
+            return TestHookSpec
+
+    return HookSpecPlugin()
+
+
+@pytest.fixture(scope='function')
+def hook_implementation_plugin():
+    class HookImplementationPlugin(Plugin):
+        name = 'hook_implementation'
+
+        @hookimpl
+        def hello(self):
+            return 'world'
+
+    return HookImplementationPlugin()
+
+
 def test_init_cache_kws(airtemp_ds):
     rest = Rest(airtemp_ds, cache_kws={'available_bytes': 999})
     assert rest.cache.available_bytes == 999
@@ -102,7 +148,7 @@ def test_custom_app_routers(airtemp_ds, dims_router, router_kws, path):
     else:
         routers = [(dims_router, router_kws)]
 
-    rest = Rest(airtemp_ds, routers=routers)
+    rest = SingleDatasetRest(airtemp_ds, routers=routers, plugins={})
     client = TestClient(rest.app)
 
     response = client.get(path)
@@ -136,6 +182,47 @@ def func2():
         Rest(airtemp_ds, routers=[(router1, {'prefix': '/same'}), router2])
 
 
+def test_custom_dataset_plugin(airtemp_ds, dataset_plugin):
+    rest = Rest({})
+    rest.register_plugin(dataset_plugin)
+
+    client = TestClient(rest.app)
+
+    datasets_response = client.get('/datasets')
+    assert 'airtemp' in datasets_response.json()
+
+    info_response = client.get('/datasets/airtemp/info')
+    json_response = info_response.json()
+    assert json_response['dimensions'] == airtemp_ds.dims
+    assert list(json_response['variables'].keys()) == list(airtemp_ds.variables.keys())
+
+
+def test_custom_plugin_hooks_register(hook_spec_plugin, hook_implementation_plugin):
+    rest = Rest({})
+    rest.register_plugin(hook_implementation_plugin)
+    rest.register_plugin(hook_spec_plugin)
+
+    assert 'world' == rest.pm.hook.hello()
+
+
+def test_custom_plugin_hooks_init(hook_spec_plugin, hook_implementation_plugin):
+    rest = Rest(
+        {},
+        plugins={'hook_implementation': hook_implementation_plugin, 'hook_spec': hook_spec_plugin},
+    )
+
+    assert 'world' == rest.pm.hook.hello()
+
+
+def test_custom_plugin_not_initialized():
+    class TestPlugin(Plugin):
+        pass
+
+    rest = Rest({})
+    with pytest.raises(AttributeError):
+        rest.register_plugin(TestPlugin)
+
+
 def test_keys(airtemp_ds, airtemp_app_client):
     response = airtemp_app_client.get('/keys')
     assert response.status_code == 200
@@ -165,6 +252,27 @@ def test_versions(airtemp_app_client):
     assert response.json()['xarray'] == xr.__version__
 
 
+def test_plugin_versions(airtemp_app_client):
+    response = airtemp_app_client.get('/plugins')
+    assert response.status_code == 200
+
+    plugins = response.json()
+
+    assert plugins['info']['version'] == xpublish.__version__
+
+
+def test_plugins_loaded(airtemp_app_client):
+    response = airtemp_app_client.get('/plugins')
+    assert response.status_code == 200
+
+    plugins = response.json()
+
+    assert 'info' in plugins
+    assert 'module_version' in plugins
+    assert 'plugin_info' in plugins
+    assert 'zarr' in plugins
+
+
 def test_repr(airtemp_ds, airtemp_app_client):
     response = airtemp_app_client.get('/')
     assert response.status_code == 200
@@ -209,7 +317,7 @@ def test_array_group_raises_404(airtemp_app_client):
 
 
 def test_cache(airtemp_ds):
-    rest = Rest(airtemp_ds, cache_kws={'available_bytes': 1e9})
+    rest = SingleDatasetRest(airtemp_ds, cache_kws={'available_bytes': 1e9})
     assert rest.cache.available_bytes == 1e9
 
     client = TestClient(rest.app)
@@ -246,6 +354,13 @@ def test_rest_accessor_kws(airtemp_ds):
     assert response.status_code == 200
 
 
+def test_rest_accessor_single_dataset(airtemp_ds):
+    client = TestClient(airtemp_ds.rest.app)
+
+    response = client.get('/datasets')
+    assert response.status_code == 404
+
+
 def test_ds_dict_keys(ds_dict, ds_dict_app_client):
     response = ds_dict_app_client.get('/datasets')
     assert response.status_code == 200
@@ -273,12 +388,18 @@ def test_ds_dict_cache(ds_dict):
 def test_single_dataset_openapi_override(airtemp_rest):
     openapi_schema = airtemp_rest.app.openapi()
 
-    # "dataset_id" parameter should be absent in all paths
-    assert len(openapi_schema['paths']['/']['get']['parameters']) == 0
-
-    # test cached value
-    openapi_schema = airtemp_rest.app.openapi()
-    assert len(openapi_schema['paths']['/']['get']['parameters']) == 0
+    with pytest.raises(KeyError):
+        # "dataset_id" parameter should be absent in all paths
+        # parameters is no longer generated when plugins use passed in deps
+        #  and get_dataset is replaced
+        assert len(openapi_schema['paths']['/']['get']['parameters']) == 0
+
+    with pytest.raises(KeyError):
+        # test cached value
+        # parameters is no longer generated when plugins use passed in deps
+        #  and get_dataset is replaced
+        openapi_schema = airtemp_rest.app.openapi()
+        assert len(openapi_schema['paths']['/']['get']['parameters']) == 0
 
 
 def test_serve(airtemp_rest, mocker):

tests/test_zarr_compat.py

@@ -3,7 +3,7 @@
 import pytest
 import xarray as xr
 
-from xpublish import Rest
+from xpublish import SingleDatasetRest
 
 from .utils import TestMapper, create_dataset
 
@@ -32,7 +32,7 @@ def test_zmetadata_identical(start, end, freq, nlats, nlons, var_const, calendar
     ds = ds.chunk(ds.dims)
     zarr_dict = {}
     ds.to_zarr(zarr_dict, consolidated=True)
-    mapper = TestMapper(Rest(ds).app)
+    mapper = TestMapper(SingleDatasetRest(ds).app)
     actual = json.loads(mapper['.zmetadata'].decode())
     expected = json.loads(zarr_dict['.zmetadata'].decode())
     assert actual == expected
@@ -60,7 +60,7 @@ def test_roundtrip(start, end, freq, nlats, nlons, var_const, calendar, use_cfti
     )
     ds = ds.chunk(ds.dims)
 
-    mapper = TestMapper(Rest(ds).app)
+    mapper = TestMapper(SingleDatasetRest(ds).app)
     actual = xr.open_zarr(mapper, consolidated=True)
 
     xr.testing.assert_identical(actual, ds)
@@ -156,7 +156,7 @@ def test_roundtrip_custom_chunks(
         decode_times=decode_times,
     )
     ds = ds.chunk(chunks)
-    mapper = TestMapper(Rest(ds).app)
+    mapper = TestMapper(SingleDatasetRest(ds).app)
     actual = xr.open_zarr(mapper, consolidated=True, decode_times=decode_times)
 
     xr.testing.assert_identical(actual, ds)

xpublish/__init__.py

@@ -1,6 +1,8 @@
 from pkg_resources import DistributionNotFound, get_distribution
 
-from .rest import Rest, RestAccessor  # noqa: F401
+from .accessor import RestAccessor  # noqa: F401
+from .plugins import Dependencies, Plugin, hookimpl, hookspec  # noqa: F401
+from .rest import Rest, SingleDatasetRest  # noqa: F401
 
 try:
     __version__ = get_distribution(__name__).version