-
Notifications
You must be signed in to change notification settings - Fork 2
[Plugin] Dynamic Plugin API #105
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
base: dev
Are you sure you want to change the base?
Conversation
@mgorny if you check out https://github.com/wheelnext/pep_xxx_wheel_variants/tree/interactive_plugins - you can test completely the concept with I still need to fix some unittests. I'll do that later :) |
@@ -46,19 +57,30 @@ def load_plugins(plugin_apis: list[str]) -> Generator[PluginType]: | |||
else: | |||
plugin_instance = plugin_callable | |||
|
|||
required_attributes = PluginType.__abstractmethods__ | |||
if isinstance(plugin_instance, PluginStaticType): |
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.
isinstance()
won't work here since the protocol allows optional methods (get_build_setup
). Also, I'm not sure if it's going to work with ModuleType
and so on.
"--plugin-api", | ||
action="append", | ||
help="Load specified plugin API", | ||
required=True, | ||
) | ||
|
||
parser.add_argument( |
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.
Just pass them via JSON. Then you won't need most of the boilerplate.
@classmethod | ||
def from_str(cls, input_str: str) -> Self: | ||
# Try matching the input string with the regex pattern | ||
match = VALIDATION_PROPERTY_REGEX.fullmatch(input_str.strip()) |
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.
The whole point of subprocess is to keep validation in the parent, and pass only validated data.
Caution
If you want to check this out, I advise using this branch: https://github.com/wheelnext/pep_xxx_wheel_variants/tree/interactive_plugins
Note
This PR is in draft, because I need to fix the unittests. However I tested the most basic use case and it seems to work reasonably well :)
This PR removes a significant portion of #99 in favor of a more flexible less "maintenance intensive" approach while allowing maximum freedom in plugin logic & sorting.
This PR introduces the concept of
PluginDynamicType
. The olderPluginType
becomesPluginStaticType
(fully backward compatible).PluginDynamicType
=> takes theVariantProperties
of the "candidate variants" in inputsdata types
:float
SemVer
CalVer
A :: B :: C
>A :: B :: D
.PluginStaticType
=> take no input and rely on purestring matching
to operateWhile
PluginDynamicType
is significantly more powerful, its implementation is significantly more delicate. And we should recommendplugin authors
to usePluginStaticType
interface as often as possible and rely only usePluginDynamicType
when absolutely necessary.Not only this PR enables to grasp fully the vast diversity of the variant ecosystem:
float/int comparison
(e.g. minimum RAM requirement)It does it in a significantly more flexible, less arbitrary, more cleanly and more efficiently than PR #99.
The latter statement can be appreciated with the "fairly reduced scope" of the PR compared to #99.