Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement more modern plugin system #1228

Draft
wants to merge 152 commits into
base: develop
Choose a base branch
from

Conversation

A-UNDERSCORE-D
Copy link
Contributor

This is unfinished, here for comment only at the moment

plugin/decorators.py Outdated Show resolved Hide resolved
@Athanasius Athanasius added enhancement Plugins Anything related to plugins labels Aug 11, 2021
@A-UNDERSCORE-D
Copy link
Contributor Author

Do we want to, perhaps, use a plugin.json (or other format) file to indicate names and things?

The benefits would be:

  • Plugins can use names other than their directory names, and these names are used before loading completes, reducing confusion
  • Plugins can define versions of other plugins they are incompatible with, which would be difficult to do safely after loading
    • This would also include possibly load order?

Not sure if there are any others, those are the ones that came to mind

@A-UNDERSCORE-D A-UNDERSCORE-D force-pushed the feature/new-plugin-system branch 2 times, most recently from b76584a to 5cb9410 Compare October 5, 2021 14:26
@A-UNDERSCORE-D
Copy link
Contributor Author

This is starting to get to a point where it works and can be merged

For details see ARCHITECTURE.md
I've been writing too much go apparently. load_plugin() now raises
exceptions when things go wrong, rather than an opaque bool return.
To force plugin authors to implement load
Also clarified an exception
Split loading and unloading tests into their own files
Added a conftest file to setup globals between said two files
Added plugin unloading and tests for said plugin unloading

Made plugin.manager.PluginManager.load_plugin() return the loaded
plugin, if possible
@Athanasius
Copy link
Contributor

To ensure my "let's get this merged ASAP, but not as the default" ideas don't get lost ... first of all a copy/paste, but look past it to a more structured set of thoughts:

If you can make the majority of the code not touch the existing system, with it taking changing a minimum of plug.py to switch to the new system, then we could merge it and have it as an optional paradigm. Mostly in the interests of it being more testable and more likely to keep up with other code changes.

e.g. have the new system look in plugins_v2 instead of plugins, certainly for the third-party ones. Perhaps even for the core ones (but then some decision on which version of those to load).

Mostly I'd like us to get away from that branch/PR just languishing and becoming more and more daunting to rebase for a merge.

  • Complete a rebase of this branch to current develop.
  • Add a CL arg --plugins-primary-version=X. Should default to 1. Yes, we're going to call the current paradigm 1 even if, technically, there would have been past changes and we might be on, like 13 by now. The new scheme will be 2.
  • Now adjust the code so that:
    • By default it uses all of the old path. This is for both core and found (third-party) plugins.
    • But the new plugin code is invoked, but only for found plugins in a plugins_v2 directory, rather than the current plugins. The idea of this is to allow someone to test v2 for some plugins whilst having others default to v1. This might get tricky if a plugin (be it directory name or the plugin_start3() return string) is present in both. I'd probably have v1 run first and v2 check if v1 already has the plugin. Or, perhaps, this isn't even a concern.
    • The new path is only used for v1 plugins if --plugins-primary-version=2 is passed and applies to ALL (inc. 'core') plugins. This may well change the semantics of loading from plugins_v2, at least in the "plugin is in both" case.
    • Settings -> Plugins will require adjusting to reflect which plugin-system version a plugin is loaded under.

There's probably going to be some "the devil is in the details" on this, but hopefully that conveys the idea of "have v2 available. plugins_v2 to always use v2 for a plugin, CL arg to make it be used for all of it".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Plugins Anything related to plugins
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants