Skip to content

resources: centralise InitializeURL through libs/workspaceurls#5346

Merged
janniklasrose merged 2 commits into
mainfrom
janniklasrose/centralise-url-handling
May 27, 2026
Merged

resources: centralise InitializeURL through libs/workspaceurls#5346
janniklasrose merged 2 commits into
mainfrom
janniklasrose/centralise-url-handling

Conversation

@janniklasrose
Copy link
Copy Markdown
Contributor

@janniklasrose janniklasrose commented May 27, 2026

Changes

Move the 11 inline resource URL builders (catalogs, schemas, volumes, database_catalogs, database_instances, postgres_catalogs, postgres_synced_tables, quality_monitors, synced_database_tables, vector_search_endpoints, vector_search_indexes) onto workspaceurls.ResourceURL, joining the ones (jobs, pipelines, apps, ...) that already used it. Each resource's InitializeURL is now a one-liner; resource-specific guards (ModifiedStatus on DatabaseInstance, 3-part name check on PostgresSyncedTable and VectorSearchIndex) stay in the resource.

The new patterns and dotSeparatedResources flags live in libs/workspaceurls/urls.go, which also drives the experimental open command — its supported-type list grows from 13 to 24. Acceptance snapshot for experimental/open regenerated to match.

One behaviour delta on postgres_synced_tables / vector_search_indexes pre-deploy: the previous strings.Cut-twice logic produced a malformed URL like [DATABRICKS_URL]/explore/data/$%7Bresources/postgres_catalogs/my_catalog.catalog_id%7D.public.trips_synced when the name still contained an unresolved ${resources.X.Y} reference. The new strict 3-part check leaves the URL empty, so summary now renders (not deployed) — matching how database_instances (and others) already report pre-deploy state.

Why

Two parallel implementations of the same idea drift over time. Putting every resource's URL on the same lookup means future additions only touch one map, and tests like TestBundleResourcePluralNamesResolveInWorkspaceURLs (extended here) can fail loudly when a bundle plural name or pattern key drifts.

Tests

  • go test ./libs/workspaceurls/ ./bundle/config/ ./bundle/config/resources/ ./bundle/config/mutator/ ./cmd/experimental/ — green
  • ./task lint — 0 issues
  • ./task fmt, ./task checks — clean
  • Acceptance: TestAccept/bundle/resources/..., TestAccept/bundle/summary/..., TestAccept/experimental/open all green. Regenerated:
    • acceptance/experimental/open/output.txt (expanded supported-type list)
    • acceptance/bundle/resources/postgres_synced_tables/basic/output.txt (URL → (not deployed) pre-deploy)

This pull request was written by Claude Code.

@eng-dev-ecosystem-bot
Copy link
Copy Markdown
Collaborator

eng-dev-ecosystem-bot commented May 27, 2026

Commit: aadde2a

Run: 26515771331

Move the 11 inline resource URL builders (catalogs, schemas, volumes,
database_catalogs, database_instances, postgres_catalogs,
postgres_synced_tables, quality_monitors, synced_database_tables,
vector_search_endpoints, vector_search_indexes) onto
workspaceurls.ResourceURL, joining the ones (jobs, pipelines, apps, ...)
that already used it. Each resource's InitializeURL is now a one-liner;
resource-specific guards (ModifiedStatus on DatabaseInstance, 3-part
name check on PostgresSyncedTable and VectorSearchIndex) stay in the
resource.

The new patterns and dotSeparatedResources flags live in
libs/workspaceurls/urls.go, which also drives the experimental open
command -- its supported-type list grows from 13 to 24.

PostgresSyncedTable's and VectorSearchIndex's previous
strings.Cut-twice logic produced a malformed URL when the name still
contained an unresolved ${resources.X.Y} reference. The new strict
3-part check leaves the URL empty, so summary now renders
"(not deployed)" -- matching the rest of the codebase (e.g.
database_instances).

Co-authored-by: Isaac
@janniklasrose janniklasrose force-pushed the janniklasrose/centralise-url-handling branch from 292cd74 to 6d3a879 Compare May 27, 2026 13:44
@janniklasrose janniklasrose marked this pull request as ready for review May 27, 2026 13:49
Postgres projects:
my_project:
Name: Test Project for Synced Table
URL: (not deployed)
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 still says "(not deployed)", is it intentional? @pietern

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.

See also PR summary. The previous value was wrong (you can see the reference still in the URL), and since the backend-minted ID is required to render the URL it shows (not deployed) until it is

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.

oops, nvm I thought you were referring to the actual edit above. But you mean my_project not getting updated post-deployment!

Related to https://github.com/databricks/cli/pull/5346/changes#r3311349591 I guess, since the resource currently opts out of URLs.

@@ -123,15 +123,26 @@ func TestBundleResourcePluralNamesResolveInWorkspaceURLs(t *testing.T) {
withURLs := []string{
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.

a) It seems easier to include missing ones?

b) Should there be missing ones? I see these are missing, is that intentional?

Copy link
Copy Markdown
Contributor

@andrewnester andrewnester May 27, 2026

Choose a reason for hiding this comment

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

external_locations and secret_scopes are intentional, these are not workspace resources with its own URL. I believe postgres ones too

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.

Now that almost all are here, I like the idea of inverting it into an opt-out for missing ones. Will do as follow-up and address postgres_* as part of upcoming work

"jobs": "jobs/%s",
"models": "ml/models/%s",
"model_serving_endpoints": "ml/endpoints/%s",
"notebooks": "#notebook/%s",
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.

q - it's not a resource we have, what does this do?

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.

@simonfaltum you added this per git blame

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.

There's databricks experimental open which can open notebooks, I guess it's needed for that

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Yeah - the idea was to have a way in the CLI to open workspace resources / generate URL's, so we can enable agents to easily present what they do to the user

"jobs": "jobs/%s",
"models": "ml/models/%s",
"model_serving_endpoints": "ml/endpoints/%s",
"notebooks": "#notebook/%s",
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.

There's databricks experimental open which can open notebooks, I guess it's needed for that

@janniklasrose janniklasrose added this pull request to the merge queue May 27, 2026
Merged via the queue into main with commit b85b937 May 27, 2026
36 of 37 checks passed
@janniklasrose janniklasrose deleted the janniklasrose/centralise-url-handling branch May 27, 2026 16:06
@eng-dev-ecosystem-bot
Copy link
Copy Markdown
Collaborator

Commit: b85b937

Run: 26523214530

denik pushed a commit that referenced this pull request May 28, 2026
## Changes

Move the 11 inline resource URL builders (`catalogs`, `schemas`,
`volumes`, `database_catalogs`, `database_instances`,
`postgres_catalogs`, `postgres_synced_tables`, `quality_monitors`,
`synced_database_tables`, `vector_search_endpoints`,
`vector_search_indexes`) onto `workspaceurls.ResourceURL`, joining the
ones (`jobs`, `pipelines`, `apps`, ...) that already used it. Each
resource's `InitializeURL` is now a one-liner; resource-specific guards
(`ModifiedStatus` on `DatabaseInstance`, 3-part name check on
`PostgresSyncedTable` and `VectorSearchIndex`) stay in the resource.

The new patterns and `dotSeparatedResources` flags live in
`libs/workspaceurls/urls.go`, which also drives the `experimental open`
command — its supported-type list grows from 13 to 24. Acceptance
snapshot for `experimental/open` regenerated to match.

One behaviour delta on `postgres_synced_tables` /
`vector_search_indexes` pre-deploy: the previous `strings.Cut`-twice
logic produced a malformed URL like
`[DATABRICKS_URL]/explore/data/$%7Bresources/postgres_catalogs/my_catalog.catalog_id%7D.public.trips_synced`
when the name still contained an unresolved `${resources.X.Y}`
reference. The new strict 3-part check leaves the URL empty, so summary
now renders `(not deployed)` — matching how `database_instances` (and
others) already report pre-deploy state.

## Why

Two parallel implementations of the same idea drift over time. Putting
every resource's URL on the same lookup means future additions only
touch one map, and tests like
`TestBundleResourcePluralNamesResolveInWorkspaceURLs` (extended here)
can fail loudly when a bundle plural name or pattern key drifts.

## Tests

- `go test ./libs/workspaceurls/ ./bundle/config/
./bundle/config/resources/ ./bundle/config/mutator/ ./cmd/experimental/`
— green
- `./task lint` — 0 issues
- `./task fmt`, `./task checks` — clean
- Acceptance: `TestAccept/bundle/resources/...`,
`TestAccept/bundle/summary/...`, `TestAccept/experimental/open` all
green. Regenerated:
- `acceptance/experimental/open/output.txt` (expanded supported-type
list)
- `acceptance/bundle/resources/postgres_synced_tables/basic/output.txt`
(URL → `(not deployed)` pre-deploy)

_This pull request was written by Claude Code._
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.

5 participants