cosalib/aws: only replicate WinLI AMIs when --winli is specified #4339
+72
−72
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
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request correctly modifies the AWS replication logic to only replicate Windows License Included (WinLI) AMIs when the --winli flag is specified, which resolves an issue where they were always replicated if present. The logic is now cleaner and correctly tied to the command-line flag. The update to the CLI help text is also a good clarification. I've added a few minor suggestions to improve code conciseness.
src/cosalib/aws.py
Outdated
Comment on lines
35
to
38
| meta_key = "amis" | ||
| # only replicate WinLI AMIs if `--winli` is used | ||
| if args.winli: | ||
| meta_key = "aws-winli" |
There was a problem hiding this comment.
For better conciseness, you can use a conditional expression to assign meta_key. The comment on line 36 can then be moved to this new line as an inline comment.
Suggested change
| meta_key = "amis" | |
| # only replicate WinLI AMIs if `--winli` is used | |
| if args.winli: | |
| meta_key = "aws-winli" | |
| meta_key = "aws-winli" if args.winli else "amis" |
The current logic always considers the `aws-winli` buildmeta key when present, which causes WinLI AMIs to be replicated in both the standard AWS and GovCloud cases. Restore `aws_run_ore_replicate`[1] to only work on the 'amis' buildmeta key and switch to aws-winli when `--winli` is used. This ensures WinLI replication happens only when explicitly requested. Also update the CLI help text for `--winli` to clarify that it applies to replication as well as creation. [1]: coreos@a5bfbde
marmijo
force-pushed
the
winli-replicate
branch
from
d7e0fa9 to
eca4877
Compare
dustymabe
approved these changes
Oct 6, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
The current logic always considers the
aws-winlibuildmeta key when present, which causes WinLI AMIs to be replicated in both the standard AWS and GovCloud cases.Restore
aws_run_ore_replicate[1]to only work on the 'amis' buildmeta key and switch to aws-winli when--winliis used. This ensures WinLI replication happens only when explicitly requested. Also update the CLI help text for--winlito clarify that it applies to replication as well as creation.[1]: a5bfbde