Add router factory classes #89
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Closed
abkfenris
added a commit
to abkfenris/xpublish
that referenced
this pull request
Dec 11, 2022
Builds on top of @benbovy 's work in building router factories in xpublish-community#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
Closed
jhamman
pushed a commit
that referenced
this pull request
Feb 1, 2023
* 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>
|
Should this be closed now that #146 is in? |
|
I believe this functionality is taken care of by #146 and other plugin related work. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is very much inspired from titiler. It will provide more flexibility for e.g., choosing alternative default values for some parameters (e.g., projection or colormap for xyz/wms services #88).
One question: should we force users to subclass
XpublishFactoryor should we still allow them providing "raw" FastAPI routers to xpublish applications?Using
XpublishFactoryexclusively would allow us to get rid of FastAPI's dependencies overriding (it always seemed hacky to me in this context) and some ugly workarounds like 13bed4b.On the other hand, I like that users can simply create an
APIRouterwith a bunch of functions. Also no breaking change in this case.Thoughts @xarray-contrib/xpublish-devs?