Pre and post loader hooks #1222
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -1,5 +1,7 @@ | ||||||
| from __future__ import annotations | ||||||
| import os | ||||||
kalisp marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
| import logging | ||||||
| from typing import ClassVar | ||||||
|
|
||||||
| from ayon_core.settings import get_project_settings | ||||||
| from ayon_core.pipeline.plugin_discover import ( | ||||||
|
|
@@ -251,6 +253,51 @@ class ProductLoaderPlugin(LoaderPlugin): | |||||
| """ | ||||||
|
|
||||||
|
|
||||||
| class PreLoaderHookPlugin: | ||||||
|
There was a problem hiding this comment. I wonder if it'd make more sense to make a single That way, one could make a Plugin that could act like a context manager by storing attributes on the plugin itself, e.g. def pre_process(self):
self.handle = do_whatever()
def post_process(self):
cleanup(self.handle)Thoughts? There was a problem hiding this comment. Also, how should the hooks behave if the loading failed? Should post hooks still trigger, or should those not? There was a problem hiding this comment. Thats probably question of who is making pre/post hook. If they want to loading to fail/be skipped, they need to raise an exception. There was a problem hiding this comment. Yes, I agree with the error handling that it is up to the developer . Loader itself should be mostly decoupled from the logic in per-load hook (otherwiser you could put everytthing to the loader). Bur the context manager idea is interesting, maybe something for the future when we rewrite the loading API. There was a problem hiding this comment. If we will really do this, then I agree with @BigRoy, we should have one hook being able to handle pre and post (less classes, less functions). And I think it should have There was a problem hiding this comment. It should be also removed from |
||||||
| """Plugin that should be run before any Loaders in 'loaders' | ||||||
|
|
||||||
| Should be used as non-invasive method to enrich core loading process. | ||||||
| Any external studio might want to modify loaded data before or after | ||||||
|
There was a problem hiding this comment.
Suggested change
I'd say almost everyone is "external studio" as there is no internal one 😜 |
||||||
| they are loaded without need to override existing core plugins. | ||||||
| """ | ||||||
| loader_identifiers: ClassVar[set[str]] | ||||||
|
|
||||||
| def process(self, context, name=None, namespace=None, options=None): | ||||||
| pass | ||||||
|
|
||||||
| def update(self, container, context): | ||||||
| pass | ||||||
|
|
||||||
| def switch(self, container, context): | ||||||
| pass | ||||||
|
|
||||||
|
|
||||||
| class PostLoaderHookPlugin: | ||||||
kalisp marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
| """Plugin that should be run after any Loaders in 'loaders' | ||||||
|
|
||||||
| Should be used as non-invasive method to enrich core loading process. | ||||||
| Any external studio might want to modify loaded data before or after | ||||||
| they are loaded without need to override existing core plugins. | ||||||
| """ | ||||||
| loader_identifiers: ClassVar[set[str]] | ||||||
|
|
||||||
| def process( | ||||||
| self, | ||||||
| container, | ||||||
| context, | ||||||
| name=None, | ||||||
| namespace=None, | ||||||
| options=None | ||||||
| ): | ||||||
| pass | ||||||
|
|
||||||
| def update(self, container, context): | ||||||
| pass | ||||||
|
|
||||||
| def switch(self, container, context): | ||||||
| pass | ||||||
|
|
||||||
|
|
||||||
| def discover_loader_plugins(project_name=None): | ||||||
| from ayon_core.lib import Logger | ||||||
| from ayon_core.pipeline import get_current_project_name | ||||||
|
|
@@ -287,3 +334,35 @@ def deregister_loader_plugin_path(path): | |||||
|
|
||||||
| def register_loader_plugin_path(path): | ||||||
| return register_plugin_path(LoaderPlugin, path) | ||||||
|
|
||||||
|
|
||||||
| def register_loader_pre_hook_plugin(plugin): | ||||||
| return register_plugin(PreLoaderHookPlugin, plugin) | ||||||
|
|
||||||
|
|
||||||
| def deregister_loader_pre_hook_plugin(plugin): | ||||||
| deregister_plugin(PreLoaderHookPlugin, plugin) | ||||||
|
|
||||||
|
|
||||||
| def register_loader_pre_hook_plugin_path(path): | ||||||
| return register_plugin_path(PreLoaderHookPlugin, path) | ||||||
|
|
||||||
|
|
||||||
| def deregister_loader_pre_hook_plugin_path(path): | ||||||
| deregister_plugin_path(PreLoaderHookPlugin, path) | ||||||
|
|
||||||
|
|
||||||
| def register_loader_post_hook_plugin(plugin): | ||||||
| return register_plugin(PostLoaderHookPlugin, plugin) | ||||||
|
|
||||||
|
|
||||||
| def deregister_loader_post_hook_plugin(plugin): | ||||||
| deregister_plugin(PostLoaderHookPlugin, plugin) | ||||||
|
|
||||||
|
|
||||||
| def register_loader_post_hook_plugin_path(path): | ||||||
| return register_plugin_path(PostLoaderHookPlugin, path) | ||||||
|
|
||||||
|
|
||||||
| def deregister_loader_post_hook_plugin_path(path): | ||||||
| deregister_plugin_path(PostLoaderHookPlugin, path) | ||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I vote for using type hints in methods arguments and return values - since annotations from
__future__is imported. If you use mypy later on, it will really help.