cmd-import: make pre-existing build ID check arch-aware #4287
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This is basically the same as 2de6b0d ("cmd-build-with-buildah: make build existence check arch-aware") but for the import command.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request makes the pre-existing build ID check in cmd-import architecture-aware, which is a good improvement for multi-architecture build support. The logic is correct. I have one suggestion to improve the efficiency of the check by avoiding iterating over the list of builds twice.
| if builds.has(buildid): | ||
| print(f"ERROR: Build ID {buildid} already exists!") | ||
| arch = get_basearch() | ||
| if builds.has(buildid) and arch in builds.get_build_arches(buildid): |
Contributor
There was a problem hiding this comment.
The current implementation correctly checks for the existence of the build for the given architecture, but it iterates over the list of builds twice: once in builds.has(buildid) and a second time in builds.get_build_arches(buildid).
This can be made more efficient by iterating only once. Here's a suggestion that achieves this using a generator expression with next().
Suggested change
| if builds.has(buildid) and arch in builds.get_build_arches(buildid): | |
| if arch in next((b.get('arches', []) for b in builds.get_builds() if b['id'] == buildid), []): |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is basically the same as 2de6b0d ("cmd-build-with-buildah: make build existence check arch-aware") but for the import command.