Repository navigation
fix: install charts from the v2 repo, keep v1 reachable with a warning - #199
Conversation
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Runtime test results (Devin) — PLAT-1006Built from ✅ Default install now resolves chart V2 (2.1.1), no deprecation warning
✅
|
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Aldo Gonzalez (aldogonzalez8)
left a comment
There was a problem hiding this comment.
Approved. The V2 default and V1 fallback look good. I left one non-blocking comment about resolving the chart before creating external resources.
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
|
/claude-review |
Summary
PLAT-1006, steps 1-3 only. abctl defaults to chart V2 but three code paths still pointed at the V1 repo. This moves them to
airbytehq.github.io/chartsand warns on V1, while deliberately keeping V1 installable — V1 chart users are still live (airbyte/servertags1.5.1,1.6.0,1.7.0,1.8.1were each pulled from DockerHub within the last hour), so bothabctl local install --chart-version 1.9.2and a V1 dataplane install must keep working until the V1 Pages site is actually retired.The dataplane install was the real bug. It was pinned to a V1 chart that will never be updated again:
The comment was simply stale —
airbytehq/chartspublishesairbyte-data-plane(currently2.1.1). The default is now the latest stable V2 chart resolved from the index rather than a new hard-coded pin, since going stale is what caused this bug; that matches what the platform chart already does (ResolveChartReference("", "")).Both chart generations stay reachable for the dataplane.
abctl install dataplanegains a--chart-versionflag, and repo selection mirrorsResolveChartReference: empty resolves latest V2,>= 2.0.0builds a V2 URL, anything lower falls back to the V1 repo. The repo entry registered with Helm follows the same choice, so a V1 install doesn't register a V2 remote.Pre-release suffixes are stripped before the comparison, so
2.2.0-rc1still resolves against V2.V1 deprecation warnings are printed from the command layer, not
internal/helm— the resolver shouldn't print UI. Forlocal installit lives inInstallCmd.setDefaultChartFlagsafter chart resolution, so it fires on any route to a sub-2.0.0 chart (explicit--chart-version 1.x, or a local/URL chart whose metadata reports 1.x), guarded on a non-empty version so an unresolvable version doesn't produce a misleading line. For the dataplane it fires before the install spinner, only on an explicitly requested V1 version, since the default path can never resolve V1.internal/service/install.gowas cosmetic but wrong. It passedAirbyteRepoURLv1asrepoURL. That value only feedsAddOrUpdateChartRepo; the already-resolvedchartLocURL determines what actually gets fetched, so no user was installing a V1 chart because of it — but it did register the wrong repo in the user's Helm config. Now V2.Out of scope, gated on V1 Pages retirement (ticket step 4): the
< 2.0.0route inResolveChartReference, theAirbyteRepoURLv1constant, the V1 branch ininternal/helm/images.go, and the V1 URL fixtures all stay.Verification
go build ./...,go test ./...pass;go vetclean on the changed packages.TestResolveDataplaneChartReferencecovers all four repo-selection cases (default, V2, V1, pre-release).TestInstallDataplaneChartnow table-drives the chart URL/version and adds an explicit-V1 case asserting both the V1 chart URL and the V1 repo entry.TestGetLatestChartURLFromRepoIndexcoversairbyte-data-planeresolution against a fake index, including that a prerelease entry is skipped.TestCommand_Install_HappyPathexpects the V2 repo URL inAddOrUpdateChartRepo.https://airbytehq.github.io/helm-charts/airbyte-data-plane-1.8.1.tgzstill returns 200, so the V1 fallback URL is live.Note for reviewers:
make fmtreformats a dozen unrelated files (pre-existing whitespace drift onmain); those are left untouched here.make vetover the whole repo fails on a pre-existinginternal/airbyte/log_scanner.go:90struct-tag issue, also unrelated.Requested by divya-airbyte (@dgplatform26).
Link to Devin session: https://app.devin.ai/sessions/62889b8a90264973a96f2155a3df178c