Skip to content

Conversation

wyattwalter
Copy link
Contributor

@wyattwalter wyattwalter commented Sep 25, 2025

Description

Tip

Add a TL;DR when the description is longer than 500 words or extremely technical (helps the content, marketing, and DevRel team).

Please also include relevant motivation and context. List any dependencies that are required for this change. Add links to Notion, Figma or any other documents that might be relevant to the PR.

Fixes #Issue Number
or
Fixes Issue URL

Warning

If no issue exists, please create an issue first, and check with the maintainers if the issue is valid.

Automation

/ok-to-test tags=""

🔍 Cypress test results

Caution

If you modify the content in this section, you are likely to disrupt the CI result for your PR.

Communication

Should the DevRel and Marketing teams inform users about this change?

  • Yes
  • No

Summary by CodeRabbit

  • Chores
    • Improved preview environment reset by running database cleanup inside the cluster for better reliability and security.
    • Added clearer start/end logs around the reset operation to aid troubleshooting.
    • Preserves the existing cleanup steps and finalizers to maintain the current reset flow.
    • Enhances stability of preview deployments without changing user-facing behavior.

Copy link
Contributor

coderabbitai bot commented Sep 25, 2025

Walkthrough

Replaces external MongoDB drop via public URL with in-cluster execution using kubectl exec to run mongosh inside the target pod, leveraging APPSMITH_DB_URL. Adds echo logs and comments. Retains subsequent cleanup steps and finalizers in the reset sequence.

Changes

Cohort / File(s) Summary
Deployment/preview reset script
scripts/deploy_preview.sh
Switch MongoDB drop from external URL to in-cluster kubectl exec running mongosh with APPSMITH_DB_URL; add echo logs and explanatory comments; keep remaining cleanup/finalizer steps unchanged.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor Dev as Script Runner
  participant Sh as deploy_preview.sh
  participant K8s as Kubernetes API
  participant Pod as Appsmith Pod
  participant Mongo as MongoDB

  Dev->>Sh: Run preview reset
  Sh->>K8s: kubectl exec into target pod
  K8s->>Pod: Start shell session
  Note right of Pod: Execute mongosh using APPSMITH_DB_URL
  Pod->>Mongo: Drop database
  Mongo-->>Pod: Drop result
  Pod-->>Sh: Exec exit status
  Sh-->>Dev: Echo logs and proceed with remaining cleanup/finalizers
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

A pod whispers, “I’ll drop it inside,”
No public gate, no ocean-wide ride.
kubectl knocks, mongosh replies—
The DB resets under cluster skies.
Echoes confirm, the cleanup’s done,
Preview’s fresh, a brand new run.

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Description Check ⚠️ Warning The pull request description is unmodified from the repository template and contains no actual details about the change such as a summary, motivation, context, dependencies, issue reference, or filled sections, leaving all placeholders and checkboxes unanswered. Please replace the template placeholders with a concise summary, include motivation, context, any dependencies, add a valid “Fixes #IssueNumber” or URL, and complete the communication checkboxes and other required sections.
✅ Passed checks (2 passed)
Check name Status Explanation
Title Check ✅ Passed The title succinctly states the fix for the deploy preview (dp) recreation failure caused by network restrictions, which directly aligns with the changes in deploy_preview.sh moving the MongoDB drop in-cluster to avoid external network calls.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch ww-fix-dp-recreate

Tip

👮 Agentic pre-merge checks are now available in preview!

Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.

  • Built-in checks – Quickly apply ready-made checks to enforce title conventions, require pull request descriptions that follow templates, validate linked issues for compliance, and more.
  • Custom agentic checks – Define your own rules using CodeRabbit’s advanced agentic capabilities to enforce organization-specific policies and workflows. For example, you can instruct CodeRabbit’s agent to verify that API documentation is updated whenever API schema files are modified in a PR. Note: Upto 5 custom checks are currently allowed during the preview period. Pricing for this feature will be announced in a few weeks.

Please see the documentation for more information.

Example:

reviews:
  pre_merge_checks:
    custom_checks:
      - name: "Undocumented Breaking Changes"
        mode: "warning"
        instructions: |
          Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).

Please share your feedback with us on this Discord post.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions github-actions bot added the Bug Something isn't working label Sep 25, 2025
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b79b160 and 2ee0b35.

📒 Files selected for processing (1)
  • scripts/deploy_preview.sh (1 hunks)

pod_name="$(kubectl get pods -n "$NAMESPACE" -o json | jq -r '.items[0].metadata.name')"
# execute this db.dropDatabase() from the k8s cluster because there's network restrictions on Atlas cluster.
# The \$ is used to escape the $ character in the APPSMITH_DB_URL environment variable so it's interpolated inside the kubectl exec command.
kubectl exec "$pod_name" -n "$NAMESPACE" -- bash -c "mongosh \$APPSMITH_DB_URL --eval 'db.dropDatabase()'"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Quote the DB URL before passing it to mongosh.
APPSMITH_DB_URL contains multiple & query parameters. When bash -c expands the current string, those & characters background the preceding command and treat the remainder (minPoolSize=...) as separate shell commands, so mongosh never executes with the full URL and the drop fails. Wrap the variable in quotes inside the remote command.

-  kubectl exec "$pod_name" -n "$NAMESPACE" -- bash -c "mongosh \$APPSMITH_DB_URL --eval 'db.dropDatabase()'"
+  kubectl exec "$pod_name" -n "$NAMESPACE" -- bash -c "mongosh \"\$APPSMITH_DB_URL\" --eval 'db.dropDatabase()'"
🤖 Prompt for AI Agents
In scripts/deploy_preview.sh around line 56, the kubectl exec remote command
passes $APPSMITH_DB_URL unquoted so embedded & characters break the shell; fix
by ensuring the DB URL is quoted inside the remote bash -c string so the entire
URL is delivered as a single argument to mongosh (i.e., wrap the variable in
double quotes inside the inner command and escape those quotes so the remote
shell sees them), leaving the rest of the command unchanged.

Copy link

github-actions bot commented Oct 2, 2025

This PR has not seen activitiy for a while. It will be closed in 7 days unless further activity is detected.

@github-actions github-actions bot added the Stale label Oct 2, 2025
@wyattwalter wyattwalter merged commit 47cd687 into release Oct 3, 2025
17 checks passed
@wyattwalter wyattwalter deleted the ww-fix-dp-recreate branch October 3, 2025 12:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Bug Something isn't working Stale

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants