-
Notifications
You must be signed in to change notification settings - Fork 59
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
Pjob/poc/pyapp binary with plugins #2035
base: pjob/SNOW-1747473-enable-disable-plugin-commands
Are you sure you want to change the base?
Pjob/poc/pyapp binary with plugins #2035
Conversation
@@ -14,4 +14,4 @@ | |||
|
|||
from __future__ import annotations | |||
|
|||
VERSION = "3.4.0.dev0" | |||
VERSION = "3.4.0.dev8" |
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 have issues with caching of my builds, that's why I bump version in my PoC. This should be resolved.
pip install -U hatch | ||
hatch clean | ||
hatch build | ||
pip install git+https://github.com/hobbsd/[email protected] |
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.
- It's external script. It should be reviewed.
- The script uses tomllib, so it requires python 3.11.
@@ -188,3 +188,6 @@ markers = [ | |||
|
|||
[tool.codespell] | |||
skip = 'tests/*,snow.spec' | |||
|
|||
[tool.hatch.build.targets.binary] | |||
python-version = "3.11" |
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 use python 3.11 but we had issues with pyinstaller binaries when using python 3.11 - we should check it.
from dataclasses import dataclass | ||
|
||
from snowflake.cli.api.plugins.command import CommandSpec | ||
|
||
sys.path.append("/tmp/cli_plugins/lib/python3.11/site-packages") |
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.
We should consider if we can do it better. For sure we need to load plugins from configurable directory.
) -> CommandResult: | ||
"""Installs a plugin from a package""" | ||
target_dir: str = ( | ||
"/tmp/cli_plugins" # TODO make it configurable in config.toml with some default |
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.
It should be configurable.
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 script is just PoC - probably not all commands are needed.
- I don't know how it works on different operating systems and CPUs.
- I don't how will work signing of the binaries.
- We should probably move the script to
scripts
directory.
"-m", | ||
"pip", | ||
"install", | ||
"--prefix", |
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.
--prefix
, not --target
to install only the plugin and new dependencies, without installation of existing CLI's dependencies.
Pre-review checklist
Changes description
PoC: PyApp + plugins installed and loaded to/from custom directory