Fixed destroy tests staging databases when a PR is merged#729
Conversation
ref no-issue - Fixed destroy staging databases when a PR is merged
WalkthroughThis change updates the Possibly related PRs
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
.github/workflows/cicd.yml (1)
311-318: Avoid duplicating GCP auth configuration—DRY up with an anchor or composite action.The “Authenticate with GCP (staging envs)” block is almost identical to other auth steps in this workflow. Consider extracting it into a YAML anchor or a reusable composite action to reduce duplication and simplify maintenance.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/cicd.yml(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Build, Test and Push
| - name: "Destroy Tests databases" | ||
| if: ${{ matrix.region == 'europe-west4' }} | ||
| env: | ||
| GCP_PROJECT: ghost-activitypub | ||
| run: | | ||
| TEST_DATABASES=$(gcloud sql databases list --instance=stg-netherlands-activitypub --filter="name~test*" --format="value(name)" --project ${GCP_PROJECT}) | ||
| for TEST_DATABASE in ${TEST_DATABASES}; do | ||
| gcloud sql databases delete ${TEST_DATABASE} --instance=stg-netherlands-activitypub --quiet --project ${GCP_PROJECT} | ||
| done |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Refine database deletion filter to target only ephemeral test databases.
Using --filter="name~test*" risks matching unintended databases. It’s safer to anchor the regex to your ephemeral naming convention (e.g. pr_<number>_test…) and enable strict bash safety flags.
Apply a targeted diff:
- run: |
- TEST_DATABASES=$(gcloud sql databases list --instance=stg-netherlands-activitypub --filter="name~test*" --format="value(name)" --project ${GCP_PROJECT})
- for TEST_DATABASE in ${TEST_DATABASES}; do
- gcloud sql databases delete ${TEST_DATABASE} --instance=stg-netherlands-activitypub --quiet --project ${GCP_PROJECT}
- done
+ run: |
+ set -euo pipefail
+ TEST_DATABASES=$(gcloud sql databases list \
+ --instance=stg-netherlands-activitypub \
+ --filter="name~'^pr_[0-9]+_test.*$'" \
+ --format="value(name)" \
+ --project="${GCP_PROJECT}")
+ for TEST_DATABASE in ${TEST_DATABASES}; do
+ gcloud sql databases delete "${TEST_DATABASE}" \
+ --instance=stg-netherlands-activitypub \
+ --quiet \
+ --project="${GCP_PROJECT}"
+ done📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - name: "Destroy Tests databases" | |
| if: ${{ matrix.region == 'europe-west4' }} | |
| env: | |
| GCP_PROJECT: ghost-activitypub | |
| run: | | |
| TEST_DATABASES=$(gcloud sql databases list --instance=stg-netherlands-activitypub --filter="name~test*" --format="value(name)" --project ${GCP_PROJECT}) | |
| for TEST_DATABASE in ${TEST_DATABASES}; do | |
| gcloud sql databases delete ${TEST_DATABASE} --instance=stg-netherlands-activitypub --quiet --project ${GCP_PROJECT} | |
| done | |
| - name: "Destroy Tests databases" | |
| if: ${{ matrix.region == 'europe-west4' }} | |
| env: | |
| GCP_PROJECT: ghost-activitypub | |
| run: | | |
| set -euo pipefail | |
| TEST_DATABASES=$(gcloud sql databases list \ | |
| --instance=stg-netherlands-activitypub \ | |
| --filter="name~'^pr_[0-9]+_test.*$'" \ | |
| --format="value(name)" \ | |
| --project="${GCP_PROJECT}") | |
| for TEST_DATABASE in ${TEST_DATABASES}; do | |
| gcloud sql databases delete "${TEST_DATABASE}" \ | |
| --instance=stg-netherlands-activitypub \ | |
| --quiet \ | |
| --project="${GCP_PROJECT}" | |
| done |
🤖 Prompt for AI Agents
In .github/workflows/cicd.yml around lines 319 to 327, the database deletion
command uses a loose filter "name~test*" which may match unintended databases.
Refine the filter to match your specific ephemeral test database naming pattern,
such as "pr_<number>_test", and add bash safety flags like 'set -euo pipefail'
at the start of the run block to improve script robustness and prevent
accidental deletions. Ensure the filter accurately targets only the intended
ephemeral test databases.
1 participant
ref no-issue