chore(ingestion): retire orphaned run-init.sh host-script family - #1486
Conversation
run-init.sh and its chain were a manual host bootstrap that the gitops
Hook-Job + Argo reconcile-cron model has fully obsoleted, and they no
longer even run (sync-flows.sh sourced airbyte-toolkit/, removed earlier).
The per-connector sync CronWorkflows they generated are now created and
maintained in-cluster by the reconcile loop (verified on the live cluster:
*-default-sync CronWorkflows are owned by insight-reconcile-loop, not
sync-flows.sh), and ClickHouse migrations moved to the clickhouse-migrate
Hook Job.
Remove (dead/broken):
- run-init.sh, scripts/init.sh, scripts/sync-flows.sh, update-workflows.sh,
workflows/schedules/sync.yaml.tpl
Keep (live manual one-shot tools, no broken deps):
- run-sync.sh, run-tt-enrich-jira.sh, workflows/onetime/, reconcile-connectors/
Fix references in current docs/code to point at reconcile-connectors.sh:
- src/ingestion/README.md, docs/domain/ingestion/{README,DESIGN,PRD,DECOMPOSITION},
docs/domain/identity-resolution/DESIGN, cypilot connector skill docs,
dbt_project.yml + reconcile-connectors/lib/airbyte.sh comments, .gitignore.
Mark superseded toolkit specs obsolete (historical, not rewritten):
- airbyte-toolkit PRD.md + DESIGN.md banners; ADR-0004 obsoleted-mechanics
note. Live reconcile ADRs/FEATURE left intact.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Anton Zelenov <antonz@constructor.tech>
📝 WalkthroughWalkthroughThe PR replaces documented host-side init and workflow update steps with ChangesIngestion reconcile docs migration
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
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 |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/components/airbyte-toolkit/specs/feature-reconcile/FEATURE.md`:
- Line 99: The current description for cpt-insightspec-actor-ci-pipeline
conflates the scheduled in-cluster CronWorkflow with the host-triggered one-shot
flow. Update the wording in the feature spec to clearly separate these paths:
the CronWorkflow schedules in-cluster reconcile runs, while the host triggers
the separate reconcile/sync execution path. Use the
cpt-insightspec-actor-ci-pipeline entry as the anchor and adjust the sentence so
it no longer implies the CronWorkflow itself is invoked on demand from the host.
In `@docs/domain/ingestion/specs/DECOMPOSITION.md`:
- Around line 295-299: The spec entry points to the wrong template location
because it omits the `src/ingestion/` prefix. Update the reference in the
reconciliation section to use the correct template path from the repository,
keeping the surrounding bullets intact and ensuring the `reconcile-connectors`
template location is accurate for readers.
In `@docs/domain/ingestion/specs/PRD.md`:
- Around line 630-634: The reconcile flow description overstates what the script
does by saying it creates the destination. Update the “Main Flow” wording to
reflect that the reconcile path in the connector reconcile script and
CronWorkflow resolves the destination ID, but only creates the source and
connection when needed. Keep the description aligned with the actual reconcile
behavior in the PRD section so it matches the
`reconcile-connectors.sh`/CronWorkflow flow.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7e2fbc06-da1e-466f-b4ad-7611c8a96dd8
📒 Files selected for processing (20)
cypilot/.core/skills/connector/workflows/deploy.mdcypilot/.core/skills/connector/workflows/workflow.mddocs/components/airbyte-toolkit/specs/ADR/0004-cluster-config-via-configmap.mddocs/components/airbyte-toolkit/specs/DESIGN.mddocs/components/airbyte-toolkit/specs/PRD.mddocs/components/airbyte-toolkit/specs/feature-reconcile/FEATURE.mddocs/domain/identity-resolution/specs/DESIGN.mddocs/domain/ingestion/README.mddocs/domain/ingestion/specs/DECOMPOSITION.mddocs/domain/ingestion/specs/DESIGN.mddocs/domain/ingestion/specs/PRD.mdsrc/ingestion/.gitignoresrc/ingestion/README.mdsrc/ingestion/dbt/dbt_project.ymlsrc/ingestion/reconcile-connectors/lib/airbyte.shsrc/ingestion/run-init.shsrc/ingestion/scripts/init.shsrc/ingestion/scripts/sync-flows.shsrc/ingestion/update-workflows.shsrc/ingestion/workflows/schedules/sync.yaml.tpl
💤 Files with no reviewable changes (5)
- src/ingestion/scripts/init.sh
- src/ingestion/update-workflows.sh
- src/ingestion/scripts/sync-flows.sh
- src/ingestion/workflows/schedules/sync.yaml.tpl
- src/ingestion/run-init.sh
The ingestion README still pointed at removed scripts/dirs after the
host-script retirement + reconcile refactor. Fix the verifiable rot:
- airbyte-toolkit/cdk-build.sh → reconcile-connectors/lib/cdk-build.sh
- airbyte-toolkit/lib/env.sh → reconcile-connectors/lib/env.sh
- update-connectors/-connections/-workflows.sh → reconcile-connectors.sh
- run-init.sh → reconcile-connectors.sh (+ migrations
are a clickhouse-migrate Hook Job)
- secrets/validate.sh → removed (validation is internal to
reconcile: valsec_check_secret)
- namespace `data` → `insight` (single-namespace umbrella)
- drop "done automatically by up.sh" (up.sh removed)
- Project Structure tree aligned to actual src/ingestion contents
Left for a product-aware follow-up (flagged, not guessed): NodePort access
URLs (localhost:30500/30123), "Kind"-specific local-dev wording, the
tenant_id ConfigMap mechanism, and the ClickHouse debug snippets that
assume a bundled in-namespace CH (now external in insight-infra).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Anton Zelenov <antonz@constructor.tech>
- FEATURE.md: separate the scheduled in-cluster CronWorkflow path from the host-run reconcile command (the CronWorkflow isn't host-invoked). - DECOMPOSITION.md: correct cron-workflow template path to the full src/ingestion/reconcile-connectors/templates/ location. - PRD.md: the reconcile step creates source + connection; the shared bronze destination is resolved (created once) per ADR-0012, not created per-connector. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Anton Zelenov <antonz@constructor.tech>
|
|
||
| ```bash | ||
| ./update-workflows.sh <tenant> | ||
| ./reconcile-connectors.sh --connector <name> |
There was a problem hiding this comment.
i'm not sure we have such script, but ok for now
There was a problem hiding this comment.
Yep, it is reconcile.sh, i will fix
There was a problem hiding this comment.
lol, this name is used over repo, let it be separate fix
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/ingestion/README.md`:
- Line 28: The README and the reconcile script disagree on the namespace for the
insight-config ConfigMap. Update the documentation in README to match the actual
behavior of reconcile-connectors/main.sh, or change the script if the intended
default namespace is now insight; make sure the namespace referenced alongside
INSIGHT_TENANT_ID and the connector reconciliation flow is consistent
everywhere.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 30a148a2-d793-4ade-9712-923925e2f45b
📒 Files selected for processing (4)
docs/components/airbyte-toolkit/specs/feature-reconcile/FEATURE.mddocs/domain/ingestion/specs/DECOMPOSITION.mddocs/domain/ingestion/specs/PRD.mdsrc/ingestion/README.md
✅ Files skipped from review due to trivial changes (3)
- docs/components/airbyte-toolkit/specs/feature-reconcile/FEATURE.md
- docs/domain/ingestion/specs/DECOMPOSITION.md
- docs/domain/ingestion/specs/PRD.md
run-init.sh and its chain were a manual host bootstrap that the gitops Hook-Job + Argo reconcile-cron model has fully obsoleted, and they no longer even run (sync-flows.sh sourced airbyte-toolkit/, removed earlier). The per-connector sync CronWorkflows they generated are now created and maintained in-cluster by the reconcile loop (verified on the live cluster: *-default-sync CronWorkflows are owned by insight-reconcile-loop, not sync-flows.sh), and ClickHouse migrations moved to the clickhouse-migrate Hook Job.
Remove (dead/broken):
Keep (live manual one-shot tools, no broken deps):
Fix references in current docs/code to point at reconcile-connectors.sh:
Mark superseded toolkit specs obsolete (historical, not rewritten):
Summary by CodeRabbit