From 89d61723de876c02b061198e9462feb7f52d98ba Mon Sep 17 00:00:00 2001 From: Jakub Trllo Date: Wed, 23 Mar 2022 19:20:37 +0100 Subject: [PATCH] applied comments --- openpype/lib/python_module_tools.py | 2 - openpype/pipeline/actions.py | 4 +- openpype/pipeline/create/creator_plugins.py | 4 +- openpype/pipeline/load/plugins.py | 2 +- openpype/pipeline/plugin_discover.py | 88 ++++++++++++++------- 5 files changed, 66 insertions(+), 34 deletions(-) diff --git a/openpype/lib/python_module_tools.py b/openpype/lib/python_module_tools.py index 4ef31b5579..6fad3b547f 100644 --- a/openpype/lib/python_module_tools.py +++ b/openpype/lib/python_module_tools.py @@ -133,8 +133,6 @@ def classes_from_module(superclass, module): if not inspect.isclass(obj) or obj is superclass: continue - # Use string comparison rather than `issubclass` - # in order to support reloading of this module. if issubclass(obj, superclass): classes.append(obj) diff --git a/openpype/pipeline/actions.py b/openpype/pipeline/actions.py index 6cb2e9a5a4..b488fe3e1f 100644 --- a/openpype/pipeline/actions.py +++ b/openpype/pipeline/actions.py @@ -97,7 +97,7 @@ class InventoryAction(object): # Launcher action def discover_launcher_actions(): - return discover(LauncherAction).plugins + return discover(LauncherAction) def register_launcher_action(plugin): @@ -110,7 +110,7 @@ def register_launcher_action_path(path): # Inventory action def discover_inventory_actions(): - actions = discover(InventoryAction).plugins + actions = discover(InventoryAction) filtered_actions = [] for action in actions: if action is not InventoryAction: diff --git a/openpype/pipeline/create/creator_plugins.py b/openpype/pipeline/create/creator_plugins.py index dbeeb94050..c3ba8b1d1c 100644 --- a/openpype/pipeline/create/creator_plugins.py +++ b/openpype/pipeline/create/creator_plugins.py @@ -303,11 +303,11 @@ class AutoCreator(BaseCreator): def discover_creator_plugins(): - return discover(BaseCreator).plugins + return discover(BaseCreator) def discover_legacy_creator_plugins(): - plugins = discover(LegacyCreator).plugins + plugins = discover(LegacyCreator) set_plugin_attributes_from_settings(plugins, LegacyCreator) return plugins diff --git a/openpype/pipeline/load/plugins.py b/openpype/pipeline/load/plugins.py index fb5d1df9b5..d60aed0083 100644 --- a/openpype/pipeline/load/plugins.py +++ b/openpype/pipeline/load/plugins.py @@ -110,7 +110,7 @@ class SubsetLoaderPlugin(LoaderPlugin): def discover_loader_plugins(): - plugins = discover(LoaderPlugin).plugins + plugins = discover(LoaderPlugin) set_plugin_attributes_from_settings(plugins, LoaderPlugin) return plugins diff --git a/openpype/pipeline/plugin_discover.py b/openpype/pipeline/plugin_discover.py index a657df7994..b5edda7e9d 100644 --- a/openpype/pipeline/plugin_discover.py +++ b/openpype/pipeline/plugin_discover.py @@ -2,25 +2,31 @@ import os import inspect import traceback +from openpype.lib import Logger from openpype.lib.python_module_tools import ( modules_from_path, classes_from_module, ) +log = Logger.get_logger(__name__) + class DiscoverResult: - """Hold result of publish plugins discovery. + """Result of Plug-ins discovery of a single superclass type. - Stores discovered plugins duplicated plugins and file paths which - crashed on execution of file. + Stores discovered, duplicated, ignored and abstract plugins and file paths + which crashed on execution of file. """ - def __init__(self): + def __init__(self, superclass): + self.superclass = superclass self.plugins = [] self.crashed_file_paths = {} self.duplicated_plugins = [] self.abstract_plugins = [] self.ignored_plugins = set() + # Store loaded modules to keep them in memory + self._modules = set() def __iter__(self): for plugin in self.plugins: @@ -32,6 +38,10 @@ class DiscoverResult: def __setitem__(self, item, value): self.plugins[item] = value + def add_module(self, module): + """Add dynamically loaded python module to keep it in memory.""" + self._modules.add(module) + def get_report(self, only_errors=True, exc_info=True, full_report=False): lines = [] if not only_errors: @@ -80,10 +90,10 @@ class DiscoverResult: return "\n".join(lines) - def print_report(self, only_errors=True, exc_info=True): + def log_report(self, only_errors=True, exc_info=True): report = self.get_report(only_errors, exc_info) if report: - print(report) + log.info(report) class PluginDiscoverContext(object): @@ -98,12 +108,25 @@ class PluginDiscoverContext(object): self._registered_plugins = {} self._registered_plugin_paths = {} self._last_discovered_plugins = {} + # Store the last result to memory + self._last_discovered_results = {} def get_last_discovered_plugins(self, superclass): + """Access last discovered plugin by a subperclass. + + Returns: + None: When superclass was not discovered yet. + list: Lastly discovered plugins of the superclass. + """ + return self._last_discovered_plugins.get(superclass) def discover( - self, superclass, allow_duplicates=True, ignore_classes=None + self, + superclass, + allow_duplicates=True, + ignore_classes=None, + return_report=False ): """Find and return subclasses of `superclass` @@ -122,7 +145,7 @@ class PluginDiscoverContext(object): if not ignore_classes: ignore_classes = [] - result = DiscoverResult() + result = DiscoverResult(superclass) plugin_names = set() registered_classes = self._registered_plugins.get(superclass) or [] registered_paths = self._registered_plugin_paths.get(superclass) or [] @@ -151,6 +174,7 @@ class PluginDiscoverContext(object): for item in modules: filepath, module = item + result.add_module(module) for cls in classes_from_module(superclass, module): if cls is superclass or cls in ignore_classes: result.ignored_plugins.add(cls) @@ -169,14 +193,18 @@ class PluginDiscoverContext(object): result.plugins.append(cls) + # Store in memory last result to keep in memory loaded modules + self._last_discovered_results[superclass] = result self._last_discovered_plugins[superclass] = list( result.plugins ) - result.print_report() - return result + result.log_report() + if return_report: + return result + return result.plugins def register_plugin(self, superclass, cls): - """Register an individual `obj` of type `superclass` + """Register a directory containing plug-ins of type `superclass` Arguments: superclass (type): Superclass of plug-in @@ -190,12 +218,13 @@ class PluginDiscoverContext(object): self._registered_plugins[superclass].append(cls) def register_plugin_path(self, superclass, path): - """Register a directory containing plug-ins of type `superclass` + """Register a directory of one or more plug-ins Arguments: - superclass (type): Superclass of plug-ins to look for during discovery - path (str): Absolute path to directory in which to discover plug-ins - + superclass (type): Superclass of plug-ins to look for during + discovery + path (str): Absolute path to directory in which to discover + plug-ins """ if superclass not in self._registered_plugin_paths: @@ -207,24 +236,29 @@ class PluginDiscoverContext(object): def registered_plugin_paths(self): """Return all currently registered plug-in paths""" - # Prohibit editing in-place - duplicate = { + # Return shallow copy so we the original data can't be changed + return { superclass: paths[:] for superclass, paths in self._registered_plugin_paths.items() } - return duplicate def deregister_plugin(self, superclass, plugin): - """Oppsite of `register_plugin()`""" + """Opposite of `register_plugin()`""" if superclass in self._registered_plugins: self._registered_plugins[superclass].remove(plugin) def deregister_plugin_path(self, superclass, path): - """Oppsite of `register_plugin_path()`""" + """Opposite of `register_plugin_path()`""" self._registered_plugin_paths[superclass].remove(path) -class GlobalDiscover: +class _GlobalDiscover: + """Access to global object of PluginDiscoverContext. + + Using singleton object to register/deregister plugins and plugin paths + and then discover them by superclass. + """ + _context = None @classmethod @@ -235,30 +269,30 @@ class GlobalDiscover: def discover(superclass, allow_duplicates=True): - context = GlobalDiscover.get_context() + context = _GlobalDiscover.get_context() return context.discover(superclass, allow_duplicates) def get_last_discovered_plugins(superclass): - context = GlobalDiscover.get_context() + context = _GlobalDiscover.get_context() return context.get_last_discovered_plugins(superclass) def register_plugin(superclass, cls): - context = GlobalDiscover.get_context() + context = _GlobalDiscover.get_context() context.register_plugin(superclass, cls) def register_plugin_path(superclass, path): - context = GlobalDiscover.get_context() + context = _GlobalDiscover.get_context() context.register_plugin_path(superclass, path) def deregister_plugin(superclass, cls): - context = GlobalDiscover.get_context() + context = _GlobalDiscover.get_context() context.deregister_plugin(superclass, cls) def deregister_plugin_path(superclass, path): - context = GlobalDiscover.get_context() + context = _GlobalDiscover.get_context() context.deregister_plugin_path(superclass, path)