coreos-meta-translator: add gcp cloud launchable to release.json #112
Conversation
This file contains 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
|
Continuation of #109, somehow GH does not allow me to reopen the PR, so moved to here. |
zonggen
pushed a commit
to zonggen/fedora-coreos-stream-generator
that referenced
this pull request
Jun 15, 2020
This change works with coreos/fedora-coreos-releng-automation#112 and adds the name, family and project metadata for GCP cloud launchable image from release.json to $stream.json. Signed-off-by: Allen Bai <[email protected]>
dustymabe
reviewed
Jun 18, 2020
dustymabe
approved these changes
Jun 18, 2020
zonggen
force-pushed
the
meta-translator-gcp
branch
from
9697d71 to
d5251ac
Compare
jlebon
reviewed
Jun 18, 2020
coreos-meta-translator/trans.py
Outdated
| arch_dict["media"]["gcp"] = arch_dict["media"].get("gcp", {}) | ||
| arch_dict["media"]["gcp"]["image"] = arch_dict["media"]["gcp"].get("image", {}) | ||
| arch_dict["media"]["gcp"]["image"].update(input_.get("gcp", {})) | ||
| if arch_dict["media"]["gcp"]["image"].get("image", None) is not None: |
There was a problem hiding this comment.
Should this be conditional? Feels like it should be an error if image wasn't in the input, no? IOW, can we drop this and instead just let pop error out if it's missing?
There was a problem hiding this comment.
Emm.. I think this condition will always be true, since arch_dict["media"]["gcp"]["image"].update(input_.get("gcp", {})) made sure it is at least a {}. Anyway this is redundant, dropping this
zonggen
force-pushed
the
meta-translator-gcp
branch
from
d5251ac to
40a6bbc
Compare
dustymabe
pushed a commit
to coreos/fedora-coreos-stream-generator
that referenced
this pull request
Jun 18, 2020
This change works with coreos/fedora-coreos-releng-automation#112 and adds the name, family and project metadata for GCP cloud launchable image from release.json to $stream.json. Signed-off-by: Allen Bai <[email protected]>
relrod
pushed a commit
to fedora-infra/fedora-websites-mirror
that referenced
this pull request
Jun 18, 2020
Adds GCP cloud launchable name, family and project on the download page. This works along side with - coreos/fedora-coreos-releng-automation#112 to add GCP info in release.json - coreos/fedora-coreos-stream-generator#11 to add GCP info in ${stream}.json This change reads metadata from ${stream}.json and renders it. It is also possible to read directly from individual meta.json file but since the download page is already utilizing ${stream}.json, this changes is trying to make use of already fetched metadata instead of fetching another meta.json. Closes: coreos/fedora-coreos-tracker#494 Signed-off-by: Allen Bai <[email protected]>
jlebon
approved these changes
Jun 18, 2020
coreos-meta-translator/trans.py
Outdated
| arch_dict["media"]["gcp"] = arch_dict["media"].get("gcp", {}) | ||
| arch_dict["media"]["gcp"]["image"] = arch_dict["media"]["gcp"].get("image", {}) | ||
| arch_dict["media"]["gcp"]["image"].update(input_.get("gcp", {})) | ||
| arch_dict["media"]["gcp"]["image"]["name"] = arch_dict["media"]["gcp"]["image"].pop("image", None) |
There was a problem hiding this comment.
My suggestion is to make it a hard error so we don't silently continue if image is missing from meta.json:
Suggested change
| arch_dict["media"]["gcp"]["image"]["name"] = arch_dict["media"]["gcp"]["image"].pop("image", None) | |
| arch_dict["media"]["gcp"]["image"]["name"] = arch_dict["media"]["gcp"]["image"].pop("image") |
There was a problem hiding this comment.
Oh I see, and the previous pop("image") too. Updatinged
Related: coreos/fedora-coreos-tracker#494 Signed-off-by: Allen Bai <[email protected]>
zonggen
force-pushed
the
meta-translator-gcp
branch
from
40a6bbc to
145a438
Compare
jlebon
approved these changes
Jun 18, 2020
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.
As AMIs have their own field
imagesunder theawskey, I think it might make sense for GCP to follow the same rule, adding animagefield under thegcpkey and rename the originalimagekey toname.If this is reasonable, it should be followed by updating
fedora-coreos-stream-generatorto support this PR. Finally, download page could read the GCP cloud launchable metadata from e.g.stable.jsoninstead ofmeta.jsonRelated: coreos/fedora-coreos-tracker#494
Signed-off-by: Allen Bai [email protected]