Testing that Review App Deployment Workflow only redeploys on Pull Request Trigger #645
Conversation
|
""" WalkthroughThis change updates the text label of a hyperlink in the Footer component, specifically within the file Changes
Possibly related PRs
Poem
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 (
|
🚀 Quick Review App CommandsWelcome! Here are the commands you can use in this PR:
|
|
🚀 Deploying to Control Plane... ⏳ Waiting for deployment to be ready... |
|
🚀 Deploying to Control Plane... ⏳ Waiting for deployment to be ready... |
![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
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/deploy-to-control-plane-review-app.yml(1 hunks)client/app/bundles/comments/components/Footer/Footer.jsx(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- client/app/bundles/comments/components/Footer/Footer.jsx
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: test (22.x, 3.3.4)
- GitHub Check: deploy
| if cpflow exists -a ${{ env.APP_NAME }}; then | ||
| if [[ "${{ github.event_name }}" == "pull_request" ]]; then | ||
| exit 0 | ||
| echo "Canceling job as review app has not been previously deployed."; sleep inf | ||
| fi | ||
| echo "APP_EXISTS=false" >> $GITHUB_ENV | ||
| else | ||
| echo "APP_EXISTS=true" >> $GITHUB_ENV | ||
| fi |
There was a problem hiding this comment.
Incorrect APP_EXISTS flag handling & invalid sleep invocation.
- The script currently sets
APP_EXISTS=falsewhen the app exists, which inverts the intended semantics for the “Setup Control Plane App if Not Existing” step. There’s also no branch for when the app does not exist, soAPP_EXISTSremains undefined in that case. - Using
sleep infis invalid as thesleepcommand does not accept “inf” and will error instead of pausing indefinitely. To gracefully skip further deployment steps on PRs, useexit 0(or a proper infinite loop) after setting the flag.
Apply this diff to correct the existence check and early-exit behavior:
- if cpflow exists -a ${{ env.APP_NAME }}; then
- if [[ "${{ github.event_name }}" == "pull_request" ]]; then
- echo "Canceling job as review app has not been previously deployed."; sleep inf
- fi
- echo "APP_EXISTS=false" >> $GITHUB_ENV
- fi
+ if cpflow exists -a ${{ env.APP_NAME }}; then
+ # App already exists
+ echo "APP_EXISTS=true" >> $GITHUB_ENV
+ if [[ "${{ github.event_name }}" == "pull_request" ]]; then
+ echo "Review app for PR #${{ env.PR_NUMBER }} already deployed; skipping redeployment."
+ exit 0
+ fi
+ else
+ # App does not exist
+ echo "APP_EXISTS=false" >> $GITHUB_ENV
+ fi📝 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.
| if cpflow exists -a ${{ env.APP_NAME }}; then | |
| if [[ "${{ github.event_name }}" == "pull_request" ]]; then | |
| exit 0 | |
| echo "Canceling job as review app has not been previously deployed."; sleep inf | |
| fi | |
| echo "APP_EXISTS=false" >> $GITHUB_ENV | |
| else | |
| echo "APP_EXISTS=true" >> $GITHUB_ENV | |
| fi | |
| if cpflow exists -a ${{ env.APP_NAME }}; then | |
| # App already exists | |
| echo "APP_EXISTS=true" >> $GITHUB_ENV | |
| if [[ "${{ github.event_name }}" == "pull_request" ]]; then | |
| echo "Review app for PR #${{ env.PR_NUMBER }} already deployed; skipping redeployment." | |
| exit 0 | |
| fi | |
| else | |
| # App does not exist | |
| echo "APP_EXISTS=false" >> $GITHUB_ENV | |
| fi |
|
🚀 Deploying to Control Plane... ⏳ Waiting for deployment to be ready... |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
.github/workflows/deploy-to-control-plane-review-app.yml (2)
119-119: 🛠️ Refactor suggestion
⚠️ Potential issueReplace invalid
sleep infwith an early exit.
sleep infis not a valid invocation ofsleepand will error out. To skip redeployment on pull requests, exit the script gracefully:- echo "Canceling job as review app has not been previously deployed."; sleep inf + echo "Review app not deployed yet; skipping redeployment for PR." + exit 0
121-123: 🛠️ Refactor suggestionHandle both existence cases and remove duplicate ENV export.
Currently there’s no
elsebranch to setAPP_EXISTS=true, and theAPP_EXISTS=falseexport is printed twice. Recommend consolidating into a two-branch check:- if cpflow exists -a "${{ env.APP_NAME }}"; then - # ... - echo "APP_EXISTS=false" - echo "APP_EXISTS=false" >> $GITHUB_ENV - fi + if cpflow exists -a "${{ env.APP_NAME }}"; then + echo "APP_EXISTS=true" >> $GITHUB_ENV + if [[ "${{ github.event_name }}" == "pull_request" ]]; then + echo "Review app already deployed; skipping redeploy on PR." + exit 0 + fi + else + echo "APP_EXISTS=false" >> $GITHUB_ENV + fiThis sets the flag correctly in both scenarios and removes the duplicated export.
🧹 Nitpick comments (1)
.github/workflows/deploy-to-control-plane-review-app.yml (1)
114-115: Remove extraneous debugging statements.The
echo "apple",echo "$(cpflow exists -a ${{ env.APP_NAME }})"andecho "bananas"lines are leftover debug noise. They should be removed to keep the workflow logs concise and clear.Also applies to: 124-124
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/deploy-to-control-plane-review-app.yml(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: test (22.x, 3.3.4)
|
🚀 Deploying to Control Plane... ⏳ Waiting for deployment to be ready... |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.github/workflows/deploy-to-control-plane-review-app.yml (1)
123-131: Gating new Control Plane app setup and settingAPP_EXISTS
Thesetup-appstep now only runs whenAPP_EXISTSisfalseandgithub.event_nameis not a pull request, then correctly setsAPP_EXISTS=trueafterward so downstream steps proceed. This aligns with the goal to avoid redeployment on PR triggers. Consider quoting the app name argument ("${{ env.APP_NAME }}") for added safety.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/deploy-to-control-plane-review-app.yml(7 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: test (22.x, 3.3.4)
🔇 Additional comments (10)
.github/workflows/deploy-to-control-plane-review-app.yml (10)
114-121: Confirm pull request cancellation behavior without exit
The step echoes a cancellation message for PR events when the app doesn’t exist but does not terminate the job. Subsequent steps are correctly gated onAPP_EXISTS, so no deployment runs—but the runner still processes the entire workflow. Please verify if you intend to keep the job alive to the end or explicitly exit (e.g.,exit 0) immediately after the echo to shorten logs and runtime.
132-136: Gate initial deployment comment onAPP_EXISTS
Addingif: env.APP_EXISTS == 'true'ensures the initial GitHub comment is only created when a review app exists or has just been set up—preventing unnecessary comments on PR workflows without an app.
146-151: Gate setting of deployment URLs onAPP_EXISTS
By gating this step, you avoid exportingWORKFLOW_URLandCONSOLE_LINKwhen no review app exists, ensuring downstream steps that rely on these variables won’t error out.
178-183: Gate GitHub deployment initialization onAPP_EXISTS
Conditioning the create-deployment call onAPP_EXISTSprevents unnecessary API requests when there’s no app, aligning with the intended behavior.
208-212: Gate 'Building' status update onAPP_EXISTS
Ensures the "Building" comment update only runs when an initial comment exists, avoiding API errors from updating a non-existent comment.
228-233: Gate Docker image build onAPP_EXISTS
Prevents starting a Docker build for non‐existent review apps, satisfying the PR objective to restrict deployments on PR triggers without apps.
238-243: Gate 'Deploying' status update onAPP_EXISTS
The gating ensures that only valid deployments update the issue comment, preventing misleading messages when no deployment occurs.
260-263: Gate actual image deployment onAPP_EXISTS
Checkingenv.APP_EXISTShere blockscpflow deploy-imagefrom running when there’s no app, fully preventing redeployment on pull_request events.
264-268: Gate retrieval of app URL onAPP_EXISTS
Conditioning theWORKLOAD_URLextraction onAPP_EXISTSavoids running thecpln workload getcommand for non-existent apps, keeping the workflow clean.
269-273: Gate final deployment status update onAPP_EXISTS
The final status update is now correctly gated, so you only post a "deployment complete" comment when a deployment actually occurred, avoiding confusion on PRs without apps.
|
✅ Review app for PR #645 was successfully deleted |
This change is
Summary by CodeRabbit
Style
Chores