Skip to content

Add postgres_catalogs bundle resource#5265

Merged
pietern merged 8 commits into
mainfrom
postgres-catalog
May 20, 2026
Merged

Add postgres_catalogs bundle resource#5265
pietern merged 8 commits into
mainfrom
postgres-catalog

Conversation

@pietern
Copy link
Copy Markdown
Contributor

@pietern pietern commented May 18, 2026

Changes

New postgres_catalogs resource binding a Unity Catalog catalog to a Postgres database on a Lakebase Autoscaling branch. Supported on both direct and terraform deployment engines.

Tests

Acceptance coverage: basic and recreate exercise each engine, plus the existing no_drift and migrate invariants pick up the new resource. Both engines produce identical human-readable output and identical wire bodies; only the captured request streams diverge by filename (out.requests.{direct,terraform}.json).

Verified end to end on a live workspace: the bundle deploys a project and catalog, a row written directly into the bound Postgres database becomes visible through the UC federated view, and a follow-up write shows up on re-read.

This pull request and its description were written by Isaac.

pietern added 2 commits May 18, 2026 22:35
## Changes

New `postgres_catalogs` resource binding a Unity Catalog catalog to a Postgres database on a Lakebase Autoscaling branch. Supported on both direct and terraform deployment engines.

The spec fields are classified as both `recreate_on_changes` and `ignore_remote_changes: input_only`. The two cover orthogonal diffs the planner runs — recreate fires on local edits to an immutable field, and ignore_remote silences the phantom drift from GET not echoing spec back today. Lift the `input_only` entries once the backend starts returning spec.

## Tests

Acceptance coverage: `basic` and `recreate` exercise each engine, plus the existing `no_drift` and `migrate` invariants pick up the new resource. Both engines produce identical human-readable output and identical wire bodies; only the captured request streams diverge by filename (`out.requests.{direct,terraform}.json`).

Verified end to end on a live workspace: the bundle deploys a project and catalog, a row written directly into the bound Postgres database becomes visible through the UC federated view, and a follow-up write shows up on re-read.

This pull request and its description were written by Isaac.
This pull request and its description were written by Isaac.
@pietern pietern temporarily deployed to test-trigger-is May 18, 2026 20:37 — with GitHub Actions Inactive
@pietern pietern temporarily deployed to test-trigger-is May 18, 2026 20:37 — with GitHub Actions Inactive
The hand-written `branch`, `postgres_database`, and `create_database_if_missing` entries under `ignore_remote_changes` are already produced by the OpenAPI autogen (`spec:input_only`); `postgres_database` is also autogen'd under `recreate_on_changes` (`spec:immutable`). Drop the duplicates from `resources.yml`.

`catalog_id` was in `ignore_remote_changes` only to mask the cosmetic `catalogs/` prefix that the old `RemapState` propagated from `remote.Name`. Source it from `remote.Status.CatalogId` instead — semantic contract from the API rather than string manipulation on the hierarchical path — and drop the entry. `catalog_id` stays in `recreate_on_changes` (synthetic hierarchical key, not in the API spec) along with `branch` and `create_database_if_missing` (no UpdateCatalog endpoint).

Co-authored-by: Isaac
@pietern pietern temporarily deployed to test-trigger-is May 19, 2026 10:02 — with GitHub Actions Inactive
@pietern pietern temporarily deployed to test-trigger-is May 19, 2026 10:02 — with GitHub Actions Inactive
@pietern pietern requested review from denik and janniklasrose and removed request for denik May 19, 2026 11:18
return "", nil, err
}

result, err := waiter.Wait(ctx)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can we put waiting into to WaitAfterCreate()? I understand we cannot use waiter convenience wrapper, but I'm sure there is API.

Splitting ensures that there is no orphaned resources if the process crashes during waiting.

The medium term plan is to move waiting into framework, splitting now would help.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Similar discussion here: #4423 (comment)

The outcome there was to do a follow-up but that didn't happen.

I'll file a ticket for this on direct engine to solve this generically for LROs.

Comment thread acceptance/bundle/refschema/out.fields.txt Outdated
Comment thread acceptance/bundle/resources/postgres_catalogs/basic/out.requests.terraform.json Outdated
Copy link
Copy Markdown
Contributor

@andrewnester andrewnester left a comment

Choose a reason for hiding this comment

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

Do we support binding the catalogs? If so, worth adding an acceptance test as well

Direct and terraform engines produced identical output. Per the repo
rule in .agent/rules/testing.md, only diverging files should be split
into per-engine variants; this matches the precedent set by
postgres_projects/basic.

Co-authored-by: Isaac
pietern added 2 commits May 19, 2026 16:37
Apply the same hoist that #5273 does for Branch/Endpoint/Project. Define
PostgresCatalogRemote that embeds CatalogCatalogSpec and exposes the
identifier and output-only fields at the top level. DoRead returns the
new shape so state-side and remote-side paths line up, which is a
prerequisite for drift detection on spec fields once the backend
echoes spec on GET. Today the embedded fields are auto-classified
spec:input_only from the API field behaviors in resources.generated.yml,
so drift is correctly suppressed.

Prompted by #5265 (comment).
Should land after #5273.

Co-authored-by: Isaac
# Conflicts:
#	NEXT_CHANGELOG.md
#	libs/testserver/fake_workspace.go
@pietern pietern temporarily deployed to test-trigger-is May 19, 2026 14:46 — with GitHub Actions Inactive
@pietern pietern temporarily deployed to test-trigger-is May 19, 2026 14:46 — with GitHub Actions Inactive
These annotations crept into the schema regeneration when the
postgres_catalogs commit (3857f0f) was first generated; they belong
to other fields and should not ride along with this PR. Keep only the
postgres_catalogs additions.

Co-authored-by: Isaac
# Conflicts:
#	acceptance/bundle/refschema/out.fields.txt
@pietern pietern temporarily deployed to test-trigger-is May 20, 2026 08:29 — with GitHub Actions Inactive
@pietern pietern temporarily deployed to test-trigger-is May 20, 2026 08:29 — with GitHub Actions Inactive
@pietern
Copy link
Copy Markdown
Contributor Author

pietern commented May 20, 2026

@andrewnester Bind/unbind is not explicitly tested yet for Postgres resources.

It should work just not tested.

@pietern pietern added this pull request to the merge queue May 20, 2026
Merged via the queue into main with commit c62b641 May 20, 2026
23 checks passed
@pietern pietern deleted the postgres-catalog branch May 20, 2026 11:24
pietern added a commit that referenced this pull request May 20, 2026
- Drop the duplicate postgres_catalogs block that the merge pulled in
  alongside the existing one in resources.yml.
- Remove postgres_catalogs from knownMissingInRemoteType now that the
  new PostgresCatalogRemote shim from #5265 surfaces the spec fields.

Co-authored-by: Isaac
pietern added a commit that referenced this pull request May 20, 2026
Adopt the same embedded-spec Remote pattern that #5273 / #5265 introduced
for postgres_catalogs: PostgresSyncedTableRemote embeds SyncedTableSyncedTableSpec
plus output-only fields, so every StateType path is also a valid RemoteType
path. RemapState just copies the embedded shape; drift on spec fields is
suppressed via the spec:input_only classifications generated from the
OpenAPI schema until GET starts echoing the spec.

Drop the now-empty postgres_synced_tables entry from
knownMissingInRemoteType, and regenerate acceptance/bundle/refschema/out.fields.txt
so the embedded spec fields show up as ALL rather than INPUT|STATE.

Co-authored-by: Isaac
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants