-
Notifications
You must be signed in to change notification settings - Fork 24
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
CEP 4 - Plugins Mechanism Implementation #32
Conversation
30422b2
to
c228569
Compare
cep-plugin-infrastructure.md
Outdated
|
||
### Hook | ||
|
||
Below is an example of a very basic plugin "hook": |
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.
Do we want to define different types of hooks in this CEP? For example, I did some quick brainstorming was able to come up with the following:
Hook | Description |
---|---|
Sub-command hook | Allows plugin authors to create an entirely new sub-command as is currently described in this open pull-request: conda/conda#11435 |
Exception handling hook | Allows a plugin author to hook in to the way that exceptions are handled and open up the possibility to add telemetry for exception logging (e.g. via services like Sentry) |
HTTP/CondaSession hook | Allows a plugin author to completely override the way that HTTP requests are handled in conda . This opens up the possibility for more complex authentication mechanisms that we currently cannot support (e.g. OAuth workflows). This too could be used for telemetry for logging. |
Solver hook | Allows plugin authors to either modify or completely override the solver used in conda . |
pre/post command hook | Allows plugin authors to run something either before or after a list of specified commands (i.e. arbitrary command decorators) |
I would like to see us detail as many of these hooks as possible in this CEP and leave the door open for defining new hooks when we need.
If we do end up just leaving the sub-command hook here as the only hook, could we at least refer to it as the "Sub-Command Hook" and announce that there will be more hooks added as necessary?
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.
I think we'd want to include the ones that make sense but avoid trying to create an exhaustive list. Somewhat of YAGNI at play, we could try to spend too much time thinking of future scenarios that might not come up. We know we won't be able to capture everything so I'd suggest we outline some pragmatic choices with perhaps details on how to document new hooks in the future.
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.
I am also skeptical about adding a lot of hooks at once. Each hook is an open door we might not be able to close in the future, and I would think very carefully about adding hooks (and where exactly in the codebase).
could we at least refer to it as the "Sub-Command Hook" and announce that there will be more hooks added as necessary?
This approach sounds perfect to me!
Some things we could consider:
|
@conda-incubator/steering This PR falls under the Enhancement Proposal Approval policy of the conda governance policy, please vote and/or comment on this PR. This PR needs 60% of the Steering Council to vote yea to pass. To vote please leave Approve (yea) or Request Changes (nay) pull request reviews. If you would like changes to the current language please leave a comment (in the PR) or push to this branch. This vote will end on 2022-08-12. |
As I mentioned before, I have a lot of interest in this topic and it would be really nice to have several pre and pos hooks for conda actions, not just the solver |
cep-4.md
Outdated
_setup.py_ | ||
```python | ||
import setuptools | ||
|
||
if __name__ == "__main__": | ||
setuptools.setup() | ||
``` |
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.
setup.py
is actually optional with setuptools now if you define pyproject.toml
.
_pyproject.toml_ | ||
```toml | ||
[build-system] | ||
requires = ["setuptools", "setuptools-scm"] |
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.
Based on my personal experience, I would avoid explicitly defining a full example of how to package a project like this and instead focus on how to specify an entry point. Otherwise I could see people copy-and-pasting either example for all their plugins as the way to package conda plugins.
I would also list the pyproject.toml
example first as that's the standard.
cep-4.md
Outdated
dependencies = ["conda"] | ||
|
||
[project.entry-points."conda"] | ||
my-conda-plugin = "my_plugin.module:function" |
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.
Is there a specific reason this is different than the setup.py
example? That one points at a module while this points as a function in a submodule.
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.
That was my mistake! I've updated it to be more consistent with the other example, thank you for pointing this out.
@conda-incubator/steering This vote falls under the Enhancement Proposal Approval policy of the conda governance policy, please vote and/or comment on this PR. This vote needs 60% of the Steering Council to vote yea to pass. This vote presently has 6, and needs 4 more for quorum. It is proposed that this vote will time out and be evaluated with the current votes in 7 days, on 2022-08-19. To vote please leave Approve (yea) or Request Changes (nay) reviews. |
01139da
to
5aac33f
Compare
- pyproject.toml needs to be listed first, as it is the standard - fix inconsistency in the project.entry-points section of pyproject.toml - remove setup.py file for pyproject.toml example
5aac33f
to
b3cba41
Compare
40700ae
to
aeb0a1d
Compare
Voting ResultsThis was a standard, non-timed-out vote.
This vote has reached quorum (10 + 0 = 10 which is at least 60% of 16). It has also passed since it recorded 10 "yes" votes and 0 "no" votes giving 10/10 which is greater than 60% of 15. It should be noted that a request for change was recorded in the pull request about minor implementation details that do not invalidate the previous votes. The author made the requested change. |
This CEP is for the implementation of a new plugins mechanism for the
conda
codebase. It is about the initial mechanism of plugins, versus for any specific new plugins (those will be proposed and discussed in their own separate CEPs after the implementation of plugins has been completed).This pull request is intended as a public forum for discussing the plugins feature proposal.