[core/plugins] Add PluginLoader, SubmitterProvider, and config.json name/version resolution#3171
[core/plugins] Add PluginLoader, SubmitterProvider, and config.json name/version resolution#3171gregoire-dl wants to merge 36 commits into
PluginLoader, SubmitterProvider, and config.json name/version resolution#3171Conversation
Break the monolithic `core/plugins.py` module into focused submodules under `core/plugins`: - `env.py`: ProcessEnvType, ProcessEnv, DirTreeProcessEnv, RezProcessEnv, and processEnvFactory (process-environment resolution for plugin execution) - `base.py`: Plugin, NodePlugin, NodePluginStatus, validateNodeDesc, formatNodeDescriptionErrorMessage - `manager.py`: NodePluginManager Update all call sites to import from the new submodules.
Change terminology for methods, function and variables. Update all call sites and documentation.
…eDescProvider.nodeDescClass`
…onErrorMessage` inside `NodeDescProvider`
…nLoader` Delete the `loadClasses` based discovery pipeline from `meshroom/core/__init__.py` (`add_to_path`, `loadClasses(Nodes|Submitters)`, `loadAllNodes`, `loadPluginFolder`, `registerSubmitter`, `loadAllSubmitters`, `initRezPlugins`), and rewrite `initNodes`/`initSubmitters`/`initPlugins` to go through `PluginManager.addPluginFromBuiltInFolder`/`addPluginFromPath`/`addPluginFromRez` instead. `PluginManager` now owns a `PluginLoader` and registers the providers a `Plugin` contains via `registerPluginProviders`, replacing the old `addPlugin`/`registerNode`/ `unregisterNode`. An invalid or name-colliding node/submitter provider is skipped without failing the whole plugin load. `Plugin` now carries its own `PluginType` and builds its own `ProcessEnv` (dirtree or rez) at construction. `NodeDescProvider`/`SubmitterProvider` gain `absolutePackage`/ `relativePackage` so a node's process environment (and Rez subrequires) can be resolved per sub-package within a plugin, not just per plugin. Port every test off the removed functions onto the new `PluginManager`/`PluginLoader` API, and flatten the test plugin fixtures from `tests/plugins/meshroom/<name>/` to `tests/plugins/<name>/meshroom/`, matching the new one-folder-per-plugin model.
This allows to have a proper name for each built-in plugin, the name of the subfolder of the given node folder.
| bool: True if the node descriptor provider has successfully been reloaded (i.e. there was | ||
| no error, and some changes were made since its last loading), False otherwise. | ||
| """ | ||
| timestamp = 0.0 |
There was a problem hiding this comment.
This PR should not modify this method code logic.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## develop #3171 +/- ##
===========================================
+ Coverage 86.07% 86.54% +0.46%
===========================================
Files 78 83 +5
Lines 12111 12400 +289
===========================================
+ Hits 10425 10731 +306
+ Misses 1686 1669 -17 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Code Review
This pull request refactors Meshroom's plugin loading and management system, replacing the old NodePlugin architecture with a more modular design centered around PluginManager, PluginLoader, NodeDescProvider, and SubmitterProvider. While this refactoring significantly improves plugin isolation and environment handling (including Rez and DirTree environments), several critical issues must be addressed. First, numerous invalid dictionary type hints use colons instead of commas (e.g., dict[str: str]), which will cause runtime errors. Second, splitting Rez package names with partition('-') will fail for package names containing hyphens; rpartition('-') should be used instead. Third, potential AttributeError and TypeError exceptions exist when reloading missing modules or accessing unassigned providers. Finally, file operations should explicitly specify utf-8 encoding to prevent Windows-specific decode errors, and environment path constructions must be guarded against trailing separators to avoid importing from the current working directory.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| self._env["PYTHONPATH"] = os.pathsep.join( | ||
| [f"{_MESHROOM_ROOT}"] + self.pythonPaths + [os.getenv('PYTHONPATH', '')]) |
There was a problem hiding this comment.
If os.getenv('PYTHONPATH') is empty or not set, appending "" to the path list and joining with os.pathsep will result in a trailing path separator. In Python/Linux, a trailing separator represents the current working directory (.), which can lead to unexpected module imports or security risks. It is safer to filter out empty paths.
| self._env["PYTHONPATH"] = os.pathsep.join( | |
| [f"{_MESHROOM_ROOT}"] + self.pythonPaths + [os.getenv('PYTHONPATH', '')]) | |
| pythonPaths = [f"{_MESHROOM_ROOT}"] + self.pythonPaths | |
| envPythonPath = os.getenv('PYTHONPATH') | |
| if envPythonPath: | |
| pythonPaths.append(envPythonPath) | |
| self._env["PYTHONPATH"] = os.pathsep.join(pythonPaths) |
| return False | ||
|
|
||
| try: | ||
| updated = importlib.reload(sys.modules.get(self.nodeDescClass.__module__)) |
There was a problem hiding this comment.
sys.modules.get(...) can return None if the module is not currently loaded or registered in sys.modules. Passing None to importlib.reload() will raise a TypeError. It is safer to guard against None before reloading.
| updated = importlib.reload(sys.modules.get(self.nodeDescClass.__module__)) | |
| module = sys.modules.get(self.nodeDescClass.__module__) | |
| if module is None: | |
| self.status = NodeDescProviderStatus.ERROR | |
| logging.error(f"[Reload] {self.name}: The module {self.nodeDescClass.__module__} was not found in sys.modules.") | |
| return False | |
| try: | |
| updated = importlib.reload(module) |
| def processChunk(self, chunk): | ||
| cmd = self.buildCommandLine(chunk) | ||
| runtimeEnv = chunk.node.nodeDesc.plugin.runtimeEnv | ||
| runtimeEnv = chunk.node.nodeDesc.provider.runtimeEnv |
There was a problem hiding this comment.
If provider is None (e.g., if the node descriptor is instantiated directly without being registered via a NodeDescProvider), accessing provider.runtimeEnv will raise an AttributeError. Adding a defensive check prevents potential crashes.
| runtimeEnv = chunk.node.nodeDesc.provider.runtimeEnv | |
| runtimeEnv = chunk.node.nodeDesc.provider.runtimeEnv if chunk.node.nodeDesc.provider else None |
| runtimeEnv = chunk.node.nodeDesc.plugin.runtimeEnv | ||
| cmdPrefix = chunk.node.nodeDesc.plugin.commandPrefix | ||
| cmdSuffix = chunk.node.nodeDesc.plugin.commandSuffix | ||
| runtimeEnv = chunk.node.nodeDesc.provider.runtimeEnv |
There was a problem hiding this comment.
If provider is None (e.g., if the node descriptor is instantiated directly without being registered via a NodeDescProvider), accessing provider.runtimeEnv will raise an AttributeError. Adding a defensive check prevents potential crashes.
| runtimeEnv = chunk.node.nodeDesc.provider.runtimeEnv | |
| runtimeEnv = chunk.node.nodeDesc.provider.runtimeEnv if chunk.node.nodeDesc.provider else None |
Description
Introduce a new
PluginLoaderand a redesignedPluginManager. Change previous in code definition of a plugin (e.g. a package / subfolder of ameshroomfolder). A plugin is now a folder with ameshroomsubfolder possibly containing multiple standalone node description modules, submitter modules, templates and a configuration file.Plugin:
Store for each plugin the name, version, type (
BUILTIN/PATH/REZ), templates, configuration and also the:NodeDescProviderobjects (node description class with validity status and loading errors)SubmitterProviderobjects (submitter instance with validity status and loading errors)PluginLoader:
Load/Unload plugin python modules / packages into a common virtual package following this pattern:
PluginNameis defined from the REZ package name, theconfig.jsonfile or in the last case the folder name of the plugin.This pattern helps to prevent collision in plugin modules and replaces the previous
Plugin.uid. The Loader ensures that only one plugin loaded withPluginNamecan be used (the first). All issues in the loading process are logged in a summary for each plugin loaded. Also, the loading mechanism has been updated to add support for new plugin layouts with standalone node description files and also submitter files inside the plugin.Plugins with standalone node files are now supported (no
__init__.py):Plugins without subfolders are now supported:
Plugins with node descriptions and/or submitters:
PluginManager:
Manage plugins and register node description classes and now submitter instances.
Following the previous implementation, this object centralizes all loaded plugins, available node descriptions and submitters.
The
PluginManagernow provides simple methods for online plugin loading:addPluginFromRezfor a plugin provided with REZaddPluginFromPathfor a plugin located at an arbitrary pathaddPluginFromBuiltInFolderfor a plugin folder using Meshroom environment (e.g.meshroom/nodes)Unit tests:
All unit tests have been updated to follow these changes.
Multiple have been added for the new
PluginLoader.Backward Compatibility:
Fully backward-compatible, previous plugin structures and configurations remain supported.
Implementation remarks
config.jsoncan now be written in two formats:[ { "key": "MY_VAR", "type": "string", "value": "myValue" }, { "key": "MY_PATH", "type": "path", "value": "relativeOrAbsolutePath" } ]name,version, andenvkeys,envusing the same entry format as above:{ "name": "customPlugin", "version": "1.0.0", "env": [ { "key": "MY_VAR", "type": "string", "value": "myValue" } ] }