-
Notifications
You must be signed in to change notification settings - Fork 40
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
[experimental] UV support for Python plugins #825
Conversation
lengau
commented
Aug 21, 2024
- Have you signed the CLA?
3138ed5
to
4e6a806
Compare
self, *, properties: PluginProperties, part_info: infos.PartInfo | ||
) -> None: | ||
super().__init__(properties=properties, part_info=part_info) | ||
use_uv_attr_name = f"{properties.plugin}_use_uv" |
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.
Having a parameter like <plugin>-package-manager: uv
(defaulting to pip
) could allow us to use other package managers in the future (and avoid using a boolean property). There's also the option to have use-uv
in build-attributes
but that would not tie to Python-based plugins specifically.
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 like that idea!
def _get_find_python_interpreter_commands(self) -> list[str]: | ||
"""Get the commands that find a staged Python interpreter. |
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.
Maybe we could have these large helper scripts available as a separate utility to be invoked at build time?
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.
This has been done separately.
4e6a806
to
e716f68
Compare
Closing this as this implementation isn't really usable with the current state of the code, but I think this is feasible in the future with @cmatsuoka's suggestions: #936 |