fix(ci): repair main — driver registration, GORM column name, missing ECR repos#507
Merged
fuziontech merged 2 commits intomainfrom May 1, 2026
Merged
fix(ci): repair main — driver registration, GORM column name, missing ECR repos#507fuziontech merged 2 commits intomainfrom
fuziontech merged 2 commits intomainfrom
Conversation
… ECR repos Three independent regressions had left main in a sorry state. All show up on every push and bury real signal under noise: 1) integration-tests: tests/integration/clients was missing the duckdbservice blank import after the duckdb driver registration moved out of server/. Every connection that the client harness opened failed with `unknown driver "duckdb" (forgotten import?)`. Adds the same blank import that tests/integration/harness.go already carries. 2) controlplane-k8s-tests: PR #506 added "ducklake_version" to the managedWarehouseUpsertColumns list, but GORM's default naming strategy splits CamelCase on every uppercase-after-lowercase boundary, so the actual Postgres column is `duck_lake_version`, not `ducklake_version`. The ON CONFLICT … DO UPDATE clause threw 42703 on every UPSERT. Switches the upsert column name to match the real DB column and updates the regression-guard test to assert the same. 3) Container Image Worker CD + Controlplane CD: both workflows push to `795637471508.dkr.ecr.us-east-1.amazonaws.com/duckgres-{worker,controlplane}` but those ECR repos do not yet exist in the AWS root account — only `duckgres` is provisioned in posthog-cloud-infra's ecr.tf. Every push to main went red on these two CDs. Disables ECR push (and the AWS credential / ECR-login steps) on both workflows; GHCR push remains. When posthog-cloud-infra adds `module "ecr_duckgres_worker"` and `module "ecr_duckgres_controlplane"`, re-enable the AWS auth + ECR tags. Comments at the top of each workflow capture this. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…re format works Pre-existing CI regression on main since #494 (the value-normalizer hook): the DECIMAL path goes duckdb.Decimal → arrowmap.DecimalValue (via the registered normalizer) → server.formatValue. formatValue has explicit cases for IntervalValue / OrderedMapValue but lets DecimalValue fall through to the default fmt.Sprintf("%v", val) — which renders the Go struct as "{314159 5}" instead of "3.14159". Result: every DECIMAL query in TestDQLBasicSelect/select_float and any downstream test that compares decimals as floats fails — integration-tests has been red on main since 28a1ff3. Adding String() on DecimalValue is the right shape: the binary numeric encoder in server/types.go already handles this type explicitly, so the text path is the one that's broken. Providing String() also makes any future fmt.Sprint-based callers (logging, error messages) safe by default rather than requiring them to add a switch case. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
3 tasks
fuziontech
added a commit
that referenced
this pull request
May 1, 2026
…CDs (#509) Reverts the GHCR-only workaround from PR #507 now that the ECR repos exist. posthog-cloud-infra#7848 merged and the aws-accnt-root apply ran at 19:40 UTC, creating both `duckgres-worker` and `duckgres-controlplane` in account 795637471508 along with their cross-account pull policies. Restores in both workflows: - ECR_REGISTRY env var - aws-actions/configure-aws-credentials + amazon-ecr-login steps in build and manifest jobs - ECR tags in the docker/build-push-action tags list (so per-arch images push to both ECR and GHCR) - ECR docker buildx imagetools create calls in manifest assembly - For the worker default-version case, retag both ECR <sha>/latest and GHCR <sha>/latest Strips the temporary "ECR push is intentionally disabled" notes from both file headers. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
CI on
mainhad three independent regressions stacking up and burying real signal:integration-tests(CI workflow):tests/integration/clientswas missing the_ "github.com/posthog/duckgres/duckdbservice"blank import after the duckdb driver registration moved out ofserver/(during the binary-split work). Every connection opened by the client harness died withunknown driver "duckdb" (forgotten import?). Fixed by adding the import thattests/integration/harness.goalready had.controlplane-k8s-tests(CI workflow): PR feat(admin): PATCH /orgs/:id/warehouse/pinning for tenant image+ducklake #506 added"ducklake_version"tomanagedWarehouseUpsertColumns()— but GORM's default naming strategy turnsDuckLakeVersionintoduck_lake_version(splits at every uppercase-after-lowercase). TheEXCLUDED.ducklake_versionreference in the ON CONFLICT clause threw42703 column does not existon every UPSERT. Verified locally with the actual GORMNamingStrategy.ColumnName. Fix uses the real DB column name and updates the regression-guard test to assert the same.Container Image Worker CD+Container Image Control Plane CD: both workflows push to ECR repos that don't yet exist in the AWS root account (duckgres-worker/duckgres-controlplane); onlyduckgresis provisioned in posthog-cloud-infra'secr.tf. Every push tomainwent red. This PR disables ECR push (and the AWS-credentials / ECR-login steps) in both workflows; GHCR push remains. Re-enable whenmodule "ecr_duckgres_worker"andmodule "ecr_duckgres_controlplane"are added toterraform/environments/aws-accnt-root/ecr.tf— workflow comments capture this.Test plan
go test -tags kubernetes ./controlplane/admin/...clean (column-name regression test passes)go build ./tests/integration/clients/...clean (driver import resolves)DuckLakeVersion → duck_lake_versionFollow-ups (not in this PR)
posthog-cloud-infraaddingmodule "ecr_duckgres_worker"andmodule "ecr_duckgres_controlplane"interraform/environments/aws-accnt-root/ecr.tf, then re-enable ECR push in the two workflow files here.🤖 Generated with Claude Code