Repository navigation
genmedia-for-commerce: shorter media bucket name, MEDIA_BUCKET honoured by setup-infra, wait for new APIs - #2859
Merged
Merged
Conversation
…waits until new APIs answer
lspataroG
requested review from
eliasecchig and
giuliosalierno
as code owners
November 3, 2026 13:48
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Automated House Rules review — 0 finding(s).
Also, on lines this PR does not change:
core/python/genmedia-for-commerce/genmedia4commerce/core/settings.py:169— [MINOR] env read carries a hardcoded default (second argument is a hardcoded default); 4 occurrence(s) across 1 file(s). Defaults belong in .env.example, not in the code (core/python/genmedia-for-commerce/genmedia4commerce/core/settings.py:169; and 3 more)
| } | ||
|
|
||
| # Step 3: at most 5 minutes; on a re-run the APIs answer at once. | ||
| apis() { |
Contributor
There was a problem hiding this comment.
[MINOR] Perhaps this should be named enable_apis() instead of apis()? The other functions in this script use verbs like adopt() and plan_and_apply().
| tr -d '"') | ||
| # shellcheck disable=SC2086 # one argument per API | ||
| gcloud services enable $list --project="$PROJECT" --quiet | ||
| for n in $(seq 30); do |
Contributor
There was a problem hiding this comment.
[MINOR] Is there a reason 30 is hardcoded here? Can we extract this retry count into a named variable?
| echo "Waiting for the new APIs to be ready (a new project" \ | ||
| "takes a minute or two)..." | ||
| fi | ||
| sleep 10 |
Contributor
There was a problem hiding this comment.
[MINOR] hardcoded sleep interval here
| # 2. The resources with a fixed name that exist but are not in the state | ||
| # (made by a run whose state is lost) are imported, not created again. | ||
| # 3. terraform plan, then "Apply these changes ...? [Y/n]" on a terminal | ||
| # 3. The APIs of terraform/main.tf are enabled first, then the script |
Contributor
There was a problem hiding this comment.
[MINOR] It seems apis runs first in the execution pipeline rather than as step 3. Should the list of steps here reflect that?
… creates or adopts
lspataroG
changed the title
tommywagz
pushed a commit
that referenced
this pull request
Nov 4, 2026
…ed by setup-infra, wait for new APIs (#2859)
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
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.
Summary
<PROJECT_ID>-gfc-media(at most 40 characters). The old<PROJECT_ID>-genmedia-for-commerce-media-payloadspassed the 63-character limit for project ids over 26 characters, somake setup-infrafailed with "Use of this bucket name is restricted".removedblock), creates the new one and moves the app's grants to it. Older runs and studios stay in the old bucket.MEDIA_BUCKETin config.env now also names the bucket Terraform creates or adopts (it was read by the app only). Naming the old bucket keeps using it and its data; when it names another bucket than the one in the state, the state's one is forgotten, never deleted.make setup-infraenables the APIs first and waits (up to 5 minutes) until Artifact Registry and IAM answer, so a new project no longer fails with "Artifact Registry API has not been used ... or it is disabled". The 3-attempt retry stays.Tested live
MEDIA_BUCKETto its old bucket: setup adopted it and moved the grants, the other bucket kept its files, Cloud Run uses the old bucket.