-
Notifications
You must be signed in to change notification settings - Fork 0
Update CLI to use single config logic #158
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
Conversation
Test Results196 tests 196 ✅ 1m 15s ⏱️ Results for commit 9747868. ♻️ This comment has been updated with latest results. |
344f2b3 to
a38302e
Compare
bschwedler
left a comment
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.
Thank you for doing this! I really like these changes.
I want to play with the CLI a bit before approving the PR.
| self.yaml.preserve_quotes = True | ||
| self.yaml.indent(mapping=2, sequence=4, offset=2) |
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.
💜
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.
Just so you know, this is causing some weird behavior. Like the quoting has been inconsistent for me when dumping models. For instance it quotes subpath, but not name for images and versions like this:
- name: 1.0.0
subpath: '1.0'I'm not sure why that is though. They're both strings and I haven't spotted anything that would force quoting. 🤷♂️
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.
Can we also force writing of the YAML file to always include the trailing newline at the end of the file?
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.
ruamel.yaml cannot, but I could do it manually other ways.
I was wrong, it does so automatically.
bschwedler
left a comment
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.
Some basic comments on the CLI functionality. I didn't do an in-depth test, just the basics
- Should we return with a non-zero exit code calling
bakery create projectif one already exists? --subpathflag- Only appears to support creating directories one leve deep.
- Does not seeom to be respected when calling
bakery create version
- Should
bakery helpdo the same thing asbakery --help? bakery create versionappears to try to validate whether there are images even when creating the first version
I'll come back and address adding versions when the second failure below is resolved.
Failures
± bakery create image --base-image ubuntu:24.04 --subpath fancy/take-3 fancy-3 1 ↵
[11:20:50] INFO Loading Bakery config from config.py:293
/home/bschwedler/repos/posit-dev/images-shared/posit-bakery/brjs_test/bakery.yaml
WARNING No versions found in image 'fancy-1'. At least one version is required for most image.py:444
commands.
WARNING No versions found in image 'fancy-2'. At least one version is required for most image.py:444
commands.
WARNING No versions found in image 'fancy-3'. At least one version is required for most image.py:444
commands.
ERROR Error creating image create.py:80
╭───────────────────────── Traceback (most recent call last) ─────────────────────────╮
│ /home/bschwedler/repos/posit-dev/images-shared/posit-bakery/posit_bakery/cli/create │
│ .py:71 in image │
│ │
│ 68 │ """ │
│ 69 │ try: │
│ 70 │ │ c = BakeryConfig.from_context(context) │
│ ❱ 71 │ │ c.create_image( │
│ 72 │ │ │ image_name, │
│ 73 │ │ │ base_image=base_image, │
│ 74 │ │ │ subpath=subpath, │
│ │
│ /home/bschwedler/repos/posit-dev/images-shared/posit-bakery/posit_bakery/config/con │
│ fig.py:373 in create_image │
│ │
│ 370 │ │ │ description=description, │
│ 371 │ │ │ documentation_url=documentation_url, │
│ 372 │ │ ) │
│ ❱ 373 │ │ self.model.create_image_files_template(new_image.path, new_image.name │
│ base_image or DEFAULT_BASE_IMAGE) │
│ 374 │ │ new_image_dict = new_image.model_dump(exclude_defaults=True, exclude_ │
│ exclude_unset=True) │
│ 375 │ │ self._config_yaml.setdefault("images", []).append(new_image_dict) │
│ 376 │ │ self.write() │
│ │
│ /home/bschwedler/repos/posit-dev/images-shared/posit-bakery/posit_bakery/config/con │
│ fig.py:146 in create_image_files_template │
│ │
│ 143 │ │ exists: bool = image_path.is_dir() │
│ 144 │ │ if not exists: │
│ 145 │ │ │ log.debug(f"Creating new image directory [bold]{image_path}") │
│ ❱ 146 │ │ │ image_path.mkdir() │
│ 147 │ │ │
│ 148 │ │ image_template_path = image_path / "template" │
│ 149 │ │ if not image_template_path.is_dir(): │
│ │
│ /usr/lib64/python3.13/pathlib/_local.py:722 in mkdir │
│ │
│ 719 │ │ Create a new directory at this given path. │
│ 720 │ │ """ │
│ 721 │ │ try: │
│ ❱ 722 │ │ │ os.mkdir(self, mode) │
│ 723 │ │ except FileNotFoundError: │
│ 724 │ │ │ if not parents or self.parent == self: │
│ 725 │ │ │ │ raise │
╰─────────────────────────────────────────────────────────────────────────────────────╯
FileNotFoundError: [Errno 2] No such file or directory:
'/home/bschwedler/repos/posit-dev/images-shared/posit-bakery/brjs_test/fancy/take-3'
❌ Failed to create image 'fancy-3'
± bakery create version fancy-2 2025.07.1
[11:18:07] INFO Loading Bakery config from config.py:293
/home/bschwedler/repos/posit-dev/images-shared/posit-bakery/brjs_test/bakery.yaml
WARNING No versions found in image 'fancy-1'. At least one version is required for most image.py:444
commands.
WARNING No versions found in image 'fancy-2'. At least one version is required for most image.py:444
commands.
WARNING No OSes defined for image version '2025.07.1'. At least one OS should be defined for image.py:139
complete tagging and labeling of images.
WARNING No OS marked as primary for image version '2025.07.1'. At least one OS should be marked image.py:202
as primary for complete tagging and labeling of images.
ERROR Error creating version create.py:141
╭──────────────────────── Traceback (most recent call last) ─────────────────────────╮
│ /home/bschwedler/repos/posit-dev/images-shared/posit-bakery/posit_bakery/cli/creat │
│ e.py:132 in version │
│ │
│ 129 │ │
│ 130 │ try: │
│ 131 │ │ c = BakeryConfig.from_context(context) │
│ ❱ 132 │ │ c.create_version( │
│ 133 │ │ │ image_name=image_name, │
│ 134 │ │ │ subpath=subpath, │
│ 135 │ │ │ version=image_version, │
│ │
│ /home/bschwedler/repos/posit-dev/images-shared/posit-bakery/posit_bakery/config/co │
│ nfig.py:428 in create_version │
│ │
│ 425 │ │ image_index = self._get_image_index(image_name) │
│ 426 │ │ if latest: │
│ 427 │ │ │ # If this is the latest version, we need to remove the latest fl │
│ other versions. │
│ ❱ 428 │ │ │ for v in self._config_yaml["images"][image_index]["versions"]: │
│ 429 │ │ │ │ if v.get("latest", False) and v["name"] != version: │
│ 430 │ │ │ │ │ v.pop("latest", None) │
│ 431 │ │ if not existing_version: │
│ │
│ /home/bschwedler/repos/posit-dev/images-shared/posit-bakery/.venv/lib/python3.13/s │
│ ite-packages/ruamel/yaml/comments.py:854 in __getitem__ │
│ │
│ 851 │ │
│ 852 │ def __getitem__(self, key: Any) -> Any: │
│ 853 │ │ try: │
│ ❱ 854 │ │ │ return ordereddict.__getitem__(self, key) │
│ 855 │ │ except KeyError: │
│ 856 │ │ │ for merged in getattr(self, merge_attrib, []): │
│ 857 │ │ │ │ # if isinstance(merged, tuple): │
╰────────────────────────────────────────────────────────────────────────────────────╯
KeyError: 'versions'
❌ Failed to create version 'fancy-2/2025.07.1'
|
These are all candidates for breaking out into follow- up QoL issues. Just putting them down so I don't forget. Functionality questions:Create versionShould we be able to specify the OS and primary OS via CLI flags?
|
bschwedler
left a comment
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 this is in great shape to merge.
There are some functionality questions I have, but I think they would be better addressed in smaller follow-up PRs.
| def pytest_bdd_apply_tag(tag, function): | ||
| """Modify scenario tags to be pytest-compatible.""" | ||
| if tag == "xdist-build": | ||
| marker = pytest.mark.xdist_group("build") | ||
| marker(function) | ||
| return True | ||
| else: | ||
| return None |
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.
💜
typerfrom0.12.5to0.16.1for compatibility with latestclickdistribution.no_args_is_helpbehavior to use the exit code 2 per 🐛 Ensure exit code 0 whenno_args_is_helpshows help fastapi/typer#1240.richfrom13.8.1to14.1.0.jinja2andgitpython.packagingas a strict requirement. It seemed unnecessary and inconvenient to explicitly pin.ruffandshellcheckversions used by pre-commit.run snykcommand.build--builderin favor of letting the user change the builder themselves ahead of time, at least for now. This was more because it was confusing to implement in Python on Whales, particularly with the addition of sequential builds.--fail-fastoption for stopping on first failure for sequential builds.--loadhas been changed toTrue. I find it's exceedingly rare that I would not want to load images after building.create--subpathtocreatecommands to specify a subpath different from a version or image name.--display-name,--description, and--documentation-urltocreate imageto specify those labeling values ahead of time.BakeryConfigDocumentdisplay_name,description, anddocumentation_urlforcreate_image_modelmethod.Containerfilenaming patterns for creating default image templates. It now correctly condenses the base image tag (e.g.docker.io/library/ubuntu:22.04createsContainerfile.ubuntu2204.jinja2).BakeryConfigfrom_contextmethod to discover abakery.yamlorbakery.ymlfile in the given path to load a project. This is a shortcut for the CLI.display_name,description, anddocumentation_urlforcreate_imagemethod.create_versionlogic.latestflag on other versions if given version is markedlatest.bake_plan_targetsfunction to return plan JSON without building.ImagedocumentationUrlto return a string instead of an object representation.VersionPathin templating values to always return a relative path regardless of which value is used.latestlogic increate_version_model.GossOptionsupdatelogic.runtimeOptionsfield for supporting images that require special run options, such as Connect with--privileged.ImageTargetremovemethod to delete image builds. This is not yet used or exposed.buildmethod.0and to20withshow_localsenabled when verbose.