Register plugin from entry points #1872
Conversation
Signed-off-by: Nig3l <[email protected]>
Signed-off-by: Nig3l <[email protected]>
Signed-off-by: Nig3l <[email protected]>
Ni-g-3l
force-pushed
the
feat/register_plugin_from_entry_points
branch
from
8a16886 to
da9e66c
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1872 +/- ##
==========================================
- Coverage 59.27% 59.25% -0.03%
==========================================
Files 126 126
Lines 17217 17166 -51
Branches 3017 2754 -263
==========================================
- Hits 10206 10171 -35
+ Misses 6326 6310 -16
Partials 685 685 ☔ View full report in Codecov by Sentry. |
Signed-off-by: Nig3l <[email protected]>
There was a problem hiding this comment.
Thanks @Ni-g-3l for creating this PR. I left a couple of comments that will need to be addressed before we can consider merging it.
Also, if you have the time and want to, we should adjust our newly written plugins doc (https://rez.readthedocs.io/en/stable/plugins.html).
| if sys.version_info.minor >= 8: | ||
| from importlib.metadata import entry_points | ||
| else: | ||
| from importlib_metadata import entry_points |
There was a problem hiding this comment.
This will force us to vendor importlib-metadata (which is fine since it's an APACHE-2.0 licensed).
| plugin = plugin.load() | ||
| plugin_name = plugin.__name__.split('.')[-1] | ||
| plugin_path = os.path.dirname(plugin.__file__) | ||
| self.register_plugin_module(plugin_name, plugin, plugin_path) | ||
| self.load_config_from_plugin(plugin) |
There was a problem hiding this comment.
We should put this code under a try/except block and add logs similar to logs found in load_plugins_from_namespace.
| @@ -68,6 +69,13 @@ def test_new_loading_style(self): | |||
| "package_repository", "cloud") | |||
| self.assertEqual(cloud_cls.name(), "cloud") | |||
|
|
|||
| def test_load_plugin_from_entry_points(self): | |||
| """Test loading rez plugin from setuptools entry points""" | |||
| with restore_pip("baz", os.path.join(self.data_path("extensions"), "baz")): | |||
There was a problem hiding this comment.
It would be safer to either just install into a temporary directory, or just commit the dist-info folder in the repo to avoid having to install it.
There was a problem hiding this comment.
Why is the plugin defined twice?
| discovered_plugins = entry_points(group='rez.plugins') | ||
| for plugin in discovered_plugins: | ||
| plugin = plugin.load() | ||
| plugin_name = plugin.__name__.split('.')[-1] |
There was a problem hiding this comment.
I'm not sure to understand why we need to do this. Can't we just use plugin.name? For the old plugin style, we needed to do split('.')[-1] because we were getting the name from rezplugins.<type>.<name>. But with entrypoints, we don't need to split.
Hello
As suggest here I implemented the support of entrypoints in order to make the process of adding plugin to REZ easier.
Closes #1661
Feel free to add feedback !
Have a great day