resources: centralise InitializeURL through libs/workspaceurls#5346
Conversation
|
Commit: aadde2a |
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
292cd74 to
6d3a879
Compare
Co-authored-by: Isaac
| Postgres projects: | ||
| my_project: | ||
| Name: Test Project for Synced Table | ||
| URL: (not deployed) |
There was a problem hiding this comment.
This still says "(not deployed)", is it intentional? @pietern
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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{ | |||
There was a problem hiding this comment.
a) It seems easier to include missing ones?
b) Should there be missing ones? I see these are missing, is that intentional?
- external_locations cc @andrewnester
- secret_scopes
- postgres_{projects,branches,endpoints} cc @pietern
There was a problem hiding this comment.
external_locations and secret_scopes are intentional, these are not workspace resources with its own URL. I believe postgres ones too
There was a problem hiding this comment.
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", |
There was a problem hiding this comment.
q - it's not a resource we have, what does this do?
There was a problem hiding this comment.
@simonfaltum you added this per git blame
There was a problem hiding this comment.
There's databricks experimental open which can open notebooks, I guess it's needed for that
There was a problem hiding this comment.
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", |
There was a problem hiding this comment.
There's databricks experimental open which can open notebooks, I guess it's needed for that
|
Commit: b85b937 |
## 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._
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) ontoworkspaceurls.ResourceURL, joining the ones (jobs,pipelines,apps, ...) that already used it. Each resource'sInitializeURLis now a one-liner; resource-specific guards (ModifiedStatusonDatabaseInstance, 3-part name check onPostgresSyncedTableandVectorSearchIndex) stay in the resource.The new patterns and
dotSeparatedResourcesflags live inlibs/workspaceurls/urls.go, which also drives theexperimental opencommand — its supported-type list grows from 13 to 24. Acceptance snapshot forexperimental/openregenerated to match.One behaviour delta on
postgres_synced_tables/vector_search_indexespre-deploy: the previousstrings.Cut-twice logic produced a malformed URL like[DATABRICKS_URL]/explore/data/$%7Bresources/postgres_catalogs/my_catalog.catalog_id%7D.public.trips_syncedwhen 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 howdatabase_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— cleanTestAccept/bundle/resources/...,TestAccept/bundle/summary/...,TestAccept/experimental/openall 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.