Skip to content

Distinguish "not supported" from "not deployed" in bundle summary - #6583

Open
janniklasrose wants to merge 26 commits into
mainfrom
janniklasrose/summary-url-not-supported
Open

Distinguish "not supported" from "not deployed" in bundle summary#6583
janniklasrose wants to merge 26 commits into
mainfrom
janniklasrose/summary-url-not-supported

Conversation

@janniklasrose

@janniklasrose janniklasrose commented Sep 9, 2026

Copy link
Copy Markdown
Member

resource.GetURL() == "" did not distinguish between "not deployed yet" and "resource does not support URLs".

Add a boolean return value to the method to allow callers (bundle summary, bundle open, pipelines open pipelines deploy) to adjust the messaging.

janniklasrose and others added 22 commits August 31, 2026 11:57
## Changes

Bump `github.com/databricks/databricks-sdk-go` from v0.175.0 to v0.177.0
and the pinned OpenAPI spec SHA to `0f2c9a3d`.

Regenerated all downstream artifacts from the new spec: CLI command stubs,
bundle JSON schema, validation, direct-engine resources, pydabs models, and
the DABs<->TF field map. User-visible additions include the jobs continuous
`maintenance_window` field and the pipelines RabbitMQ connector options; the
postgres branch `source_snapshot` field is now handled as immutable/input-only
by the direct engine.

The v0.177.0 `ml.*` field removals (feature-engineering) are not referenced by
hand-written CLI code, so they required no source changes. The SDK now marks
the pipelines `continuous` field deprecated; it remains a supported bundle
config field, so its four usages carry `//nolint:staticcheck` (SA1019).

## Why

Keep the CLI current with the latest SDK and provide a clean base for
upcoming feature work.

## Tests

Acceptance goldens regenerated via `-update` (help text + refschema). The
non-update verify pass is green apart from load-induced terraform-engine
timeout flakes, each confirmed passing in isolation. Root-module unit suite
passes (9532 tests). `./task fmt`, `./task lint`, and `./task checks` clean.

Co-authored-by: Isaac <no-reply@databricks.com>
## Changes

Add the `postgres_snapshot_schedules` bundle resource, which manages the
automatic-snapshot schedule of a Lakebase Postgres branch.

The snapshot schedule is a per-branch singleton with no create or delete API —
only `GetSnapshotSchedule` and `UpdateSnapshotSchedule`. The resource maps the
lifecycle onto that single write call:

- create/update set the branch's schedule via `UpdateSnapshotSchedule`
  (update_mask `schedule`, awaiting the long-running operation);
- delete disables automatic snapshots by setting an empty cadence set, since
  DoDelete also fires when the resource is removed from config, not only on
  `bundle destroy`;
- `branch` composes the schedule's hierarchical name and is a provided id field
  (a change recreates).

Modeled on the existing `postgres_*` resources. Direct engine only: the schedule
was added in databricks-sdk-go v0.177.0 and the pinned Terraform provider has no
equivalent resource yet, so the acceptance and bind tests are pinned to the
direct engine.

## Tests

- direct-engine unit CRUD (`bundle/direct/dresources`);
- acceptance (direct engine), four end-to-end scenarios, each reading the
  schedule back with `postgres get-snapshot-schedule`:
  - `basic`: create with a cadence, in-place cadence update, then remove the
    resource from config, which disables the schedule (reads back
    `schedule: null`);
  - `update`: add the schedule to an already-deployed branch, then remove it;
  - `out-of-band`: an out-of-band `update-snapshot-schedule` is detected as
    drift by `bundle plan` (`update postgres_snapshot_schedules.main_schedule`)
    and reconciled back by deploy;
  - `orphaned`: removing the branch from config while keeping the schedule that
    references it — the reference still resolves from the deployed state, so
    validate passes and plan sequences a branch delete alongside a schedule
    recreate (documented, not a validation error);
- the invariant and bind/unbind suites;
- resource-enumeration unit tests (bind support, run_as, permissions,
  target-mode) extended for the new type.

Acceptance goldens regenerated via `-update`; the verify pass is green apart
from pre-existing load-induced terraform-engine timeout flakes, each confirmed
passing in isolation.

Co-authored-by: Isaac <no-reply@databricks.com>
Drop the `update` subtest (create/update/remove is already covered by
`basic`) and the `orphaned` subtest (deleting a branch while keeping a
schedule that references it is user error — it plans but fails at apply,
which we don't need to pin in a golden).

Co-authored-by: Isaac <no-reply@databricks.com>
DoCreate built the schedule resource name as `config.Branch + "/snapshot-schedule"`,
which doubles the separator if a user writes `branch:` with a trailing slash.
Route it through snapshotScheduleName, which trims trailing slashes first, and
cover it with a unit test.

Co-authored-by: Isaac <no-reply@databricks.com>
Add a `bundle plan` after the removal step in the basic acceptance test; it
reports "0 to add, 0 to change, 0 to delete" (project and branch unchanged),
confirming the disable settled cleanly and leaves no lingering drift.

Co-authored-by: Isaac <no-reply@databricks.com>
…s-snapshot-schedules

# Conflicts:
#	.nextchanges/dependency-updates/bump-sdk-0.177.0.md
#	bundle/terraform_dabs_map/generated.go
#	go.mod
Completes wiring for the postgres_snapshot_schedules resource so it
satisfies the repo-wide "for all resources" guard tests:

- Add MarshalJSON/UnmarshalJSON to resources.PostgresSnapshotSchedule so
  the outer wrapper's id/url/lifecycle/modified_status survive a JSON
  round-trip instead of being dropped by the embedded config marshaler
  (mirrors resources.PostgresBranch).
- Pin the resource to Beta via launchStageOverrides and cover it in the
  launch-stage override test.
- Populate PostgresSnapshotSchedules in the StateToBundle fixtures.
- Mark it direct-engine only in the Terraform lifecycle test; the pinned
  provider has no resource that maps to a snapshot schedule.

Regenerate the resource's out.test.toml snapshots to pick up the DMS
EnvMatrix inherited from a parent test.toml.

Co-authored-by: Isaac <no-reply@databricks.com>
Add the required bullet marker and trailing PR link so the fragment
passes check-changelog.

Co-authored-by: Isaac <no-reply@databricks.com>
The snapshot schedule has no web UI page of its own. Per agreement with
the Postgres team, point its URL at the parent branch's restore view:
lakebase/projects/{project_id}/branches/{branch_id}/restore, resolved
from the resource's branch. Covered in TestInitializeURLs and reflected
in the bind acceptance golden.

Co-authored-by: Isaac <no-reply@databricks.com>
Register a postgres_snapshot_schedules pattern (lakebase/%s/restore, %s =
parent branch) and use workspaceurls.ResourceURL in InitializeURL instead
of hand-assembling the path, matching every other resource.

Co-authored-by: Isaac <no-reply@databricks.com>
Adding postgres_snapshot_schedules to workspaceurls broke tests that
assert the exact ResourceTypes() list (cmd/experimental open command and
its acceptance golden); update them.

Also derive the branch from the schedule's resolved ID rather than the
Branch field, which may hold an unresolved "${...}" reference at summary
time. This matches the other parent-referencing postgres resources and
correctly shows "(not deployed)" until the schedule has an ID.

Co-authored-by: Isaac <no-reply@databricks.com>
It now has a workspace URL, so TestBundleResourcePluralNamesResolveInWorkspaceURLs
requires it to resolve rather than be empty.

Co-authored-by: Isaac <no-reply@databricks.com>
Main's new TestMarshalerValueReceiver requires marshalling by value and by
pointer to agree; match the sibling postgres configs.

Co-authored-by: Isaac <no-reply@databricks.com>
The Lakebase web UI addresses resources by uid, not by the resource name
in bundle state, so the name-based URL was wrong. Revert to no URL (back
in the noURL allowlist) until a uid-based scheme is designed.

Co-authored-by: Isaac <no-reply@databricks.com>
The backend rejects a snapshot schedule on any non-root branch ("not
allowed to snapshot non-root branch"), which surfaced only on cloud. Teach
the testserver the same rule, and point the acceptance and CRUD fixtures at
the project's implicit root "production" branch instead of a child branch.

Co-authored-by: Isaac <no-reply@databricks.com>
The schedule's resource name is its ID ("{branch}/snapshot-schedule");
return it from GetName so bundle summary shows it instead of a blank,
matching the SecretScope precedent (per review feedback).

Co-authored-by: Isaac <no-reply@databricks.com>
…s-snapshot-schedules

# Conflicts:
#	bundle/config/resources.go
#	bundle/config/resources_test.go
#	bundle/direct/dresources/all_test.go
The Python codegen had no mapping for duration.Duration (the schedule's
retention type), failing pydabs-codegen with a KeyError. Treat it as a
str-serialized primitive, matching time.Time (PRIMITIVES + RENAMES + the
test-case synthesizer), and regenerate the PyDABs module. Add the required
acceptance/bundle/python support fixture and refresh the core public-API
snapshot.

Co-authored-by: Isaac <no-reply@databricks.com>
Resource types that never expose a workspace URL (the postgres_* resources, secret scopes, external locations, snapshots) rendered "URL:  (not deployed)" in `bundle summary` even after a successful deploy, which was misleading.

Add a type-level SupportsURL() to the ConfigResource interface (default true on BaseResource, overridden to false on the no-URL types) and render "(not supported)" for those, while keeping "(not deployed)" for URL-capable resources that are not deployed yet.

Co-authored-by: Isaac <no-reply@databricks.com>
Co-authored-by: Isaac <no-reply@databricks.com>
Base automatically changed from janniklasrose/postgres-snapshot-schedules to main September 9, 2026 07:53
…-url-not-supported

# Conflicts:
#	acceptance/bundle/deployment/bind/postgres_snapshot_schedule/output.txt
#	bundle/config/resources/postgres_snapshot_schedule.go
@eng-dev-ecosystem-bot

eng-dev-ecosystem-bot commented Sep 9, 2026

Copy link
Copy Markdown
Collaborator

Integration test report

Commit: feb4032

Run: 34350575021

Env ✅​pass 🙈​skip Time
✅​ aws linux 304 18 6:19
✅​ aws windows 306 16 7:28
✅​ azure linux 275 25 7:03
✅​ azure windows 277 23 6:10
✅​ gcp linux 276 25 5:57
✅​ gcp windows 278 23 4:51
Top 6 slowest tests (at least 2 minutes):
duration env testname
5:34 azure windows TestAccept
3:57 aws windows TestAccept
3:45 gcp windows TestAccept
2:10 aws linux TestAccept
2:09 gcp linux TestAccept
2:07 azure linux TestAccept

bundle open and pipelines open told the user a no-URL resource might just need deploying ("has it been deployed?"), and pipelines deploy printed "View your <resource> here:" with a blank URL for such resources. Guard all three on SupportsURL() so the message is honest and the blank line is skipped.

Co-authored-by: Isaac <no-reply@databricks.com>
Comment thread bundle/config/resources.go Outdated
Comment on lines +69 to +72
// SupportsURL reports whether this resource type can have a workspace URL.
// It distinguishes an empty GetURL() that means "not deployed yet" from one
// that means the type never has a URL.
SupportsURL() bool

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

alternative: GetURL() (string, bool)

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.

slight preference for this, no chance of two methods going out of sync.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

yep, already refactored :-) also makes it clear for the caller of GetURL

database1:
Name:
URL: (not deployed)
URL: (not supported)

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.

This could be read a bit like resource is not supported (e.g. engine limitation), not that URL is not supported.

Maybe it should be "(unknown)" or "(not available)".

Actually, why show "URL:" row at all for such cases? Cannot we just omit it?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I think any string has the potential to be misunderstood. +1 to skip the line for resources that don't support it

return ""
}

func (b *PostgresSnapshotSchedule) SupportsURL() bool {

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.

Is GetURL() returning "" not enough?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

GetURL() == "" currently means either not supported (InitializeURL is no-op) or not deployed yet (InitializeURL guarded on e.g. r.ID being empty)

Address review: rather than a separate SupportsURL() method, GetURL now returns (url, supported), following the (value, ok) idiom (cf. context.Context.Deadline). The default lives on BaseResource like GetLifecycle, so the 27 trivial 'return X.URL' overrides are dropped and only the 9 no-URL types override to return ("", false).

Also consolidate the changelog into a single generic entry covering all affected commands.

Co-authored-by: Isaac <no-reply@databricks.com>
Per review: instead of printing a placeholder like "(not supported)" (which reads ambiguously), the summary template now skips the URL line entirely when GetURL reports the type has no URL. URL-capable resources are unchanged, including the "(not deployed)" case.

Co-authored-by: Isaac <no-reply@databricks.com>
@@ -18,15 +18,12 @@ Resources:
Postgres branches:
main:
Name:

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.

out of scope for this PR, but related theme: why do we show Name: without a value there? Is it because there is no Name or it not known yet?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Taking a look as follow-up

@denik

denik commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

description is out of date and overly verbose

@janniklasrose
janniklasrose marked this pull request as ready for review September 9, 2026 12:37
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