Skip to content

Plugin system implementation #11121

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

Closed
wants to merge 8 commits into from
Closed

Plugin system implementation #11121

wants to merge 8 commits into from

Conversation

FFY00
Copy link
Contributor

@FFY00 FFY00 commented Dec 31, 2021

This got a bit delayed with Christmas, and then I fell ill, but it is here at last.

The plugin system itself is fairly simple, it essentially consists of a few definitions in a new conda.plugins module.
Initially, I planned to only expose the plugin manager instance to the rest of the conda codebase via the context object, but that proved insufficient right on the first plugin implementation, as the argument parsing happens before the context object is created, and the context object creation is currently inherently dependent on the argument parsing. As such, the plugin manager is also exposed via a helper function in the context module. This might limit us in the future, but that could potentially be solved by refactoring the context object a bit.
Instead of exposing the plugin manager directly, I think we might benefit of having a wrapper around it that does all the data necessary gathering, but this is not clear to me at this point, hopefully it will become clearer as we implement more plugins and get an idea of their needs.

Overall, I think the code size is pretty small, which was my goal. It demonstrates how easily plugins can be implemented in the conda codebase.

Regarding the actual subcommand plugin implementation, I did explore improving things a little bit more in regards to the argparse API usage and how to plug this functionality into it, but ultimately settled on just setting epilog instead of overwriting print_help. Part of the reasoning was that jezdez expressed the wish to move away from argparse to a different module, like click, so spending more time trying to improve this design did not seem worth it. For the same reason, and backwards compatibility, I decided to give plugin users full control over the argument parsing, instead of locking them into a specific API, like integrating the hook directly into argparse

I still need to add the tests, but I think I am reasonably happy with the end result so far.

@FFY00 FFY00 requested review from jezdez and jaimergp December 31, 2021 17:59
@FFY00 FFY00 requested a review from a team as a code owner December 31, 2021 17:59
@anaconda-issue-bot anaconda-issue-bot added the cla-signed [bot] added once the contributor has signed the CLA label Dec 31, 2021
@FFY00 FFY00 mentioned this pull request Dec 31, 2021
8 tasks
@jezdez jezdez linked an issue Jan 7, 2022 that may be closed by this pull request
8 tasks
Copy link
Member

@jezdez jezdez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @FFY00, this is a great start, thank you! I've left a few notes, but nothing blocks this from my perspective.

@FFY00
Copy link
Contributor Author

FFY00 commented Jan 20, 2022

I have been having some issues with the tests, it seems things get cached somewhere 😕

@FFY00
Copy link
Contributor Author

FFY00 commented Jan 20, 2022

Seems like the argparse instance is cached 🤦

@FFY00 FFY00 force-pushed the plugins branch 5 times, most recently from 5c2b067 to 7c15834 Compare January 20, 2022 19:15
@FFY00 FFY00 changed the title First plugin work Plugin system implementation Feb 23, 2022
@FFY00
Copy link
Contributor Author

FFY00 commented Feb 28, 2022

The latest commit (416f348) implements virtual packages via the plugin system. Virtual packages defined by conda are implemented as core plugins, which exist under the conda.plugins.* namespace and are registered directly in conda.base.context.get_plugin_manager, instead of via entrypoints.

FFY00 and others added 5 commits March 21, 2022 18:40
Signed-off-by: Filipe Laíns <lains@riseup.net>
Signed-off-by: Filipe Laíns <lains@riseup.net>
Instatiating the parser is cheap and should only happen one time in
cases where performance matters anyway. Caching this makes the tests
more complex and, I believe, just adds unnecessary maintenance cost.

Signed-off-by: Filipe Laíns <lains@riseup.net>
Signed-off-by: Filipe Laíns <lains@riseup.net>
* docs: add plugin API

Signed-off-by: Filipe Laíns <lains@riseup.net>

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Update docs/source/plugin-api/index.rst

Co-authored-by: Katherine Kinnaman <kkinnaman@anaconda.com>

* Update docs/source/plugin-api/index.rst

* Update docs/source/plugin-api/index.rst

* Update docs/source/plugin-api/index.rst

* Update docs/source/plugin-api/index.rst

Co-authored-by: Katherine Kinnaman <kkinnaman@anaconda.com>

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Jannis Leidel <jannis@leidel.info>
Co-authored-by: Katherine Kinnaman <kkinnaman@anaconda.com>
Signed-off-by: Filipe Laíns <lains@riseup.net>
FFY00 added 3 commits March 21, 2022 18:54
Signed-off-by: Filipe Laíns <lains@riseup.net>
Signed-off-by: Filipe Laíns <lains@riseup.net>
Signed-off-by: Filipe Laíns <lains@riseup.net>
@jezdez jezdez self-assigned this Apr 4, 2022
@beeankha beeankha mentioned this pull request Apr 27, 2022
@jezdez
Copy link
Member

jezdez commented Sep 7, 2022

Closing, now that #11435 has been merged into the plugins feature branch.

@jezdez jezdez closed this Sep 7, 2022
@jezdez jezdez deleted the plugins branch September 12, 2022 20:25
@jezdez jezdez restored the plugins branch September 12, 2022 20:25
@github-actions github-actions bot added the locked [bot] locked due to inactivity label Sep 13, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 13, 2023
@kenodegard kenodegard deleted the plugins branch August 14, 2024 21:01
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
cla-signed [bot] added once the contributor has signed the CLA locked [bot] locked due to inactivity
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Design and prototype a plugin system
4 participants