chore(ci): drop Fly.io Electric deploys (use Electric Cloud)#3590
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
💤 Files with no reviewable changes (4)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughRemoved Fly.io / Electric deployment and preview integrations: deleted Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
Greptile SummaryThis PR removes the Fly.io-based Key changes:
Confidence Score: 5/5Safe to merge — changes are purely subtractive and correctly wire Electric Cloud secrets into the remaining jobs. All removals are well-scoped: the defunct Fly.io jobs, the associated fly.toml, the cleanup step, and the preview comment row are all consistently deleted. The ELECTRIC_URL/ELECTRIC_SECRET secrets were already present in the production deploy-api step and are now mirrored to the preview deploy-api step, which is the correct approach. The only action item is confirming those secrets exist in the preview GitHub environment — an operational step noted in the P2 comment — but it does not block merging this code change. .github/workflows/deploy-preview.yml — verify ELECTRIC_URL/ELECTRIC_SECRET are provisioned in the preview GitHub environment before the next preview PR is opened.
|
| Filename | Overview |
|---|---|
| .github/workflows/deploy-production.yml | Removes the deploy-electric Fly.io job; ELECTRIC_URL/ELECTRIC_SECRET were already present in the deploy-api step, so production Electric connectivity is unaffected. |
| .github/workflows/deploy-preview.yml | Removes the deploy-electric Fly.io job and adds ELECTRIC_URL/ELECTRIC_SECRET from shared secrets to the preview deploy-api step; requires those secrets to be available in the preview GitHub environment. |
| .github/workflows/cleanup-preview.yml | Removes the Fly.io Electric app teardown step; only Neon branch deletion remains, which is correct. |
| .github/templates/preview-comment.md | Removes the Electric row from the preview status table; remaining rows and template variables are consistent with the updated workflow. |
| fly.toml | Root fly.toml deleted — correct cleanup since the Fly.io Electric app no longer exists. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
subgraph prod["Deploy Production (after PR)"]
PDB[deploy-database] --> PAPI[deploy-api<br/>uses ELECTRIC_URL / ELECTRIC_SECRET]
PDB --> PWEB[deploy-web]
PDB --> PMKT[deploy-marketing]
PDB --> PADM[deploy-admin]
PDB --> PDOCS[deploy-docs]
PPRXY[deploy-electric-proxy<br/>Cloudflare Worker — unchanged]
end
subgraph preview["Deploy Preview (after PR)"]
DB[deploy-database<br/>Neon branch] --> API[deploy-api<br/>uses shared ELECTRIC_URL / ELECTRIC_SECRET]
DB --> WEB[deploy-web]
DB --> MKT[deploy-marketing]
DB --> ADM[deploy-admin]
DB --> DOCS[deploy-docs]
API & WEB & MKT & ADM & DOCS & DB --> CMT[post-final-comment]
end
subgraph removed["Removed"]
OLD1["❌ deploy-electric<br/>Fly.io app deploy (prod)"]
OLD2["❌ deploy-electric<br/>Fly.io app deploy (preview)"]
OLD3["❌ Fly.io teardown<br/>(cleanup-preview)"]
end
style removed fill:#fee,stroke:#f88
Prompt To Fix All With AI
This is a comment left during a code review.
Path: .github/workflows/deploy-preview.yml
Line: 162-163
Comment:
**Confirm `ELECTRIC_URL`/`ELECTRIC_SECRET` exist in `preview` environment**
The `deploy-api` job runs under `environment: preview` (line 83), so these two secrets must be defined in the **preview** GitHub environment — not just in `production`. If they were previously only set at the production environment level, preview API deploys will receive empty strings and Electric-dependent features will silently break in PR previews.
Worth double-checking that both `ELECTRIC_URL` and `ELECTRIC_SECRET` have been added to the `preview` environment in the repository's GitHub secrets settings before merging.
How can I resolve this? If you propose a fix, please make it concise.Reviews (1): Last reviewed commit: "chore(ci): drop Fly.io Electric deploys ..." | Re-trigger Greptile
| ELECTRIC_URL: ${{ secrets.ELECTRIC_URL }} | ||
| ELECTRIC_SECRET: ${{ secrets.ELECTRIC_SECRET }} |
There was a problem hiding this comment.
Confirm
ELECTRIC_URL/ELECTRIC_SECRET exist in preview environment
The deploy-api job runs under environment: preview (line 83), so these two secrets must be defined in the preview GitHub environment — not just in production. If they were previously only set at the production environment level, preview API deploys will receive empty strings and Electric-dependent features will silently break in PR previews.
Worth double-checking that both ELECTRIC_URL and ELECTRIC_SECRET have been added to the preview environment in the repository's GitHub secrets settings before merging.
Prompt To Fix With AI
This is a comment left during a code review.
Path: .github/workflows/deploy-preview.yml
Line: 162-163
Comment:
**Confirm `ELECTRIC_URL`/`ELECTRIC_SECRET` exist in `preview` environment**
The `deploy-api` job runs under `environment: preview` (line 83), so these two secrets must be defined in the **preview** GitHub environment — not just in `production`. If they were previously only set at the production environment level, preview API deploys will receive empty strings and Electric-dependent features will silently break in PR previews.
Worth double-checking that both `ELECTRIC_URL` and `ELECTRIC_SECRET` have been added to the `preview` environment in the repository's GitHub secrets settings before merging.
How can I resolve this? If you propose a fix, please make it concise.
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
There was a problem hiding this comment.
4 issues found across 4 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name=".github/templates/preview-comment.md">
<violation number="1">
P2: This change re-adds the Electric (Fly.io) preview row, which contradicts the Fly.io removal migration and keeps stale Electric status/link placeholders in the comment template.</violation>
</file>
<file name=".github/workflows/cleanup-preview.yml">
<violation number="1">
P1: This change reintroduces Fly.io cleanup into preview CI, undoing the intended Fly.io removal and re-adding a secret/infrastructure dependency the migration is trying to eliminate.</violation>
</file>
<file name=".github/workflows/deploy-preview.yml">
<violation number="1">
P1: This change reintroduces the Fly.io `deploy-electric` preview job and makes downstream status reporting depend on it, adding back the CI dependency that was meant to be removed.</violation>
<violation number="2">
P1: `deploy-api` now uses a hard-coded Fly.io Electric URL instead of repository secret configuration, which couples previews to Fly host naming and breaks secret-driven Electric endpoint management.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
a300506 to
cc6e24f
Compare
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/cleanup-preview.yml (1)
25-38:⚠️ Potential issue | 🔴 CriticalResolve leftover merge-conflict markers (workflow is currently invalid YAML).
Line 25 still contains Git conflict markers, which breaks parsing and blocks the workflow from running. It also reintroduces Fly.io cleanup steps this PR intends to remove.
Suggested fix
-<<<<<<< HEAD - - name: Setup Fly CLI - uses: superfly/flyctl-actions/setup-flyctl@ed8efb33836e8b2096c7fd3ba1c8afe303ebbff1 # 1.6 - - - name: Delete Electric Fly.io app - id: electric-cleanup - continue-on-error: true - env: - FLY_API_TOKEN: ${{ secrets.FLY_API_TOKEN }} - run: | - flyctl apps destroy "superset-electric-pr-${{ github.event.pull_request.number }}" --yes - -======= ->>>>>>> 96ea05cfe (chore(ci): drop Fly.io Electric deploys (use Electric Cloud))🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/cleanup-preview.yml around lines 25 - 38, Remove the Git merge-conflict markers (<<<<<<< HEAD, =======, >>>>>>>) and delete the entire conflicted Fly.io cleanup block that includes the steps named "Setup Fly CLI" and "Delete Electric Fly.io app" so the workflow YAML is valid and no longer reintroduces Fly.io cleanup steps; ensure the remaining workflow is syntactically correct YAML after removing those lines.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In @.github/workflows/cleanup-preview.yml:
- Around line 25-38: Remove the Git merge-conflict markers (<<<<<<< HEAD,
=======, >>>>>>>) and delete the entire conflicted Fly.io cleanup block that
includes the steps named "Setup Fly CLI" and "Delete Electric Fly.io app" so the
workflow YAML is valid and no longer reintroduces Fly.io cleanup steps; ensure
the remaining workflow is syntactically correct YAML after removing those lines.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9bf581e1-0c34-43b1-b0ea-55c7bc1a880d
📒 Files selected for processing (5)
.github/templates/preview-comment.md.github/workflows/cleanup-preview.yml.github/workflows/deploy-preview.yml.github/workflows/deploy-production.ymlfly.toml
💤 Files with no reviewable changes (3)
- .github/templates/preview-comment.md
- .github/workflows/deploy-production.yml
- fly.toml
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/deploy-preview.yml
Production Electric runs on Electric Cloud and local dev runs on Docker — the Fly.io `superset-electric` app no longer exists, so every deploy-electric run was failing on `flyctl secrets set --app superset-electric`. Remove the job, the preview Fly.io counterpart, and the root fly.toml. Point preview API at the shared ELECTRIC_URL/ELECTRIC_SECRET secrets.
cc6e24f to
c1f914f
Compare
Summary
deploy-electricjob indeploy-production.ymlhas been failing every run withCould not find App "superset-electric"because the Fly.io app no longer exists — we use Electric Cloud in prod and Docker locally.deploy-electricjob, its preview counterpart, the preview cleanup step, and the rootfly.toml. Point preview API at the sameELECTRIC_URL/ELECTRIC_SECRETsecrets already used in prod, and drop the Electric row from the preview comment template.deploy-electric-proxy) is untouched — that's the path desktop uses.Test plan
mainpush:deploy-electricjob is gone and the overall deploy-production workflow succeeds.deploy-electricjob, API deploy gets Electric env from shared secrets, and the preview comment renders without the Electric row.Summary by CodeRabbit
Summary by cubic
Dropped Fly.io Electric deploys from CI (prod and preview) to stop failing runs and use Electric Cloud instead. Preview API now uses shared
ELECTRIC_URLandELECTRIC_SECRET; the Cloudflare Electric proxy is unchanged.deploy-electricfromdeploy-production.yml.needs; switched API to shared secrets.fly.toml.cleanup-preview.yml.Written for commit c1f914f. Summary will update on new commits.