-
Notifications
You must be signed in to change notification settings - Fork 105
feat(profiles): bound dependencies, remove uv.lock files, improve init output #2754
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: main
Are you sure you want to change the base?
Changes from all commits
d8ef91b
7609a3f
bb5192e
2140cba
beedd51
849b16f
107a86e
c6fcf1c
e10c029
a2981b4
8004d45
32c5811
aee7352
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -86,10 +86,11 @@ | |||||
| ├── LICENSE - Your charm license, we recommend Apache 2 | ||||||
| ├── pyproject.toml - Configuration for testing, formatting and | ||||||
| │ linting tools. Specifies Python dependencies for | ||||||
| │ your charm if profile is 'kubernetes' or 'machine' | ||||||
| │ all profiles except 12-factor app profiles | ||||||
| │ targeting Ubuntu 24.04 or lower | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. lower->earlier? Same below.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 'higher' and 'lower' are preferred in the craft apps https://documentation.ubuntu.com/starflow/latest/how-to/starcraft-style-guide/#version-names However, we should change 'Ubuntu 24.04' to 'Ubuntu 24.04 LTS' for branding and consistency with the subsequent change to this file. |
||||||
| ├── README.md - Frontpage for your charmhub.io/charm/ | ||||||
| ├── requirements.txt - Python dependencies for your charm, with Ops, | ||||||
| │ created for 12-factor app profiles only | ||||||
| ├── requirements.txt - Python dependencies for 12-factor app profiles | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Optional:
Suggested change
|
||||||
| │ targeting Ubuntu 24.04 LTS or lower | ||||||
| ├── src | ||||||
| │ ├── charm.py - Python code that operates your charm's workload | ||||||
| │ └── <workload>.py - Standalone module for workload-specific logic, | ||||||
|
|
@@ -100,8 +101,6 @@ | |||||
| │ └── unit | ||||||
| │ └── test_charm.py - Unit tests | ||||||
| ├── tox.ini - Configuration for tox, the tool to run all tests | ||||||
| ├── uv.lock - Specifies exact versions of Python dependencies, | ||||||
| │ created if profile is 'kubernetes' or 'machine' | ||||||
|
|
||||||
| You will need to edit at least charmcraft.yaml and README.md. | ||||||
|
|
||||||
|
|
@@ -111,23 +110,29 @@ | |||||
| """ | ||||||
|
|
||||||
|
|
||||||
| def _make_success_message(src_files: list[str]) -> str: | ||||||
| src_files_str = "\n".join(src_files) | ||||||
| return f"""\ | ||||||
| Charmed operator package file and directory tree initialised. | ||||||
|
|
||||||
| Now edit the following package files to provide fundamental charm metadata | ||||||
| and other information: | ||||||
|
|
||||||
| charmcraft.yaml | ||||||
| {src_files_str} | ||||||
| README.md | ||||||
| def _make_success_message(project_files: list[str]) -> str: | ||||||
| project_files_str = "\n".join(sorted(project_files, key=str.casefold)) | ||||||
| default_message = f"""\ | ||||||
| Created project files for your charm: | ||||||
|
|
||||||
| {project_files_str} | ||||||
| ... | ||||||
| """ | ||||||
| uv_message = """\ | ||||||
| To manage your charm's dependencies, use uv. | ||||||
|
|
||||||
| To migrate from the Charm plugin to the uv plugin, see: | ||||||
| https://documentation.ubuntu.com/charmcraft/stable/howto/migrate-plugins/charm-to-uv/ | ||||||
| https://canonical.com/juju/docs/charmcraft/stable/howto/migrate-plugins/charm-to-uv/ | ||||||
|
|
||||||
| Next steps: | ||||||
|
|
||||||
| 1. Run 'uv lock' | ||||||
| 2. Edit charmcraft.yaml and pyproject.toml to provide metadata, then commit (including uv.lock) | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Optional:
Suggested change
Since this is in CLI copy, I could go either way it adds characters. Up to you! |
||||||
| 3. Write your charm code and tests | ||||||
| """ | ||||||
| if "pyproject.toml" in project_files and "requirements.txt" not in project_files: | ||||||
| return f"{default_message}\n{uv_message}" | ||||||
| return default_message | ||||||
|
|
||||||
|
|
||||||
| def _make_workload_module_name(charm_name: str) -> str: | ||||||
|
|
@@ -249,7 +254,16 @@ def run(self, parsed_args: argparse.Namespace): | |||||
| "tests/spread/lib/tools/retry", | ||||||
| "spread/.extension", | ||||||
| ] | ||||||
| src_files = ["src/charm.py"] | ||||||
| notable_project_files = [ # files worth mentioning in the command output | ||||||
| "charmcraft.yaml", | ||||||
| "pyproject.toml", | ||||||
| "README.md", | ||||||
| "requirements.txt", | ||||||
| "spread.yaml", | ||||||
| "spread/deploy/basic/task.yaml", | ||||||
| "src/charm.py", | ||||||
| ] | ||||||
| mention_project_files = [] # files to actually mention in the command output | ||||||
| for template_name in env.list_templates(): | ||||||
| if not template_name.endswith(".j2"): | ||||||
| continue | ||||||
|
|
@@ -266,10 +280,12 @@ def run(self, parsed_args: argparse.Namespace): | |||||
| if template_name in executables and os.name == "posix": | ||||||
| make_executable(fh) | ||||||
| emit.debug(" made executable") | ||||||
| if path.name == "workload.py" and path.parent.name == "src": | ||||||
| if template_name in notable_project_files: | ||||||
| mention_project_files.append(template_name) | ||||||
| if template_name == "src/workload.py": | ||||||
| workload_module = context["workload_module"] | ||||||
| workload_module_path = path.with_name(f"{workload_module}.py") | ||||||
| path.rename(workload_module_path) | ||||||
| src_files.append(f"src/{workload_module}.py") | ||||||
| for line in _make_success_message(src_files).split("\n"): | ||||||
| mention_project_files.append(f"src/{workload_module}.py") | ||||||
| for line in _make_success_message(mention_project_files).split("\n"): | ||||||
| emit.message(line) | ||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -13,28 +13,28 @@ requires-python = ">=3.10" | |
| # 'charm-libs' block in charmcraft.yaml, run `charmcraft fetch-libs` to download the libraries, | ||
| # then inspect the libraries for dependencies specified in PYDEPS. List those dependencies here. | ||
| dependencies = [ | ||
| "ops~=3.7", | ||
| "ops>=3.8,<4", | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I know not everyone agrees, and it's annoying that there isn't "one obvious way to do it", but I think |
||
| ] | ||
|
|
||
| [dependency-groups] | ||
| # Dependencies of linting and static type checks | ||
| lint = [ | ||
| "ruff", | ||
| "codespell", | ||
| "pyright", | ||
| "ruff>=0.15,<1", | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure No upper bound is probably not ideal either. |
||
| "codespell>=2.4,<3", | ||
| "pyright>=1.1,<2", | ||
| ] | ||
| # Dependencies of unit tests | ||
| unit = [ | ||
| "coverage[toml]", | ||
| "ops[testing]", | ||
| "pytest", | ||
| "coverage[toml]>=7.15,<8", | ||
| "ops[testing]>=3.8,<4", | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We don't really need the versions here, because |
||
| "pytest>=9.1,<10", | ||
| ] | ||
| # Dependencies of integration tests | ||
| integration = [ | ||
| "jubilant>=1.8,<2", | ||
| "pytest", | ||
| "pytest-jubilant>=2.0.1,<3", | ||
| "PyYAML", | ||
| "jubilant>=1.11,<2", | ||
| "pytest>=9.1,<10", | ||
| "pytest-jubilant>=2.1,<3", | ||
| "PyYAML>=6.0,<7", | ||
| ] | ||
|
|
||
| # Testing tools configuration | ||
|
|
||
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.
Optional:
See https://github.com/canonical/charmcraft/pull/2754/changes#r3634177095