From 4093a5ae705e40b215cc131e467e32a20e0bd006 Mon Sep 17 00:00:00 2001 From: simon Date: Fri, 13 Mar 2026 00:33:49 +0100 Subject: [PATCH 01/10] Add experimental workspace open command Adds `databricks experimental open RESOURCE_TYPE ID_OR_PATH` to open workspace resources (jobs, notebooks, clusters, pipelines, dashboards, warehouses, queries, apps) in the default browser. Handles both path-based and fragment-based URL patterns correctly. Includes shell completion for resource types. Co-authored-by: Isaac --- cmd/experimental/experimental.go | 1 + cmd/experimental/workspace_open.go | 113 ++++++++++++++++++++++++ cmd/experimental/workspace_open_test.go | 86 ++++++++++++++++++ 3 files changed, 200 insertions(+) create mode 100644 cmd/experimental/workspace_open.go create mode 100644 cmd/experimental/workspace_open_test.go diff --git a/cmd/experimental/experimental.go b/cmd/experimental/experimental.go index eb3b7814e1..36ad876589 100644 --- a/cmd/experimental/experimental.go +++ b/cmd/experimental/experimental.go @@ -21,6 +21,7 @@ development. They may change or be removed in future versions without notice.`, } cmd.AddCommand(aitoolscmd.NewAitoolsCmd()) + cmd.AddCommand(newWorkspaceOpenCommand()) return cmd } diff --git a/cmd/experimental/workspace_open.go b/cmd/experimental/workspace_open.go new file mode 100644 index 0000000000..27c9fbe010 --- /dev/null +++ b/cmd/experimental/workspace_open.go @@ -0,0 +1,113 @@ +package experimental + +import ( + "fmt" + "io" + "net/url" + "sort" + + browserpkg "github.com/pkg/browser" + "github.com/spf13/cobra" + + "github.com/databricks/cli/cmd/root" + "github.com/databricks/cli/libs/cmdctx" + "github.com/databricks/cli/libs/cmdio" +) + +// resourceURLPatterns maps resource type names to their URL path patterns. +// Patterns starting with "#" use URL fragments instead of path segments. +var resourceURLPatterns = map[string]string{ + "job": "jobs/%s", + "notebook": "#notebook/%s", + "cluster": "#/setting/clusters/%s/configuration", + "pipeline": "pipelines/%s", + "dashboard": "sql/dashboards/%s", + "warehouse": "sql/warehouses/%s", + "query": "sql/editor/%s", + "app": "apps/%s", +} + +func resourceTypeNames() []string { + names := make([]string, 0, len(resourceURLPatterns)) + for name := range resourceURLPatterns { + names = append(names, name) + } + sort.Strings(names) + return names +} + +// buildWorkspaceURL constructs a full workspace URL for a given resource type and ID. +func buildWorkspaceURL(host, resourceType, id string) (string, error) { + pattern, ok := resourceURLPatterns[resourceType] + if !ok { + return "", fmt.Errorf("unknown resource type %q, must be one of: %v", resourceType, resourceTypeNames()) + } + + baseURL, err := url.Parse(host) + if err != nil { + return "", fmt.Errorf("invalid workspace host %q: %w", host, err) + } + + resourcePath := fmt.Sprintf(pattern, id) + + // Fragment-based URLs (starting with #) need special handling. + if len(resourcePath) > 0 && resourcePath[0] == '#' { + baseURL.Fragment = resourcePath[1:] + } else { + baseURL.Path = resourcePath + } + + return baseURL.String(), nil +} + +func newWorkspaceOpenCommand() *cobra.Command { + cmd := &cobra.Command{ + Use: "open RESOURCE_TYPE ID_OR_PATH", + Short: "Open a workspace resource in the browser", + Long: `Open a workspace resource in the default browser. + +Supported resource types: job, notebook, cluster, pipeline, dashboard, warehouse, query, app. + +Examples: + databricks experimental open job 123456789 + databricks experimental open notebook /Users/user@example.com/my-notebook + databricks experimental open cluster 0123-456789-abcdef`, + Args: cobra.ExactArgs(2), + PreRunE: root.MustWorkspaceClient, + RunE: func(cmd *cobra.Command, args []string) error { + ctx := cmd.Context() + w := cmdctx.WorkspaceClient(ctx) + + resourceType := args[0] + id := args[1] + + resourceURL, err := buildWorkspaceURL(w.Config.Host, resourceType, id) + if err != nil { + return err + } + + cmdio.LogString(ctx, fmt.Sprintf("Opening %s %s in the browser...", resourceType, id)) + + return openURLSuppressingBrowserStderr(resourceURL) + }, + ValidArgsFunction: func(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) { + if len(args) == 0 { + return resourceTypeNames(), cobra.ShellCompDirectiveNoFileComp + } + return nil, cobra.ShellCompDirectiveNoFileComp + }, + } + + return cmd +} + +func openURLSuppressingBrowserStderr(targetURL string) error { + originalStderr := browserpkg.Stderr + defer func() { + browserpkg.Stderr = originalStderr + }() + + browserpkg.Stderr = io.Discard + + return browserpkg.OpenURL(targetURL) +} diff --git a/cmd/experimental/workspace_open_test.go b/cmd/experimental/workspace_open_test.go new file mode 100644 index 0000000000..59d0b50768 --- /dev/null +++ b/cmd/experimental/workspace_open_test.go @@ -0,0 +1,86 @@ +package experimental + +import ( + "testing" + + "github.com/spf13/cobra" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestBuildWorkspaceURLPathBasedResources(t *testing.T) { + tests := []struct { + resourceType string + id string + expected string + }{ + {"job", "123", "https://myworkspace.databricks.com/jobs/123"}, + {"pipeline", "abc-def", "https://myworkspace.databricks.com/pipelines/abc-def"}, + {"dashboard", "dash-1", "https://myworkspace.databricks.com/sql/dashboards/dash-1"}, + {"warehouse", "wh-1", "https://myworkspace.databricks.com/sql/warehouses/wh-1"}, + {"query", "q-1", "https://myworkspace.databricks.com/sql/editor/q-1"}, + {"app", "my-app", "https://myworkspace.databricks.com/apps/my-app"}, + } + + for _, tt := range tests { + t.Run(tt.resourceType, func(t *testing.T) { + got, err := buildWorkspaceURL("https://myworkspace.databricks.com", tt.resourceType, tt.id) + require.NoError(t, err) + assert.Equal(t, tt.expected, got) + }) + } +} + +func TestBuildWorkspaceURLFragmentBasedResources(t *testing.T) { + tests := []struct { + resourceType string + id string + expected string + }{ + {"notebook", "12345", "https://myworkspace.databricks.com#notebook/12345"}, + {"cluster", "0123-456789-abc", "https://myworkspace.databricks.com#/setting/clusters/0123-456789-abc/configuration"}, + } + + for _, tt := range tests { + t.Run(tt.resourceType, func(t *testing.T) { + got, err := buildWorkspaceURL("https://myworkspace.databricks.com", tt.resourceType, tt.id) + require.NoError(t, err) + assert.Equal(t, tt.expected, got) + }) + } +} + +func TestBuildWorkspaceURLUnknownResourceType(t *testing.T) { + _, err := buildWorkspaceURL("https://myworkspace.databricks.com", "unknown", "123") + assert.ErrorContains(t, err, "unknown resource type \"unknown\"") +} + +func TestBuildWorkspaceURLHostWithTrailingSlash(t *testing.T) { + got, err := buildWorkspaceURL("https://myworkspace.databricks.com/", "job", "123") + require.NoError(t, err) + assert.Equal(t, "https://myworkspace.databricks.com/jobs/123", got) +} + +func TestWorkspaceOpenCommandCompletion(t *testing.T) { + cmd := newWorkspaceOpenCommand() + + completions, directive := cmd.ValidArgsFunction(cmd, []string{}, "") + assert.Equal(t, cobra.ShellCompDirectiveNoFileComp, directive) + assert.Contains(t, completions, "job") + assert.Contains(t, completions, "notebook") + assert.Contains(t, completions, "cluster") + assert.Contains(t, completions, "pipeline") + assert.Contains(t, completions, "dashboard") + assert.Contains(t, completions, "warehouse") + assert.Contains(t, completions, "query") + assert.Contains(t, completions, "app") + assert.Len(t, completions, 8) +} + +func TestWorkspaceOpenCommandCompletionSecondArg(t *testing.T) { + cmd := newWorkspaceOpenCommand() + + completions, directive := cmd.ValidArgsFunction(cmd, []string{"job"}, "") + assert.Equal(t, cobra.ShellCompDirectiveNoFileComp, directive) + assert.Nil(t, completions) +} From db5bf278487964dfa7d889797bf796929e2448a9 Mon Sep 17 00:00:00 2001 From: simon Date: Fri, 13 Mar 2026 03:58:16 +0100 Subject: [PATCH 02/10] Fix review findings: URL patterns, fragment handling, workspace ID - Fix fragment URLs to include `/` before `#` (host/#notebook vs host#notebook) - Update cluster pattern to `compute/clusters/%s` (matching bundle code) - Update dashboard pattern to `dashboardsv3/%s/published` (matching bundle code) - Add leading `/` to all path-based URL patterns - Add `?o=` via CurrentWorkspaceID for shared-domain workspaces - Add tests for notebook path-style IDs, workspace ID parameter - Add doc comment for openURLSuppressingBrowserStderr Co-authored-by: Isaac --- cmd/experimental/workspace_open.go | 41 +++++++++++++++++++------ cmd/experimental/workspace_open_test.go | 36 +++++++++++++++++----- 2 files changed, 60 insertions(+), 17 deletions(-) diff --git a/cmd/experimental/workspace_open.go b/cmd/experimental/workspace_open.go index 27c9fbe010..371be0497c 100644 --- a/cmd/experimental/workspace_open.go +++ b/cmd/experimental/workspace_open.go @@ -5,6 +5,8 @@ import ( "io" "net/url" "sort" + "strconv" + "strings" browserpkg "github.com/pkg/browser" "github.com/spf13/cobra" @@ -17,14 +19,14 @@ import ( // resourceURLPatterns maps resource type names to their URL path patterns. // Patterns starting with "#" use URL fragments instead of path segments. var resourceURLPatterns = map[string]string{ - "job": "jobs/%s", + "job": "/jobs/%s", "notebook": "#notebook/%s", - "cluster": "#/setting/clusters/%s/configuration", - "pipeline": "pipelines/%s", - "dashboard": "sql/dashboards/%s", - "warehouse": "sql/warehouses/%s", - "query": "sql/editor/%s", - "app": "apps/%s", + "cluster": "/compute/clusters/%s", + "pipeline": "/pipelines/%s", + "dashboard": "/dashboardsv3/%s/published", + "warehouse": "/sql/warehouses/%s", + "query": "/sql/editor/%s", + "app": "/apps/%s", } func resourceTypeNames() []string { @@ -37,7 +39,7 @@ func resourceTypeNames() []string { } // buildWorkspaceURL constructs a full workspace URL for a given resource type and ID. -func buildWorkspaceURL(host, resourceType, id string) (string, error) { +func buildWorkspaceURL(host, resourceType, id string, workspaceID int64) (string, error) { pattern, ok := resourceURLPatterns[resourceType] if !ok { return "", fmt.Errorf("unknown resource type %q, must be one of: %v", resourceType, resourceTypeNames()) @@ -48,10 +50,23 @@ func buildWorkspaceURL(host, resourceType, id string) (string, error) { return "", fmt.Errorf("invalid workspace host %q: %w", host, err) } + // Append ?o= when the workspace ID is not already in the + // hostname (e.g. vanity URLs or legacy workspace URLs). + // See https://docs.databricks.com/en/workspace/workspace-details.html + if workspaceID != 0 { + orgID := strconv.FormatInt(workspaceID, 10) + if !strings.Contains(baseURL.Hostname(), orgID) { + values := baseURL.Query() + values.Add("o", orgID) + baseURL.RawQuery = values.Encode() + } + } + resourcePath := fmt.Sprintf(pattern, id) // Fragment-based URLs (starting with #) need special handling. if len(resourcePath) > 0 && resourcePath[0] == '#' { + baseURL.Path = "/" baseURL.Fragment = resourcePath[1:] } else { baseURL.Path = resourcePath @@ -81,7 +96,12 @@ Examples: resourceType := args[0] id := args[1] - resourceURL, err := buildWorkspaceURL(w.Config.Host, resourceType, id) + workspaceID, err := w.CurrentWorkspaceID(ctx) + if err != nil { + workspaceID = 0 + } + + resourceURL, err := buildWorkspaceURL(w.Config.Host, resourceType, id, workspaceID) if err != nil { return err } @@ -101,6 +121,9 @@ Examples: return cmd } +// openURLSuppressingBrowserStderr suppresses stderr output from the browser +// launcher, which often emits noisy warnings (e.g. from xdg-open) that +// would confuse CLI users. func openURLSuppressingBrowserStderr(targetURL string) error { originalStderr := browserpkg.Stderr defer func() { diff --git a/cmd/experimental/workspace_open_test.go b/cmd/experimental/workspace_open_test.go index 59d0b50768..b0dcb856fc 100644 --- a/cmd/experimental/workspace_open_test.go +++ b/cmd/experimental/workspace_open_test.go @@ -16,15 +16,16 @@ func TestBuildWorkspaceURLPathBasedResources(t *testing.T) { }{ {"job", "123", "https://myworkspace.databricks.com/jobs/123"}, {"pipeline", "abc-def", "https://myworkspace.databricks.com/pipelines/abc-def"}, - {"dashboard", "dash-1", "https://myworkspace.databricks.com/sql/dashboards/dash-1"}, + {"dashboard", "dash-1", "https://myworkspace.databricks.com/dashboardsv3/dash-1/published"}, {"warehouse", "wh-1", "https://myworkspace.databricks.com/sql/warehouses/wh-1"}, {"query", "q-1", "https://myworkspace.databricks.com/sql/editor/q-1"}, {"app", "my-app", "https://myworkspace.databricks.com/apps/my-app"}, + {"cluster", "0123-456789-abc", "https://myworkspace.databricks.com/compute/clusters/0123-456789-abc"}, } for _, tt := range tests { t.Run(tt.resourceType, func(t *testing.T) { - got, err := buildWorkspaceURL("https://myworkspace.databricks.com", tt.resourceType, tt.id) + got, err := buildWorkspaceURL("https://myworkspace.databricks.com", tt.resourceType, tt.id, 0) require.NoError(t, err) assert.Equal(t, tt.expected, got) }) @@ -37,13 +38,13 @@ func TestBuildWorkspaceURLFragmentBasedResources(t *testing.T) { id string expected string }{ - {"notebook", "12345", "https://myworkspace.databricks.com#notebook/12345"}, - {"cluster", "0123-456789-abc", "https://myworkspace.databricks.com#/setting/clusters/0123-456789-abc/configuration"}, + {"notebook", "12345", "https://myworkspace.databricks.com/#notebook/12345"}, + {"notebook", "/Users/user@example.com/my-notebook", "https://myworkspace.databricks.com/#notebook//Users/user@example.com/my-notebook"}, } for _, tt := range tests { - t.Run(tt.resourceType, func(t *testing.T) { - got, err := buildWorkspaceURL("https://myworkspace.databricks.com", tt.resourceType, tt.id) + t.Run(tt.id, func(t *testing.T) { + got, err := buildWorkspaceURL("https://myworkspace.databricks.com", tt.resourceType, tt.id, 0) require.NoError(t, err) assert.Equal(t, tt.expected, got) }) @@ -51,16 +52,35 @@ func TestBuildWorkspaceURLFragmentBasedResources(t *testing.T) { } func TestBuildWorkspaceURLUnknownResourceType(t *testing.T) { - _, err := buildWorkspaceURL("https://myworkspace.databricks.com", "unknown", "123") + _, err := buildWorkspaceURL("https://myworkspace.databricks.com", "unknown", "123", 0) assert.ErrorContains(t, err, "unknown resource type \"unknown\"") } func TestBuildWorkspaceURLHostWithTrailingSlash(t *testing.T) { - got, err := buildWorkspaceURL("https://myworkspace.databricks.com/", "job", "123") + got, err := buildWorkspaceURL("https://myworkspace.databricks.com/", "job", "123", 0) require.NoError(t, err) assert.Equal(t, "https://myworkspace.databricks.com/jobs/123", got) } +func TestBuildWorkspaceURLWithWorkspaceID(t *testing.T) { + got, err := buildWorkspaceURL("https://myworkspace.databricks.com", "job", "123", 123456) + require.NoError(t, err) + assert.Equal(t, "https://myworkspace.databricks.com/jobs/123?o=123456", got) +} + +func TestBuildWorkspaceURLWithWorkspaceIDInHostname(t *testing.T) { + got, err := buildWorkspaceURL("https://adb-123456.azuredatabricks.net", "job", "123", 123456) + require.NoError(t, err) + // Workspace ID is already in the hostname, so ?o= should not be appended. + assert.Equal(t, "https://adb-123456.azuredatabricks.net/jobs/123", got) +} + +func TestBuildWorkspaceURLFragmentWithWorkspaceID(t *testing.T) { + got, err := buildWorkspaceURL("https://myworkspace.databricks.com", "notebook", "12345", 789) + require.NoError(t, err) + assert.Equal(t, "https://myworkspace.databricks.com/?o=789#notebook/12345", got) +} + func TestWorkspaceOpenCommandCompletion(t *testing.T) { cmd := newWorkspaceOpenCommand() From edf1682ed84e5904b924f35080c56377a82bca3f Mon Sep 17 00:00:00 2001 From: simon Date: Fri, 13 Mar 2026 07:26:21 +0100 Subject: [PATCH 03/10] Fix BROWSER contract and workspace ID detection --- cmd/auth/login.go | 57 +------------------ cmd/auth/token.go | 3 +- cmd/experimental/workspace_open.go | 30 ++++------ cmd/experimental/workspace_open_test.go | 6 ++ libs/browser/open.go | 70 ++++++++++++++++++++++++ libs/browser/open_test.go | 73 +++++++++++++++++++++++++ 6 files changed, 164 insertions(+), 75 deletions(-) create mode 100644 libs/browser/open.go create mode 100644 libs/browser/open_test.go diff --git a/cmd/auth/login.go b/cmd/auth/login.go index 1a1240d84c..6de4ddc304 100644 --- a/cmd/auth/login.go +++ b/cmd/auth/login.go @@ -4,23 +4,21 @@ import ( "context" "errors" "fmt" - "io" "runtime" "strings" "time" "github.com/databricks/cli/libs/auth" + "github.com/databricks/cli/libs/browser" "github.com/databricks/cli/libs/cmdio" "github.com/databricks/cli/libs/databrickscfg" "github.com/databricks/cli/libs/databrickscfg/cfgpickers" "github.com/databricks/cli/libs/databrickscfg/profile" "github.com/databricks/cli/libs/env" - "github.com/databricks/cli/libs/exec" "github.com/databricks/databricks-sdk-go" "github.com/databricks/databricks-sdk-go/config" "github.com/databricks/databricks-sdk-go/config/experimental/auth/authconv" "github.com/databricks/databricks-sdk-go/credentials/u2m" - browserpkg "github.com/pkg/browser" "github.com/spf13/cobra" ) @@ -174,7 +172,7 @@ depends on the existing profiles you have set in your configuration file } persistentAuthOpts := []u2m.PersistentAuthOption{ u2m.WithOAuthArgument(oauthArgument), - u2m.WithBrowser(getBrowserFunc(cmd)), + u2m.WithBrowser(browser.NewOpener(cmd.Context(), ".")), } if len(scopesList) > 0 { persistentAuthOpts = append(persistentAuthOpts, u2m.WithScopes(scopesList)) @@ -400,60 +398,9 @@ func loadProfileByName(ctx context.Context, profileName string, profiler profile return nil, nil } -// openURLSuppressingStderr opens a URL in the browser while suppressing stderr output. -// This prevents xdg-open error messages from being displayed to the user. -func openURLSuppressingStderr(url string) error { - // Save the original stderr from the browser package - originalStderr := browserpkg.Stderr - defer func() { - browserpkg.Stderr = originalStderr - }() - - // Redirect stderr to discard to suppress xdg-open errors - browserpkg.Stderr = io.Discard - - // Call the browser open function - return browserpkg.OpenURL(url) -} - // oauthLoginClearKeys returns profile keys that should be explicitly removed // when performing an OAuth login. Derives auth credential fields dynamically // from the SDK's ConfigAttributes to stay in sync as new auth methods are added. func oauthLoginClearKeys() []string { return databrickscfg.AuthCredentialKeys() } - -// getBrowserFunc returns a function that opens the given URL in the browser. -// It respects the BROWSER environment variable: -// - empty string: uses the default browser -// - "none": prints the URL to stdout without opening a browser -// - custom command: executes the specified command with the URL as argument -func getBrowserFunc(cmd *cobra.Command) func(url string) error { - browser := env.Get(cmd.Context(), "BROWSER") - switch browser { - case "": - return openURLSuppressingStderr - case "none": - return func(url string) error { - cmdio.LogString(cmd.Context(), "Please complete authentication by opening this link in your browser:\n"+url) - return nil - } - default: - return func(url string) error { - // Run the browser command via a shell. - // It can be a script or a binary and scripts cannot be executed directly on Windows. - e, err := exec.NewCommandExecutor(".") - if err != nil { - return err - } - - e.WithInheritOutput() - cmd, err := e.StartCommand(cmd.Context(), fmt.Sprintf("%q %q", browser, url)) - if err != nil { - return err - } - - return cmd.Wait() - } - } -} diff --git a/cmd/auth/token.go b/cmd/auth/token.go index b33722c1ed..15fc30745b 100644 --- a/cmd/auth/token.go +++ b/cmd/auth/token.go @@ -9,6 +9,7 @@ import ( "time" "github.com/databricks/cli/libs/auth" + "github.com/databricks/cli/libs/browser" "github.com/databricks/cli/libs/cmdio" "github.com/databricks/cli/libs/databrickscfg" "github.com/databricks/cli/libs/databrickscfg/profile" @@ -432,7 +433,7 @@ func runInlineLogin(ctx context.Context, profiler profile.Profiler) (string, *pr } persistentAuthOpts := []u2m.PersistentAuthOption{ u2m.WithOAuthArgument(oauthArgument), - u2m.WithBrowser(openURLSuppressingStderr), + u2m.WithBrowser(browser.NewOpener(ctx, ".")), } if len(scopesList) > 0 { persistentAuthOpts = append(persistentAuthOpts, u2m.WithScopes(scopesList)) diff --git a/cmd/experimental/workspace_open.go b/cmd/experimental/workspace_open.go index 371be0497c..c961d3fe80 100644 --- a/cmd/experimental/workspace_open.go +++ b/cmd/experimental/workspace_open.go @@ -2,16 +2,15 @@ package experimental import ( "fmt" - "io" "net/url" "sort" "strconv" "strings" - browserpkg "github.com/pkg/browser" "github.com/spf13/cobra" "github.com/databricks/cli/cmd/root" + "github.com/databricks/cli/libs/browser" "github.com/databricks/cli/libs/cmdctx" "github.com/databricks/cli/libs/cmdio" ) @@ -55,7 +54,7 @@ func buildWorkspaceURL(host, resourceType, id string, workspaceID int64) (string // See https://docs.databricks.com/en/workspace/workspace-details.html if workspaceID != 0 { orgID := strconv.FormatInt(workspaceID, 10) - if !strings.Contains(baseURL.Hostname(), orgID) { + if !hasWorkspaceIDInHostname(baseURL.Hostname(), orgID) { values := baseURL.Query() values.Add("o", orgID) baseURL.RawQuery = values.Encode() @@ -75,6 +74,11 @@ func buildWorkspaceURL(host, resourceType, id string, workspaceID int64) (string return baseURL.String(), nil } +func hasWorkspaceIDInHostname(hostname, workspaceID string) bool { + remainder, ok := strings.CutPrefix(strings.ToLower(hostname), "adb-"+workspaceID) + return ok && (remainder == "" || strings.HasPrefix(remainder, ".")) +} + func newWorkspaceOpenCommand() *cobra.Command { cmd := &cobra.Command{ Use: "open RESOURCE_TYPE ID_OR_PATH", @@ -106,9 +110,11 @@ Examples: return err } - cmdio.LogString(ctx, fmt.Sprintf("Opening %s %s in the browser...", resourceType, id)) + if !browser.IsDisabled(ctx) { + cmdio.LogString(ctx, fmt.Sprintf("Opening %s %s in the browser...", resourceType, id)) + } - return openURLSuppressingBrowserStderr(resourceURL) + return browser.OpenURL(ctx, ".", resourceURL) }, ValidArgsFunction: func(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) { if len(args) == 0 { @@ -120,17 +126,3 @@ Examples: return cmd } - -// openURLSuppressingBrowserStderr suppresses stderr output from the browser -// launcher, which often emits noisy warnings (e.g. from xdg-open) that -// would confuse CLI users. -func openURLSuppressingBrowserStderr(targetURL string) error { - originalStderr := browserpkg.Stderr - defer func() { - browserpkg.Stderr = originalStderr - }() - - browserpkg.Stderr = io.Discard - - return browserpkg.OpenURL(targetURL) -} diff --git a/cmd/experimental/workspace_open_test.go b/cmd/experimental/workspace_open_test.go index b0dcb856fc..c3195533d8 100644 --- a/cmd/experimental/workspace_open_test.go +++ b/cmd/experimental/workspace_open_test.go @@ -75,6 +75,12 @@ func TestBuildWorkspaceURLWithWorkspaceIDInHostname(t *testing.T) { assert.Equal(t, "https://adb-123456.azuredatabricks.net/jobs/123", got) } +func TestBuildWorkspaceURLWithWorkspaceIDInVanityHostname(t *testing.T) { + got, err := buildWorkspaceURL("https://workspace-123456.example.com", "job", "123", 123456) + require.NoError(t, err) + assert.Equal(t, "https://workspace-123456.example.com/jobs/123?o=123456", got) +} + func TestBuildWorkspaceURLFragmentWithWorkspaceID(t *testing.T) { got, err := buildWorkspaceURL("https://myworkspace.databricks.com", "notebook", "12345", 789) require.NoError(t, err) diff --git a/libs/browser/open.go b/libs/browser/open.go new file mode 100644 index 0000000000..f55b2bb47e --- /dev/null +++ b/libs/browser/open.go @@ -0,0 +1,70 @@ +package browser + +import ( + "context" + "fmt" + "io" + + "github.com/databricks/cli/libs/cmdio" + "github.com/databricks/cli/libs/env" + "github.com/databricks/cli/libs/exec" + browserpkg "github.com/pkg/browser" +) + +const ( + browserEnvVar = "BROWSER" + disabledBrowser = "none" +) + +var openDefaultBrowserURL = func(targetURL string) error { + originalStderr := browserpkg.Stderr + defer func() { + browserpkg.Stderr = originalStderr + }() + + browserpkg.Stderr = io.Discard + return browserpkg.OpenURL(targetURL) +} + +var runBrowserCommand = func(ctx context.Context, workingDirectory, browserCommand, targetURL string) error { + e, err := exec.NewCommandExecutor(workingDirectory) + if err != nil { + return err + } + + e.WithInheritOutput() + cmd, err := e.StartCommand(ctx, fmt.Sprintf("%q %q", browserCommand, targetURL)) + if err != nil { + return err + } + + return cmd.Wait() +} + +// IsDisabled reports whether browser launching is disabled for the context. +func IsDisabled(ctx context.Context) bool { + return env.Get(ctx, browserEnvVar) == disabledBrowser +} + +// NewOpener returns a function that opens URLs in the browser. +func NewOpener(ctx context.Context, workingDirectory string) func(string) error { + browserCommand := env.Get(ctx, browserEnvVar) + switch browserCommand { + case "": + return openDefaultBrowserURL + case disabledBrowser: + return func(targetURL string) error { + cmdio.LogString(ctx, "Please complete authentication by opening this link in your browser:\n"+targetURL) + return nil + } + default: + return func(targetURL string) error { + return runBrowserCommand(ctx, workingDirectory, browserCommand, targetURL) + } + } +} + +// OpenURL opens a URL in the browser. +func OpenURL(ctx context.Context, workingDirectory, targetURL string) error { + return NewOpener(ctx, workingDirectory)(targetURL) +} diff --git a/libs/browser/open_test.go b/libs/browser/open_test.go new file mode 100644 index 0000000000..b5e65348eb --- /dev/null +++ b/libs/browser/open_test.go @@ -0,0 +1,73 @@ +package browser + +import ( + "context" + "testing" + + "github.com/databricks/cli/libs/cmdio" + "github.com/databricks/cli/libs/env" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestOpenURLUsesDefaultBrowser(t *testing.T) { + original := openDefaultBrowserURL + t.Cleanup(func() { + openDefaultBrowserURL = original + }) + + var got string + openDefaultBrowserURL = func(targetURL string) error { + got = targetURL + return nil + } + + err := OpenURL(t.Context(), ".", "https://example.com") + require.NoError(t, err) + assert.Equal(t, "https://example.com", got) +} + +func TestOpenURLWithDisabledBrowser(t *testing.T) { + ctx, stderr := cmdio.NewTestContextWithStderr(t.Context()) + ctx = env.Set(ctx, browserEnvVar, disabledBrowser) + + err := OpenURL(ctx, ".", "https://example.com") + require.NoError(t, err) + assert.Contains(t, stderr.String(), "Please complete authentication by opening this link in your browser:") + assert.Contains(t, stderr.String(), "https://example.com") +} + +func TestOpenURLUsesCustomBrowserCommand(t *testing.T) { + original := runBrowserCommand + t.Cleanup(func() { + runBrowserCommand = original + }) + + ctx := env.Set(t.Context(), browserEnvVar, "custom-browser") + + var gotCtx context.Context + var gotDirectory string + var gotCommand string + var gotURL string + runBrowserCommand = func(ctx context.Context, workingDirectory, browserCommand, targetURL string) error { + gotCtx = ctx + gotDirectory = workingDirectory + gotCommand = browserCommand + gotURL = targetURL + return nil + } + + err := OpenURL(ctx, "test-dir", "https://example.com") + require.NoError(t, err) + assert.Same(t, ctx, gotCtx) + assert.Equal(t, "test-dir", gotDirectory) + assert.Equal(t, "custom-browser", gotCommand) + assert.Equal(t, "https://example.com", gotURL) +} + +func TestIsDisabled(t *testing.T) { + assert.False(t, IsDisabled(t.Context())) + + ctx := env.Set(t.Context(), browserEnvVar, disabledBrowser) + assert.True(t, IsDisabled(ctx)) +} From 25e54ee1e68a7cb417c3196b3c9f30d808a60088 Mon Sep 17 00:00:00 2001 From: simon Date: Fri, 13 Mar 2026 08:32:21 +0100 Subject: [PATCH 04/10] Use plural resource types and add --url flag --- cmd/experimental/workspace_open.go | 59 +++++++--- cmd/experimental/workspace_open_test.go | 146 ++++++++++++++++++++---- 2 files changed, 164 insertions(+), 41 deletions(-) diff --git a/cmd/experimental/workspace_open.go b/cmd/experimental/workspace_open.go index c961d3fe80..7a6574cfd6 100644 --- a/cmd/experimental/workspace_open.go +++ b/cmd/experimental/workspace_open.go @@ -1,6 +1,7 @@ package experimental import ( + "context" "fmt" "net/url" "sort" @@ -18,14 +19,22 @@ import ( // resourceURLPatterns maps resource type names to their URL path patterns. // Patterns starting with "#" use URL fragments instead of path segments. var resourceURLPatterns = map[string]string{ - "job": "/jobs/%s", - "notebook": "#notebook/%s", - "cluster": "/compute/clusters/%s", - "pipeline": "/pipelines/%s", - "dashboard": "/dashboardsv3/%s/published", - "warehouse": "/sql/warehouses/%s", - "query": "/sql/editor/%s", - "app": "/apps/%s", + "apps": "/apps/%s", + "clusters": "/compute/clusters/%s", + "dashboards": "/dashboardsv3/%s/published", + "jobs": "/jobs/%s", + "notebooks": "#notebook/%s", + "pipelines": "/pipelines/%s", + "queries": "/sql/editor/%s", + "warehouses": "/sql/warehouses/%s", +} + +var currentWorkspaceID = func(ctx context.Context) (int64, error) { + return cmdctx.WorkspaceClient(ctx).CurrentWorkspaceID(ctx) +} + +var openWorkspaceURL = func(ctx context.Context, targetURL string) error { + return browser.OpenURL(ctx, ".", targetURL) } func resourceTypeNames() []string { @@ -37,11 +46,15 @@ func resourceTypeNames() []string { return names } +func supportedResourceTypes() string { + return strings.Join(resourceTypeNames(), ", ") +} + // buildWorkspaceURL constructs a full workspace URL for a given resource type and ID. func buildWorkspaceURL(host, resourceType, id string, workspaceID int64) (string, error) { pattern, ok := resourceURLPatterns[resourceType] if !ok { - return "", fmt.Errorf("unknown resource type %q, must be one of: %v", resourceType, resourceTypeNames()) + return "", fmt.Errorf("unknown resource type %q, must be one of: %s", resourceType, supportedResourceTypes()) } baseURL, err := url.Parse(host) @@ -80,17 +93,20 @@ func hasWorkspaceIDInHostname(hostname, workspaceID string) bool { } func newWorkspaceOpenCommand() *cobra.Command { + var printURL bool + cmd := &cobra.Command{ - Use: "open RESOURCE_TYPE ID_OR_PATH", - Short: "Open a workspace resource in the browser", - Long: `Open a workspace resource in the default browser. + Use: "open [flags] RESOURCE_TYPE ID_OR_PATH", + Short: "Open a workspace resource or print its URL", + Long: fmt.Sprintf(`Open a workspace resource in the default browser, or print its URL. -Supported resource types: job, notebook, cluster, pipeline, dashboard, warehouse, query, app. +Supported resource types: %s. Examples: - databricks experimental open job 123456789 - databricks experimental open notebook /Users/user@example.com/my-notebook - databricks experimental open cluster 0123-456789-abcdef`, + databricks experimental open jobs 123456789 + databricks experimental open notebooks /Users/user@example.com/my-notebook + databricks experimental open clusters 0123-456789-abcdef + databricks experimental open jobs 123456789 --url`, supportedResourceTypes()), Args: cobra.ExactArgs(2), PreRunE: root.MustWorkspaceClient, RunE: func(cmd *cobra.Command, args []string) error { @@ -100,7 +116,7 @@ Examples: resourceType := args[0] id := args[1] - workspaceID, err := w.CurrentWorkspaceID(ctx) + workspaceID, err := currentWorkspaceID(ctx) if err != nil { workspaceID = 0 } @@ -110,11 +126,16 @@ Examples: return err } + if printURL { + _, err := fmt.Fprintln(cmd.OutOrStdout(), resourceURL) + return err + } + if !browser.IsDisabled(ctx) { cmdio.LogString(ctx, fmt.Sprintf("Opening %s %s in the browser...", resourceType, id)) } - return browser.OpenURL(ctx, ".", resourceURL) + return openWorkspaceURL(ctx, resourceURL) }, ValidArgsFunction: func(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) { if len(args) == 0 { @@ -124,5 +145,7 @@ Examples: }, } + cmd.Flags().BoolVar(&printURL, "url", false, "Print the workspace URL instead of opening the browser") + return cmd } diff --git a/cmd/experimental/workspace_open_test.go b/cmd/experimental/workspace_open_test.go index c3195533d8..0f858ba7e1 100644 --- a/cmd/experimental/workspace_open_test.go +++ b/cmd/experimental/workspace_open_test.go @@ -1,8 +1,14 @@ package experimental import ( + "bytes" + "context" "testing" + "github.com/databricks/cli/libs/cmdctx" + "github.com/databricks/cli/libs/cmdio" + "github.com/databricks/databricks-sdk-go" + "github.com/databricks/databricks-sdk-go/config" "github.com/spf13/cobra" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -14,13 +20,13 @@ func TestBuildWorkspaceURLPathBasedResources(t *testing.T) { id string expected string }{ - {"job", "123", "https://myworkspace.databricks.com/jobs/123"}, - {"pipeline", "abc-def", "https://myworkspace.databricks.com/pipelines/abc-def"}, - {"dashboard", "dash-1", "https://myworkspace.databricks.com/dashboardsv3/dash-1/published"}, - {"warehouse", "wh-1", "https://myworkspace.databricks.com/sql/warehouses/wh-1"}, - {"query", "q-1", "https://myworkspace.databricks.com/sql/editor/q-1"}, - {"app", "my-app", "https://myworkspace.databricks.com/apps/my-app"}, - {"cluster", "0123-456789-abc", "https://myworkspace.databricks.com/compute/clusters/0123-456789-abc"}, + {"jobs", "123", "https://myworkspace.databricks.com/jobs/123"}, + {"pipelines", "abc-def", "https://myworkspace.databricks.com/pipelines/abc-def"}, + {"dashboards", "dash-1", "https://myworkspace.databricks.com/dashboardsv3/dash-1/published"}, + {"warehouses", "wh-1", "https://myworkspace.databricks.com/sql/warehouses/wh-1"}, + {"queries", "q-1", "https://myworkspace.databricks.com/sql/editor/q-1"}, + {"apps", "my-app", "https://myworkspace.databricks.com/apps/my-app"}, + {"clusters", "0123-456789-abc", "https://myworkspace.databricks.com/compute/clusters/0123-456789-abc"}, } for _, tt := range tests { @@ -38,8 +44,8 @@ func TestBuildWorkspaceURLFragmentBasedResources(t *testing.T) { id string expected string }{ - {"notebook", "12345", "https://myworkspace.databricks.com/#notebook/12345"}, - {"notebook", "/Users/user@example.com/my-notebook", "https://myworkspace.databricks.com/#notebook//Users/user@example.com/my-notebook"}, + {"notebooks", "12345", "https://myworkspace.databricks.com/#notebook/12345"}, + {"notebooks", "/Users/user@example.com/my-notebook", "https://myworkspace.databricks.com/#notebook//Users/user@example.com/my-notebook"}, } for _, tt := range tests { @@ -54,35 +60,36 @@ func TestBuildWorkspaceURLFragmentBasedResources(t *testing.T) { func TestBuildWorkspaceURLUnknownResourceType(t *testing.T) { _, err := buildWorkspaceURL("https://myworkspace.databricks.com", "unknown", "123", 0) assert.ErrorContains(t, err, "unknown resource type \"unknown\"") + assert.ErrorContains(t, err, "apps, clusters, dashboards, jobs, notebooks, pipelines, queries, warehouses") } func TestBuildWorkspaceURLHostWithTrailingSlash(t *testing.T) { - got, err := buildWorkspaceURL("https://myworkspace.databricks.com/", "job", "123", 0) + got, err := buildWorkspaceURL("https://myworkspace.databricks.com/", "jobs", "123", 0) require.NoError(t, err) assert.Equal(t, "https://myworkspace.databricks.com/jobs/123", got) } func TestBuildWorkspaceURLWithWorkspaceID(t *testing.T) { - got, err := buildWorkspaceURL("https://myworkspace.databricks.com", "job", "123", 123456) + got, err := buildWorkspaceURL("https://myworkspace.databricks.com", "jobs", "123", 123456) require.NoError(t, err) assert.Equal(t, "https://myworkspace.databricks.com/jobs/123?o=123456", got) } func TestBuildWorkspaceURLWithWorkspaceIDInHostname(t *testing.T) { - got, err := buildWorkspaceURL("https://adb-123456.azuredatabricks.net", "job", "123", 123456) + got, err := buildWorkspaceURL("https://adb-123456.azuredatabricks.net", "jobs", "123", 123456) require.NoError(t, err) // Workspace ID is already in the hostname, so ?o= should not be appended. assert.Equal(t, "https://adb-123456.azuredatabricks.net/jobs/123", got) } func TestBuildWorkspaceURLWithWorkspaceIDInVanityHostname(t *testing.T) { - got, err := buildWorkspaceURL("https://workspace-123456.example.com", "job", "123", 123456) + got, err := buildWorkspaceURL("https://workspace-123456.example.com", "jobs", "123", 123456) require.NoError(t, err) assert.Equal(t, "https://workspace-123456.example.com/jobs/123?o=123456", got) } func TestBuildWorkspaceURLFragmentWithWorkspaceID(t *testing.T) { - got, err := buildWorkspaceURL("https://myworkspace.databricks.com", "notebook", "12345", 789) + got, err := buildWorkspaceURL("https://myworkspace.databricks.com", "notebooks", "12345", 789) require.NoError(t, err) assert.Equal(t, "https://myworkspace.databricks.com/?o=789#notebook/12345", got) } @@ -92,21 +99,114 @@ func TestWorkspaceOpenCommandCompletion(t *testing.T) { completions, directive := cmd.ValidArgsFunction(cmd, []string{}, "") assert.Equal(t, cobra.ShellCompDirectiveNoFileComp, directive) - assert.Contains(t, completions, "job") - assert.Contains(t, completions, "notebook") - assert.Contains(t, completions, "cluster") - assert.Contains(t, completions, "pipeline") - assert.Contains(t, completions, "dashboard") - assert.Contains(t, completions, "warehouse") - assert.Contains(t, completions, "query") - assert.Contains(t, completions, "app") + assert.Contains(t, completions, "jobs") + assert.Contains(t, completions, "notebooks") + assert.Contains(t, completions, "clusters") + assert.Contains(t, completions, "pipelines") + assert.Contains(t, completions, "dashboards") + assert.Contains(t, completions, "warehouses") + assert.Contains(t, completions, "queries") + assert.Contains(t, completions, "apps") assert.Len(t, completions, 8) } func TestWorkspaceOpenCommandCompletionSecondArg(t *testing.T) { cmd := newWorkspaceOpenCommand() - completions, directive := cmd.ValidArgsFunction(cmd, []string{"job"}, "") + completions, directive := cmd.ValidArgsFunction(cmd, []string{"jobs"}, "") assert.Equal(t, cobra.ShellCompDirectiveNoFileComp, directive) assert.Nil(t, completions) } + +func TestWorkspaceOpenCommandHelpText(t *testing.T) { + cmd := newWorkspaceOpenCommand() + + assert.Contains(t, cmd.Long, "Supported resource types: apps, clusters, dashboards, jobs, notebooks, pipelines, queries, warehouses.") + assert.Contains(t, cmd.Long, "databricks experimental open jobs 123456789") + assert.Contains(t, cmd.Long, "databricks experimental open notebooks /Users/user@example.com/my-notebook") + assert.Contains(t, cmd.Long, "databricks experimental open jobs 123456789 --url") + + flag := cmd.Flags().Lookup("url") + require.NotNil(t, flag) + assert.Equal(t, "false", flag.DefValue) +} + +func TestWorkspaceOpenCommandOpensBrowserByDefault(t *testing.T) { + originalCurrentWorkspaceID := currentWorkspaceID + originalOpenWorkspaceURL := openWorkspaceURL + t.Cleanup(func() { + currentWorkspaceID = originalCurrentWorkspaceID + openWorkspaceURL = originalOpenWorkspaceURL + }) + + currentWorkspaceID = func(context.Context) (int64, error) { + return 0, nil + } + + var gotURL string + openWorkspaceURL = func(ctx context.Context, targetURL string) error { + gotURL = targetURL + return nil + } + + ctx, stderr := cmdio.NewTestContextWithStderr(t.Context()) + ctx = cmdctx.SetWorkspaceClient(ctx, &databricks.WorkspaceClient{ + Config: &config.Config{ + Host: "https://myworkspace.databricks.com", + }, + }) + + cmd := newWorkspaceOpenCommand() + cmd.SetContext(ctx) + + var stdout bytes.Buffer + cmd.SetOut(&stdout) + + err := cmd.RunE(cmd, []string{"jobs", "123"}) + require.NoError(t, err) + + assert.Equal(t, "https://myworkspace.databricks.com/jobs/123", gotURL) + assert.Equal(t, "", stdout.String()) + assert.Contains(t, stderr.String(), "Opening jobs 123 in the browser...") +} + +func TestWorkspaceOpenCommandURLFlag(t *testing.T) { + originalCurrentWorkspaceID := currentWorkspaceID + originalOpenWorkspaceURL := openWorkspaceURL + t.Cleanup(func() { + currentWorkspaceID = originalCurrentWorkspaceID + openWorkspaceURL = originalOpenWorkspaceURL + }) + + currentWorkspaceID = func(context.Context) (int64, error) { + return 789, nil + } + + browserOpened := false + openWorkspaceURL = func(ctx context.Context, targetURL string) error { + browserOpened = true + return nil + } + + ctx, stderr := cmdio.NewTestContextWithStderr(t.Context()) + ctx = cmdctx.SetWorkspaceClient(ctx, &databricks.WorkspaceClient{ + Config: &config.Config{ + Host: "https://myworkspace.databricks.com", + }, + }) + + cmd := newWorkspaceOpenCommand() + cmd.SetContext(ctx) + + var stdout bytes.Buffer + cmd.SetOut(&stdout) + + require.NoError(t, cmd.Flags().Set("url", "true")) + + err := cmd.RunE(cmd, []string{"jobs", "123"}) + require.NoError(t, err) + + assert.False(t, browserOpened) + assert.Equal(t, "https://myworkspace.databricks.com/jobs/123?o=789\n", stdout.String()) + assert.Equal(t, "", stderr.String()) +} From c514e85a7c523a9601493d5d92ae76ca9720a926 Mon Sep 17 00:00:00 2001 From: simon Date: Fri, 13 Mar 2026 09:22:31 +0100 Subject: [PATCH 05/10] Add experiments resource type --- cmd/experimental/workspace_open.go | 20 +++++++++++--------- cmd/experimental/workspace_open_test.go | 8 +++++--- 2 files changed, 16 insertions(+), 12 deletions(-) diff --git a/cmd/experimental/workspace_open.go b/cmd/experimental/workspace_open.go index 7a6574cfd6..e8806f1b8b 100644 --- a/cmd/experimental/workspace_open.go +++ b/cmd/experimental/workspace_open.go @@ -16,17 +16,19 @@ import ( "github.com/databricks/cli/libs/cmdio" ) -// resourceURLPatterns maps resource type names to their URL path patterns. +// resourceURLPatterns is a hardcoded list of known resource types and their URL path patterns. // Patterns starting with "#" use URL fragments instead of path segments. +// bundle open uses a similar hardcoded approach via InitializeURL methods on each resource type. var resourceURLPatterns = map[string]string{ - "apps": "/apps/%s", - "clusters": "/compute/clusters/%s", - "dashboards": "/dashboardsv3/%s/published", - "jobs": "/jobs/%s", - "notebooks": "#notebook/%s", - "pipelines": "/pipelines/%s", - "queries": "/sql/editor/%s", - "warehouses": "/sql/warehouses/%s", + "apps": "/apps/%s", + "clusters": "/compute/clusters/%s", + "dashboards": "/dashboardsv3/%s/published", + "experiments": "ml/experiments/%s", + "jobs": "/jobs/%s", + "notebooks": "#notebook/%s", + "pipelines": "/pipelines/%s", + "queries": "/sql/editor/%s", + "warehouses": "/sql/warehouses/%s", } var currentWorkspaceID = func(ctx context.Context) (int64, error) { diff --git a/cmd/experimental/workspace_open_test.go b/cmd/experimental/workspace_open_test.go index 0f858ba7e1..a0af09b97c 100644 --- a/cmd/experimental/workspace_open_test.go +++ b/cmd/experimental/workspace_open_test.go @@ -23,6 +23,7 @@ func TestBuildWorkspaceURLPathBasedResources(t *testing.T) { {"jobs", "123", "https://myworkspace.databricks.com/jobs/123"}, {"pipelines", "abc-def", "https://myworkspace.databricks.com/pipelines/abc-def"}, {"dashboards", "dash-1", "https://myworkspace.databricks.com/dashboardsv3/dash-1/published"}, + {"experiments", "exp-1", "https://myworkspace.databricks.com/ml/experiments/exp-1"}, {"warehouses", "wh-1", "https://myworkspace.databricks.com/sql/warehouses/wh-1"}, {"queries", "q-1", "https://myworkspace.databricks.com/sql/editor/q-1"}, {"apps", "my-app", "https://myworkspace.databricks.com/apps/my-app"}, @@ -60,7 +61,7 @@ func TestBuildWorkspaceURLFragmentBasedResources(t *testing.T) { func TestBuildWorkspaceURLUnknownResourceType(t *testing.T) { _, err := buildWorkspaceURL("https://myworkspace.databricks.com", "unknown", "123", 0) assert.ErrorContains(t, err, "unknown resource type \"unknown\"") - assert.ErrorContains(t, err, "apps, clusters, dashboards, jobs, notebooks, pipelines, queries, warehouses") + assert.ErrorContains(t, err, "apps, clusters, dashboards, experiments, jobs, notebooks, pipelines, queries, warehouses") } func TestBuildWorkspaceURLHostWithTrailingSlash(t *testing.T) { @@ -104,10 +105,11 @@ func TestWorkspaceOpenCommandCompletion(t *testing.T) { assert.Contains(t, completions, "clusters") assert.Contains(t, completions, "pipelines") assert.Contains(t, completions, "dashboards") + assert.Contains(t, completions, "experiments") assert.Contains(t, completions, "warehouses") assert.Contains(t, completions, "queries") assert.Contains(t, completions, "apps") - assert.Len(t, completions, 8) + assert.Len(t, completions, 9) } func TestWorkspaceOpenCommandCompletionSecondArg(t *testing.T) { @@ -121,7 +123,7 @@ func TestWorkspaceOpenCommandCompletionSecondArg(t *testing.T) { func TestWorkspaceOpenCommandHelpText(t *testing.T) { cmd := newWorkspaceOpenCommand() - assert.Contains(t, cmd.Long, "Supported resource types: apps, clusters, dashboards, jobs, notebooks, pipelines, queries, warehouses.") + assert.Contains(t, cmd.Long, "Supported resource types: apps, clusters, dashboards, experiments, jobs, notebooks, pipelines, queries, warehouses.") assert.Contains(t, cmd.Long, "databricks experimental open jobs 123456789") assert.Contains(t, cmd.Long, "databricks experimental open notebooks /Users/user@example.com/my-notebook") assert.Contains(t, cmd.Long, "databricks experimental open jobs 123456789 --url") From 59d29593c8f5da4782188572451dec7ec81b5a3d Mon Sep 17 00:00:00 2001 From: simon Date: Fri, 13 Mar 2026 09:54:16 +0100 Subject: [PATCH 06/10] Share workspace URL helpers between experimental and bundle open Move shared workspace UI routes and base URL synthesis into libs/workspaceurls so experimental open and bundle resource URLs stay aligned, including vanity hosts that still require the ?o workspace suffix. --- bundle/config/mutator/initialize_urls.go | 26 +--- bundle/config/mutator/initialize_urls_test.go | 24 +++- bundle/config/resources/alerts.go | 4 +- bundle/config/resources/apps.go | 4 +- bundle/config/resources/clusters.go | 4 +- bundle/config/resources/dashboard.go | 5 +- bundle/config/resources/job.go | 4 +- bundle/config/resources/mlflow_experiment.go | 4 +- bundle/config/resources/mlflow_model.go | 4 +- .../resources/model_serving_endpoint.go | 4 +- bundle/config/resources/pipeline.go | 4 +- bundle/config/resources/registered_model.go | 8 +- bundle/config/resources/sql_warehouses.go | 4 +- cmd/experimental/workspace_open.go | 73 +++-------- libs/workspaceurls/urls.go | 121 ++++++++++++++++++ 15 files changed, 192 insertions(+), 101 deletions(-) create mode 100644 libs/workspaceurls/urls.go diff --git a/bundle/config/mutator/initialize_urls.go b/bundle/config/mutator/initialize_urls.go index 35ff53d0b6..4d8e91e3c1 100644 --- a/bundle/config/mutator/initialize_urls.go +++ b/bundle/config/mutator/initialize_urls.go @@ -2,12 +2,10 @@ package mutator import ( "context" - "net/url" - "strconv" - "strings" "github.com/databricks/cli/bundle" "github.com/databricks/cli/libs/diag" + "github.com/databricks/cli/libs/workspaceurls" ) type initializeURLs struct{} @@ -25,38 +23,24 @@ func (m *initializeURLs) Name() string { } func (m *initializeURLs) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { - workspaceId, err := b.WorkspaceClient().CurrentWorkspaceID(ctx) + workspaceID, err := b.WorkspaceClient().CurrentWorkspaceID(ctx) if err != nil { return diag.FromErr(err) } - orgId := strconv.FormatInt(workspaceId, 10) host := b.WorkspaceClient().Config.CanonicalHostName() - err = initializeForWorkspace(b, orgId, host) + err = initializeForWorkspace(b, workspaceID, host) if err != nil { return diag.FromErr(err) } return nil } -func initializeForWorkspace(b *bundle.Bundle, orgId, host string) error { - baseURL, err := url.Parse(host) +func initializeForWorkspace(b *bundle.Bundle, workspaceID int64, host string) error { + baseURL, err := workspaceurls.WorkspaceBaseURL(host, workspaceID) if err != nil { return err } - // Add ?o= only if wasn't in the subdomain already. - // The ?o= is needed when vanity URLs / legacy workspace URLs are used. - // If it's not needed we prefer to leave it out since these URLs are rather - // long for most terminals. - // - // See https://docs.databricks.com/en/workspace/workspace-details.html for - // further reading about the '?o=' suffix. - if !strings.Contains(baseURL.Hostname(), orgId) { - values := baseURL.Query() - values.Add("o", orgId) - baseURL.RawQuery = values.Encode() - } - for _, group := range b.Config.Resources.AllResources() { for _, r := range group.Resources { r.InitializeURL(*baseURL) diff --git a/bundle/config/mutator/initialize_urls_test.go b/bundle/config/mutator/initialize_urls_test.go index 6b0c8d6655..9444ff1ad9 100644 --- a/bundle/config/mutator/initialize_urls_test.go +++ b/bundle/config/mutator/initialize_urls_test.go @@ -109,7 +109,7 @@ func TestInitializeURLs(t *testing.T) { "dashboard1": "https://mycompany.databricks.com/dashboardsv3/01ef8d56871e1d50ae30ce7375e42478/published?o=123456", } - err := initializeForWorkspace(b, "123456", "https://mycompany.databricks.com/") + err := initializeForWorkspace(b, 123456, "https://mycompany.databricks.com/") require.NoError(t, err) for _, group := range b.Config.Resources.AllResources() { @@ -133,8 +133,28 @@ func TestInitializeURLsWithoutOrgId(t *testing.T) { }, } - err := initializeForWorkspace(b, "123456", "https://adb-123456.azuredatabricks.net/") + err := initializeForWorkspace(b, 123456, "https://adb-123456.azuredatabricks.net/") require.NoError(t, err) require.Equal(t, "https://adb-123456.azuredatabricks.net/jobs/1", b.Config.Resources.Jobs["job1"].URL) } + +func TestInitializeURLsWithWorkspaceIDInVanityHostname(t *testing.T) { + b := &bundle.Bundle{ + Config: config.Root{ + Resources: config.Resources{ + Jobs: map[string]*resources.Job{ + "job1": { + BaseResource: resources.BaseResource{ID: "1"}, + JobSettings: jobs.JobSettings{Name: "job1"}, + }, + }, + }, + }, + } + + err := initializeForWorkspace(b, 123456, "https://workspace-123456.example.com/") + require.NoError(t, err) + + require.Equal(t, "https://workspace-123456.example.com/jobs/1?o=123456", b.Config.Resources.Jobs["job1"].URL) +} diff --git a/bundle/config/resources/alerts.go b/bundle/config/resources/alerts.go index bfdd64e900..f1d81c8c2f 100644 --- a/bundle/config/resources/alerts.go +++ b/bundle/config/resources/alerts.go @@ -5,6 +5,7 @@ import ( "net/url" "github.com/databricks/cli/libs/log" + "github.com/databricks/cli/libs/workspaceurls" "github.com/databricks/databricks-sdk-go" "github.com/databricks/databricks-sdk-go/marshal" "github.com/databricks/databricks-sdk-go/service/sql" @@ -52,8 +53,7 @@ func (a *Alert) InitializeURL(baseURL url.URL) { if a.ID == "" { return } - baseURL.Path = "sql/alerts-v2/" + a.ID - a.URL = baseURL.String() + a.URL = workspaceurls.ResourceURL(baseURL, workspaceurls.AlertPattern, a.ID) } func (a *Alert) GetName() string { diff --git a/bundle/config/resources/apps.go b/bundle/config/resources/apps.go index e48f6e7dae..ee05be7c9f 100644 --- a/bundle/config/resources/apps.go +++ b/bundle/config/resources/apps.go @@ -5,6 +5,7 @@ import ( "net/url" "github.com/databricks/cli/libs/log" + "github.com/databricks/cli/libs/workspaceurls" "github.com/databricks/databricks-sdk-go" "github.com/databricks/databricks-sdk-go/marshal" "github.com/databricks/databricks-sdk-go/service/apps" @@ -83,8 +84,7 @@ func (a *App) InitializeURL(baseURL url.URL) { if a.ModifiedStatus == "" || a.ModifiedStatus == ModifiedStatusCreated { return } - baseURL.Path = "apps/" + a.GetName() - a.URL = baseURL.String() + a.URL = workspaceurls.ResourceURL(baseURL, workspaceurls.AppPattern, a.GetName()) } func (a *App) GetName() string { diff --git a/bundle/config/resources/clusters.go b/bundle/config/resources/clusters.go index c549ac4a6b..89c6319c2e 100644 --- a/bundle/config/resources/clusters.go +++ b/bundle/config/resources/clusters.go @@ -5,6 +5,7 @@ import ( "net/url" "github.com/databricks/cli/libs/log" + "github.com/databricks/cli/libs/workspaceurls" "github.com/databricks/databricks-sdk-go" "github.com/databricks/databricks-sdk-go/marshal" "github.com/databricks/databricks-sdk-go/service/compute" @@ -47,8 +48,7 @@ func (s *Cluster) InitializeURL(baseURL url.URL) { if s.ID == "" { return } - baseURL.Path = "compute/clusters/" + s.ID - s.URL = baseURL.String() + s.URL = workspaceurls.ResourceURL(baseURL, workspaceurls.ClusterPattern, s.ID) } func (s *Cluster) GetName() string { diff --git a/bundle/config/resources/dashboard.go b/bundle/config/resources/dashboard.go index c108ac8abe..94c5cbec5d 100644 --- a/bundle/config/resources/dashboard.go +++ b/bundle/config/resources/dashboard.go @@ -2,10 +2,10 @@ package resources import ( "context" - "fmt" "net/url" "github.com/databricks/cli/libs/log" + "github.com/databricks/cli/libs/workspaceurls" "github.com/databricks/databricks-sdk-go" "github.com/databricks/databricks-sdk-go/marshal" "github.com/databricks/databricks-sdk-go/service/dashboards" @@ -114,8 +114,7 @@ func (r *Dashboard) InitializeURL(baseURL url.URL) { return } - baseURL.Path = fmt.Sprintf("dashboardsv3/%s/published", r.ID) - r.URL = baseURL.String() + r.URL = workspaceurls.ResourceURL(baseURL, workspaceurls.DashboardPattern, r.ID) } func (r *Dashboard) GetName() string { diff --git a/bundle/config/resources/job.go b/bundle/config/resources/job.go index b2b7ff15f1..67dff8b6cc 100644 --- a/bundle/config/resources/job.go +++ b/bundle/config/resources/job.go @@ -6,6 +6,7 @@ import ( "strconv" "github.com/databricks/cli/libs/log" + "github.com/databricks/cli/libs/workspaceurls" "github.com/databricks/databricks-sdk-go" "github.com/databricks/databricks-sdk-go/marshal" "github.com/databricks/databricks-sdk-go/service/jobs" @@ -54,8 +55,7 @@ func (j *Job) InitializeURL(baseURL url.URL) { if j.ID == "" { return } - baseURL.Path = "jobs/" + j.ID - j.URL = baseURL.String() + j.URL = workspaceurls.ResourceURL(baseURL, workspaceurls.JobPattern, j.ID) } func (j *Job) GetName() string { diff --git a/bundle/config/resources/mlflow_experiment.go b/bundle/config/resources/mlflow_experiment.go index 0a2a36b840..75af42f435 100644 --- a/bundle/config/resources/mlflow_experiment.go +++ b/bundle/config/resources/mlflow_experiment.go @@ -5,6 +5,7 @@ import ( "net/url" "github.com/databricks/cli/libs/log" + "github.com/databricks/cli/libs/workspaceurls" "github.com/databricks/databricks-sdk-go" "github.com/databricks/databricks-sdk-go/marshal" "github.com/databricks/databricks-sdk-go/service/ml" @@ -49,8 +50,7 @@ func (s *MlflowExperiment) InitializeURL(baseURL url.URL) { if s.ID == "" { return } - baseURL.Path = "ml/experiments/" + s.ID - s.URL = baseURL.String() + s.URL = workspaceurls.ResourceURL(baseURL, workspaceurls.ExperimentPattern, s.ID) } func (s *MlflowExperiment) GetName() string { diff --git a/bundle/config/resources/mlflow_model.go b/bundle/config/resources/mlflow_model.go index a867f55a0e..9b739b8787 100644 --- a/bundle/config/resources/mlflow_model.go +++ b/bundle/config/resources/mlflow_model.go @@ -5,6 +5,7 @@ import ( "net/url" "github.com/databricks/cli/libs/log" + "github.com/databricks/cli/libs/workspaceurls" "github.com/databricks/databricks-sdk-go" "github.com/databricks/databricks-sdk-go/marshal" "github.com/databricks/databricks-sdk-go/service/ml" @@ -49,8 +50,7 @@ func (s *MlflowModel) InitializeURL(baseURL url.URL) { if s.ID == "" { return } - baseURL.Path = "ml/models/" + s.ID - s.URL = baseURL.String() + s.URL = workspaceurls.ResourceURL(baseURL, workspaceurls.ModelPattern, s.ID) } func (s *MlflowModel) GetName() string { diff --git a/bundle/config/resources/model_serving_endpoint.go b/bundle/config/resources/model_serving_endpoint.go index d3e390596d..06d39f7093 100644 --- a/bundle/config/resources/model_serving_endpoint.go +++ b/bundle/config/resources/model_serving_endpoint.go @@ -5,6 +5,7 @@ import ( "net/url" "github.com/databricks/cli/libs/log" + "github.com/databricks/cli/libs/workspaceurls" "github.com/databricks/databricks-sdk-go" "github.com/databricks/databricks-sdk-go/marshal" "github.com/databricks/databricks-sdk-go/service/serving" @@ -54,8 +55,7 @@ func (s *ModelServingEndpoint) InitializeURL(baseURL url.URL) { if s.ID == "" { return } - baseURL.Path = "ml/endpoints/" + s.ID - s.URL = baseURL.String() + s.URL = workspaceurls.ResourceURL(baseURL, workspaceurls.ModelServingEndpointPattern, s.ID) } func (s *ModelServingEndpoint) GetName() string { diff --git a/bundle/config/resources/pipeline.go b/bundle/config/resources/pipeline.go index e213ff9c00..90d9b4c071 100644 --- a/bundle/config/resources/pipeline.go +++ b/bundle/config/resources/pipeline.go @@ -5,6 +5,7 @@ import ( "net/url" "github.com/databricks/cli/libs/log" + "github.com/databricks/cli/libs/workspaceurls" "github.com/databricks/databricks-sdk-go" "github.com/databricks/databricks-sdk-go/marshal" "github.com/databricks/databricks-sdk-go/service/pipelines" @@ -49,8 +50,7 @@ func (p *Pipeline) InitializeURL(baseURL url.URL) { if p.ID == "" { return } - baseURL.Path = "pipelines/" + p.ID - p.URL = baseURL.String() + p.URL = workspaceurls.ResourceURL(baseURL, workspaceurls.PipelinePattern, p.ID) } func (p *Pipeline) GetName() string { diff --git a/bundle/config/resources/registered_model.go b/bundle/config/resources/registered_model.go index c51450bf74..d30886ae9b 100644 --- a/bundle/config/resources/registered_model.go +++ b/bundle/config/resources/registered_model.go @@ -6,6 +6,7 @@ import ( "strings" "github.com/databricks/cli/libs/log" + "github.com/databricks/cli/libs/workspaceurls" "github.com/databricks/databricks-sdk-go" "github.com/databricks/databricks-sdk-go/marshal" "github.com/databricks/databricks-sdk-go/service/catalog" @@ -54,8 +55,11 @@ func (s *RegisteredModel) InitializeURL(baseURL url.URL) { if s.ID == "" { return } - baseURL.Path = "explore/data/models/" + strings.ReplaceAll(s.ID, ".", "/") - s.URL = baseURL.String() + s.URL = workspaceurls.ResourceURL( + baseURL, + workspaceurls.RegisteredModelPattern, + strings.ReplaceAll(s.ID, ".", "/"), + ) } func (s *RegisteredModel) GetName() string { diff --git a/bundle/config/resources/sql_warehouses.go b/bundle/config/resources/sql_warehouses.go index bed567b805..4841c7387e 100644 --- a/bundle/config/resources/sql_warehouses.go +++ b/bundle/config/resources/sql_warehouses.go @@ -5,6 +5,7 @@ import ( "net/url" "github.com/databricks/cli/libs/log" + "github.com/databricks/cli/libs/workspaceurls" "github.com/databricks/databricks-sdk-go" "github.com/databricks/databricks-sdk-go/marshal" "github.com/databricks/databricks-sdk-go/service/sql" @@ -47,8 +48,7 @@ func (sw *SqlWarehouse) InitializeURL(baseURL url.URL) { if sw.ID == "" { return } - baseURL.Path = "sql/warehouses/" + sw.ID - sw.URL = baseURL.String() + sw.URL = workspaceurls.ResourceURL(baseURL, workspaceurls.WarehousePattern, sw.ID) } func (sw *SqlWarehouse) GetName() string { diff --git a/cmd/experimental/workspace_open.go b/cmd/experimental/workspace_open.go index e8806f1b8b..4136ae562e 100644 --- a/cmd/experimental/workspace_open.go +++ b/cmd/experimental/workspace_open.go @@ -3,9 +3,7 @@ package experimental import ( "context" "fmt" - "net/url" - "sort" - "strconv" + "slices" "strings" "github.com/spf13/cobra" @@ -14,21 +12,19 @@ import ( "github.com/databricks/cli/libs/browser" "github.com/databricks/cli/libs/cmdctx" "github.com/databricks/cli/libs/cmdio" + "github.com/databricks/cli/libs/workspaceurls" ) -// resourceURLPatterns is a hardcoded list of known resource types and their URL path patterns. -// Patterns starting with "#" use URL fragments instead of path segments. -// bundle open uses a similar hardcoded approach via InitializeURL methods on each resource type. -var resourceURLPatterns = map[string]string{ - "apps": "/apps/%s", - "clusters": "/compute/clusters/%s", - "dashboards": "/dashboardsv3/%s/published", - "experiments": "ml/experiments/%s", - "jobs": "/jobs/%s", - "notebooks": "#notebook/%s", - "pipelines": "/pipelines/%s", - "queries": "/sql/editor/%s", - "warehouses": "/sql/warehouses/%s", +var supportedOpenResourceTypes = []string{ + workspaceurls.ResourceApps, + workspaceurls.ResourceClusters, + workspaceurls.ResourceDashboards, + workspaceurls.ResourceExperiments, + workspaceurls.ResourceJobs, + workspaceurls.ResourceNotebooks, + workspaceurls.ResourcePipelines, + workspaceurls.ResourceQueries, + workspaceurls.ResourceWarehouses, } var currentWorkspaceID = func(ctx context.Context) (int64, error) { @@ -40,12 +36,7 @@ var openWorkspaceURL = func(ctx context.Context, targetURL string) error { } func resourceTypeNames() []string { - names := make([]string, 0, len(resourceURLPatterns)) - for name := range resourceURLPatterns { - names = append(names, name) - } - sort.Strings(names) - return names + return workspaceurls.SortResourceTypes(supportedOpenResourceTypes) } func supportedResourceTypes() string { @@ -54,44 +45,16 @@ func supportedResourceTypes() string { // buildWorkspaceURL constructs a full workspace URL for a given resource type and ID. func buildWorkspaceURL(host, resourceType, id string, workspaceID int64) (string, error) { - pattern, ok := resourceURLPatterns[resourceType] - if !ok { + if !slices.Contains(supportedOpenResourceTypes, resourceType) { return "", fmt.Errorf("unknown resource type %q, must be one of: %s", resourceType, supportedResourceTypes()) } - baseURL, err := url.Parse(host) - if err != nil { - return "", fmt.Errorf("invalid workspace host %q: %w", host, err) - } - - // Append ?o= when the workspace ID is not already in the - // hostname (e.g. vanity URLs or legacy workspace URLs). - // See https://docs.databricks.com/en/workspace/workspace-details.html - if workspaceID != 0 { - orgID := strconv.FormatInt(workspaceID, 10) - if !hasWorkspaceIDInHostname(baseURL.Hostname(), orgID) { - values := baseURL.Query() - values.Add("o", orgID) - baseURL.RawQuery = values.Encode() - } - } - - resourcePath := fmt.Sprintf(pattern, id) - - // Fragment-based URLs (starting with #) need special handling. - if len(resourcePath) > 0 && resourcePath[0] == '#' { - baseURL.Path = "/" - baseURL.Fragment = resourcePath[1:] - } else { - baseURL.Path = resourcePath + pattern, ok := workspaceurls.LookupPattern(resourceType) + if !ok { + return "", fmt.Errorf("unknown resource type %q, must be one of: %s", resourceType, supportedResourceTypes()) } - return baseURL.String(), nil -} - -func hasWorkspaceIDInHostname(hostname, workspaceID string) bool { - remainder, ok := strings.CutPrefix(strings.ToLower(hostname), "adb-"+workspaceID) - return ok && (remainder == "" || strings.HasPrefix(remainder, ".")) + return workspaceurls.BuildResourceURL(host, pattern, id, workspaceID) } func newWorkspaceOpenCommand() *cobra.Command { diff --git a/libs/workspaceurls/urls.go b/libs/workspaceurls/urls.go new file mode 100644 index 0000000000..b9a1892263 --- /dev/null +++ b/libs/workspaceurls/urls.go @@ -0,0 +1,121 @@ +package workspaceurls + +import ( + "fmt" + "net/url" + "slices" + "strconv" + "strings" +) + +const ( + ResourceAlerts = "alerts" + ResourceApps = "apps" + ResourceClusters = "clusters" + ResourceDashboards = "dashboards" + ResourceExperiments = "experiments" + ResourceJobs = "jobs" + ResourceModels = "models" + ResourceModelServingEndpoints = "model_serving_endpoints" + ResourceNotebooks = "notebooks" + ResourcePipelines = "pipelines" + ResourceQueries = "queries" + ResourceRegisteredModels = "registered_models" + ResourceWarehouses = "warehouses" +) + +const ( + AlertPattern = "sql/alerts-v2/%s" + AppPattern = "apps/%s" + ClusterPattern = "compute/clusters/%s" + DashboardPattern = "dashboardsv3/%s/published" + ExperimentPattern = "ml/experiments/%s" + JobPattern = "jobs/%s" + ModelPattern = "ml/models/%s" + ModelServingEndpointPattern = "ml/endpoints/%s" + NotebookPattern = "#notebook/%s" + PipelinePattern = "pipelines/%s" + QueryPattern = "sql/editor/%s" + RegisteredModelPattern = "explore/data/models/%s" + WarehousePattern = "sql/warehouses/%s" +) + +var resourceURLPatterns = map[string]string{ + ResourceAlerts: AlertPattern, + ResourceApps: AppPattern, + ResourceClusters: ClusterPattern, + ResourceDashboards: DashboardPattern, + ResourceExperiments: ExperimentPattern, + ResourceJobs: JobPattern, + ResourceModels: ModelPattern, + ResourceModelServingEndpoints: ModelServingEndpointPattern, + ResourceNotebooks: NotebookPattern, + ResourcePipelines: PipelinePattern, + ResourceQueries: QueryPattern, + ResourceRegisteredModels: RegisteredModelPattern, + ResourceWarehouses: WarehousePattern, +} + +// LookupPattern returns the workspace URL pattern for a resource type. +func LookupPattern(resourceType string) (string, bool) { + pattern, ok := resourceURLPatterns[resourceType] + return pattern, ok +} + +// SortResourceTypes returns a sorted copy of resource types. +func SortResourceTypes(resourceTypes []string) []string { + names := append([]string(nil), resourceTypes...) + slices.Sort(names) + return names +} + +// WorkspaceBaseURL parses a workspace host and appends ?o= when needed. +func WorkspaceBaseURL(host string, workspaceID int64) (*url.URL, error) { + baseURL, err := url.Parse(host) + if err != nil { + return nil, fmt.Errorf("invalid workspace host %q: %w", host, err) + } + + if workspaceID == 0 { + return baseURL, nil + } + + orgID := strconv.FormatInt(workspaceID, 10) + if hasWorkspaceIDInHostname(baseURL.Hostname(), orgID) { + return baseURL, nil + } + + values := baseURL.Query() + values.Add("o", orgID) + baseURL.RawQuery = values.Encode() + + return baseURL, nil +} + +// BuildResourceURL constructs a full workspace URL for a resource path pattern. +func BuildResourceURL(host, pattern, id string, workspaceID int64) (string, error) { + baseURL, err := WorkspaceBaseURL(host, workspaceID) + if err != nil { + return "", err + } + + return ResourceURL(*baseURL, pattern, id), nil +} + +// ResourceURL formats a workspace URL for a resource path pattern. +func ResourceURL(baseURL url.URL, pattern, id string) string { + resourcePath := fmt.Sprintf(pattern, id) + if strings.HasPrefix(resourcePath, "#") { + baseURL.Path = "/" + baseURL.Fragment = resourcePath[1:] + } else { + baseURL.Path = resourcePath + } + + return baseURL.String() +} + +func hasWorkspaceIDInHostname(hostname, workspaceID string) bool { + remainder, ok := strings.CutPrefix(strings.ToLower(hostname), "adb-"+workspaceID) + return ok && (remainder == "" || strings.HasPrefix(remainder, ".")) +} From c5ae950f3d5d5a4165e6ee2b2ab550f4db5ef79f Mon Sep 17 00:00:00 2001 From: simon Date: Fri, 13 Mar 2026 12:22:42 +0100 Subject: [PATCH 07/10] Revert bundle initialize_urls changes to avoid URL output regression The shared workspaceurls lib uses a stricter adb- hostname check for workspace ID detection. This is correct but changes bundle summary URL output on non-Azure workspaces (adds ?o= where it was previously omitted due to a loose strings.Contains match). That behavior change should be a separate PR with its own integration test updates. experimental open still uses the shared lib with the strict check. --- bundle/config/mutator/initialize_urls.go | 26 +++++++++++++++---- bundle/config/mutator/initialize_urls_test.go | 24 ++--------------- 2 files changed, 23 insertions(+), 27 deletions(-) diff --git a/bundle/config/mutator/initialize_urls.go b/bundle/config/mutator/initialize_urls.go index 4d8e91e3c1..35ff53d0b6 100644 --- a/bundle/config/mutator/initialize_urls.go +++ b/bundle/config/mutator/initialize_urls.go @@ -2,10 +2,12 @@ package mutator import ( "context" + "net/url" + "strconv" + "strings" "github.com/databricks/cli/bundle" "github.com/databricks/cli/libs/diag" - "github.com/databricks/cli/libs/workspaceurls" ) type initializeURLs struct{} @@ -23,24 +25,38 @@ func (m *initializeURLs) Name() string { } func (m *initializeURLs) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { - workspaceID, err := b.WorkspaceClient().CurrentWorkspaceID(ctx) + workspaceId, err := b.WorkspaceClient().CurrentWorkspaceID(ctx) if err != nil { return diag.FromErr(err) } + orgId := strconv.FormatInt(workspaceId, 10) host := b.WorkspaceClient().Config.CanonicalHostName() - err = initializeForWorkspace(b, workspaceID, host) + err = initializeForWorkspace(b, orgId, host) if err != nil { return diag.FromErr(err) } return nil } -func initializeForWorkspace(b *bundle.Bundle, workspaceID int64, host string) error { - baseURL, err := workspaceurls.WorkspaceBaseURL(host, workspaceID) +func initializeForWorkspace(b *bundle.Bundle, orgId, host string) error { + baseURL, err := url.Parse(host) if err != nil { return err } + // Add ?o= only if wasn't in the subdomain already. + // The ?o= is needed when vanity URLs / legacy workspace URLs are used. + // If it's not needed we prefer to leave it out since these URLs are rather + // long for most terminals. + // + // See https://docs.databricks.com/en/workspace/workspace-details.html for + // further reading about the '?o=' suffix. + if !strings.Contains(baseURL.Hostname(), orgId) { + values := baseURL.Query() + values.Add("o", orgId) + baseURL.RawQuery = values.Encode() + } + for _, group := range b.Config.Resources.AllResources() { for _, r := range group.Resources { r.InitializeURL(*baseURL) diff --git a/bundle/config/mutator/initialize_urls_test.go b/bundle/config/mutator/initialize_urls_test.go index 9444ff1ad9..6b0c8d6655 100644 --- a/bundle/config/mutator/initialize_urls_test.go +++ b/bundle/config/mutator/initialize_urls_test.go @@ -109,7 +109,7 @@ func TestInitializeURLs(t *testing.T) { "dashboard1": "https://mycompany.databricks.com/dashboardsv3/01ef8d56871e1d50ae30ce7375e42478/published?o=123456", } - err := initializeForWorkspace(b, 123456, "https://mycompany.databricks.com/") + err := initializeForWorkspace(b, "123456", "https://mycompany.databricks.com/") require.NoError(t, err) for _, group := range b.Config.Resources.AllResources() { @@ -133,28 +133,8 @@ func TestInitializeURLsWithoutOrgId(t *testing.T) { }, } - err := initializeForWorkspace(b, 123456, "https://adb-123456.azuredatabricks.net/") + err := initializeForWorkspace(b, "123456", "https://adb-123456.azuredatabricks.net/") require.NoError(t, err) require.Equal(t, "https://adb-123456.azuredatabricks.net/jobs/1", b.Config.Resources.Jobs["job1"].URL) } - -func TestInitializeURLsWithWorkspaceIDInVanityHostname(t *testing.T) { - b := &bundle.Bundle{ - Config: config.Root{ - Resources: config.Resources{ - Jobs: map[string]*resources.Job{ - "job1": { - BaseResource: resources.BaseResource{ID: "1"}, - JobSettings: jobs.JobSettings{Name: "job1"}, - }, - }, - }, - }, - } - - err := initializeForWorkspace(b, 123456, "https://workspace-123456.example.com/") - require.NoError(t, err) - - require.Equal(t, "https://workspace-123456.example.com/jobs/1?o=123456", b.Config.Resources.Jobs["job1"].URL) -} From d2d9e608c8c247b253567232a08988b427fca840 Mon Sep 17 00:00:00 2001 From: simon Date: Fri, 13 Mar 2026 13:35:58 +0100 Subject: [PATCH 08/10] Handle multi-argument BROWSER and warn on workspace ID lookup failure --- cmd/experimental/workspace_open.go | 3 +- cmd/experimental/workspace_open_test.go | 45 +++++++++++++++++++++++++ libs/browser/open.go | 33 ++++++++---------- libs/browser/open_test.go | 8 ++--- 4 files changed, 65 insertions(+), 24 deletions(-) diff --git a/cmd/experimental/workspace_open.go b/cmd/experimental/workspace_open.go index 4136ae562e..80cce99018 100644 --- a/cmd/experimental/workspace_open.go +++ b/cmd/experimental/workspace_open.go @@ -12,6 +12,7 @@ import ( "github.com/databricks/cli/libs/browser" "github.com/databricks/cli/libs/cmdctx" "github.com/databricks/cli/libs/cmdio" + "github.com/databricks/cli/libs/log" "github.com/databricks/cli/libs/workspaceurls" ) @@ -83,7 +84,7 @@ Examples: workspaceID, err := currentWorkspaceID(ctx) if err != nil { - workspaceID = 0 + log.Warnf(ctx, "Could not determine workspace ID: %v", err) } resourceURL, err := buildWorkspaceURL(w.Config.Host, resourceType, id, workspaceID) diff --git a/cmd/experimental/workspace_open_test.go b/cmd/experimental/workspace_open_test.go index a0af09b97c..78eff654da 100644 --- a/cmd/experimental/workspace_open_test.go +++ b/cmd/experimental/workspace_open_test.go @@ -3,10 +3,14 @@ package experimental import ( "bytes" "context" + "errors" + "log/slog" "testing" "github.com/databricks/cli/libs/cmdctx" "github.com/databricks/cli/libs/cmdio" + "github.com/databricks/cli/libs/log" + "github.com/databricks/cli/libs/log/handler" "github.com/databricks/databricks-sdk-go" "github.com/databricks/databricks-sdk-go/config" "github.com/spf13/cobra" @@ -212,3 +216,44 @@ func TestWorkspaceOpenCommandURLFlag(t *testing.T) { assert.Equal(t, "https://myworkspace.databricks.com/jobs/123?o=789\n", stdout.String()) assert.Equal(t, "", stderr.String()) } + +func TestWorkspaceOpenCommandWarnsWhenWorkspaceIDLookupFails(t *testing.T) { + originalCurrentWorkspaceID := currentWorkspaceID + originalOpenWorkspaceURL := openWorkspaceURL + t.Cleanup(func() { + currentWorkspaceID = originalCurrentWorkspaceID + openWorkspaceURL = originalOpenWorkspaceURL + }) + + currentWorkspaceID = func(context.Context) (int64, error) { + return 0, errors.New("lookup failed") + } + + openWorkspaceURL = func(ctx context.Context, targetURL string) error { + return nil + } + + ctx, stderr := cmdio.NewTestContextWithStderr(t.Context()) + ctx = log.NewContext(ctx, slog.New(handler.NewFriendlyHandler(stderr, &handler.Options{ + Level: log.LevelWarn, + }))) + ctx = cmdctx.SetWorkspaceClient(ctx, &databricks.WorkspaceClient{ + Config: &config.Config{ + Host: "https://myworkspace.databricks.com", + }, + }) + + cmd := newWorkspaceOpenCommand() + cmd.SetContext(ctx) + + var stdout bytes.Buffer + cmd.SetOut(&stdout) + + require.NoError(t, cmd.Flags().Set("url", "true")) + + err := cmd.RunE(cmd, []string{"jobs", "123"}) + require.NoError(t, err) + + assert.Equal(t, "https://myworkspace.databricks.com/jobs/123\n", stdout.String()) + assert.Contains(t, stderr.String(), "Could not determine workspace ID: lookup failed") +} diff --git a/libs/browser/open.go b/libs/browser/open.go index f55b2bb47e..83e7da22d2 100644 --- a/libs/browser/open.go +++ b/libs/browser/open.go @@ -2,12 +2,13 @@ package browser import ( "context" - "fmt" "io" + "os" + osexec "os/exec" + "strings" "github.com/databricks/cli/libs/cmdio" "github.com/databricks/cli/libs/env" - "github.com/databricks/cli/libs/exec" browserpkg "github.com/pkg/browser" ) @@ -26,19 +27,13 @@ var openDefaultBrowserURL = func(targetURL string) error { return browserpkg.OpenURL(targetURL) } -var runBrowserCommand = func(ctx context.Context, workingDirectory, browserCommand, targetURL string) error { - e, err := exec.NewCommandExecutor(workingDirectory) - if err != nil { - return err - } - - e.WithInheritOutput() - cmd, err := e.StartCommand(ctx, fmt.Sprintf("%q %q", browserCommand, targetURL)) - if err != nil { - return err - } - - return cmd.Wait() +var runBrowserCommand = func(ctx context.Context, workingDirectory string, browserCommand []string, targetURL string) error { + cmd := osexec.CommandContext(ctx, browserCommand[0], append(browserCommand[1:], targetURL)...) + cmd.Dir = workingDirectory + cmd.Stdin = os.Stdin + cmd.Stdout = os.Stdout + cmd.Stderr = os.Stderr + return cmd.Run() } // IsDisabled reports whether browser launching is disabled for the context. @@ -48,11 +43,11 @@ func IsDisabled(ctx context.Context) bool { // NewOpener returns a function that opens URLs in the browser. func NewOpener(ctx context.Context, workingDirectory string) func(string) error { - browserCommand := env.Get(ctx, browserEnvVar) - switch browserCommand { - case "": + browserCommand := strings.Fields(env.Get(ctx, browserEnvVar)) + switch { + case len(browserCommand) == 0: return openDefaultBrowserURL - case disabledBrowser: + case len(browserCommand) == 1 && browserCommand[0] == disabledBrowser: return func(targetURL string) error { cmdio.LogString(ctx, "Please complete authentication by opening this link in your browser:\n"+targetURL) return nil diff --git a/libs/browser/open_test.go b/libs/browser/open_test.go index b5e65348eb..e529dbf726 100644 --- a/libs/browser/open_test.go +++ b/libs/browser/open_test.go @@ -43,13 +43,13 @@ func TestOpenURLUsesCustomBrowserCommand(t *testing.T) { runBrowserCommand = original }) - ctx := env.Set(t.Context(), browserEnvVar, "custom-browser") + ctx := env.Set(t.Context(), browserEnvVar, "custom-browser --private-window") var gotCtx context.Context var gotDirectory string - var gotCommand string + var gotCommand []string var gotURL string - runBrowserCommand = func(ctx context.Context, workingDirectory, browserCommand, targetURL string) error { + runBrowserCommand = func(ctx context.Context, workingDirectory string, browserCommand []string, targetURL string) error { gotCtx = ctx gotDirectory = workingDirectory gotCommand = browserCommand @@ -61,7 +61,7 @@ func TestOpenURLUsesCustomBrowserCommand(t *testing.T) { require.NoError(t, err) assert.Same(t, ctx, gotCtx) assert.Equal(t, "test-dir", gotDirectory) - assert.Equal(t, "custom-browser", gotCommand) + assert.Equal(t, []string{"custom-browser", "--private-window"}, gotCommand) assert.Equal(t, "https://example.com", gotURL) } From e14c23c26c77de3a2280a924665b5b06ecd32da9 Mon Sep 17 00:00:00 2001 From: simon Date: Fri, 13 Mar 2026 15:28:06 +0100 Subject: [PATCH 09/10] Address review: remove redundant check, add all resource types, add acceptance test Remove the redundant slices.Contains check in buildWorkspaceURL since LookupPattern already handles unknown resource types. Add missing resource types (alerts, models, model_serving_endpoints, registered_models) so the command supports all types from libs/workspaceurls. Add acceptance test for the experimental open command covering URL generation, error handling, and tab completion. --- acceptance/experimental/open/out.test.toml | 9 ++++++ acceptance/experimental/open/output.txt | 32 ++++++++++++++++++++++ acceptance/experimental/open/script | 11 ++++++++ acceptance/experimental/open/test.toml | 5 ++++ cmd/experimental/workspace_open.go | 9 +++--- cmd/experimental/workspace_open_test.go | 20 ++++++++------ 6 files changed, 73 insertions(+), 13 deletions(-) create mode 100644 acceptance/experimental/open/out.test.toml create mode 100644 acceptance/experimental/open/output.txt create mode 100644 acceptance/experimental/open/script create mode 100644 acceptance/experimental/open/test.toml diff --git a/acceptance/experimental/open/out.test.toml b/acceptance/experimental/open/out.test.toml new file mode 100644 index 0000000000..216969a761 --- /dev/null +++ b/acceptance/experimental/open/out.test.toml @@ -0,0 +1,9 @@ +Local = true +Cloud = false + +[GOOS] + linux = false + windows = false + +[EnvMatrix] + DATABRICKS_BUNDLE_ENGINE = ["terraform", "direct"] diff --git a/acceptance/experimental/open/output.txt b/acceptance/experimental/open/output.txt new file mode 100644 index 0000000000..a83e0676fe --- /dev/null +++ b/acceptance/experimental/open/output.txt @@ -0,0 +1,32 @@ + +=== print URL for a job +>>> [CLI] experimental open --url jobs 123 +[DATABRICKS_URL]/jobs/123?o=[NUMID] + +=== print URL for a notebook +>>> [CLI] experimental open --url notebooks 12345 +[DATABRICKS_URL]/?o=[NUMID]#notebook/12345 + +=== unknown resource type +>>> [CLI] experimental open --url unknown 123 +Error: unknown resource type "unknown", must be one of: alerts, apps, clusters, dashboards, experiments, jobs, model_serving_endpoints, models, notebooks, pipelines, queries, registered_models, warehouses + +Exit code: 1 + +=== test auto-completion handler +>>> [CLI] __complete experimental open , +alerts +apps +clusters +dashboards +experiments +jobs +model_serving_endpoints +models +notebooks +pipelines +queries +registered_models +warehouses +:4 +Completion ended with directive: ShellCompDirectiveNoFileComp diff --git a/acceptance/experimental/open/script b/acceptance/experimental/open/script new file mode 100644 index 0000000000..820175db8d --- /dev/null +++ b/acceptance/experimental/open/script @@ -0,0 +1,11 @@ +title "print URL for a job" +trace $CLI experimental open --url jobs 123 + +title "print URL for a notebook" +trace $CLI experimental open --url notebooks 12345 + +title "unknown resource type" +errcode trace $CLI experimental open --url unknown 123 + +title "test auto-completion handler" +trace $CLI __complete experimental open , diff --git a/acceptance/experimental/open/test.toml b/acceptance/experimental/open/test.toml new file mode 100644 index 0000000000..9e03c20d49 --- /dev/null +++ b/acceptance/experimental/open/test.toml @@ -0,0 +1,5 @@ +GOOS.windows = false +GOOS.linux = false + +# No bundle engine needed for this command. +[EnvMatrix] diff --git a/cmd/experimental/workspace_open.go b/cmd/experimental/workspace_open.go index 80cce99018..bf8b46a035 100644 --- a/cmd/experimental/workspace_open.go +++ b/cmd/experimental/workspace_open.go @@ -3,7 +3,6 @@ package experimental import ( "context" "fmt" - "slices" "strings" "github.com/spf13/cobra" @@ -17,14 +16,18 @@ import ( ) var supportedOpenResourceTypes = []string{ + workspaceurls.ResourceAlerts, workspaceurls.ResourceApps, workspaceurls.ResourceClusters, workspaceurls.ResourceDashboards, workspaceurls.ResourceExperiments, workspaceurls.ResourceJobs, + workspaceurls.ResourceModels, + workspaceurls.ResourceModelServingEndpoints, workspaceurls.ResourceNotebooks, workspaceurls.ResourcePipelines, workspaceurls.ResourceQueries, + workspaceurls.ResourceRegisteredModels, workspaceurls.ResourceWarehouses, } @@ -46,10 +49,6 @@ func supportedResourceTypes() string { // buildWorkspaceURL constructs a full workspace URL for a given resource type and ID. func buildWorkspaceURL(host, resourceType, id string, workspaceID int64) (string, error) { - if !slices.Contains(supportedOpenResourceTypes, resourceType) { - return "", fmt.Errorf("unknown resource type %q, must be one of: %s", resourceType, supportedResourceTypes()) - } - pattern, ok := workspaceurls.LookupPattern(resourceType) if !ok { return "", fmt.Errorf("unknown resource type %q, must be one of: %s", resourceType, supportedResourceTypes()) diff --git a/cmd/experimental/workspace_open_test.go b/cmd/experimental/workspace_open_test.go index 78eff654da..95f74f92c2 100644 --- a/cmd/experimental/workspace_open_test.go +++ b/cmd/experimental/workspace_open_test.go @@ -65,7 +65,7 @@ func TestBuildWorkspaceURLFragmentBasedResources(t *testing.T) { func TestBuildWorkspaceURLUnknownResourceType(t *testing.T) { _, err := buildWorkspaceURL("https://myworkspace.databricks.com", "unknown", "123", 0) assert.ErrorContains(t, err, "unknown resource type \"unknown\"") - assert.ErrorContains(t, err, "apps, clusters, dashboards, experiments, jobs, notebooks, pipelines, queries, warehouses") + assert.ErrorContains(t, err, "alerts, apps, clusters, dashboards, experiments, jobs, model_serving_endpoints, models, notebooks, pipelines, queries, registered_models, warehouses") } func TestBuildWorkspaceURLHostWithTrailingSlash(t *testing.T) { @@ -104,16 +104,20 @@ func TestWorkspaceOpenCommandCompletion(t *testing.T) { completions, directive := cmd.ValidArgsFunction(cmd, []string{}, "") assert.Equal(t, cobra.ShellCompDirectiveNoFileComp, directive) - assert.Contains(t, completions, "jobs") - assert.Contains(t, completions, "notebooks") + assert.Contains(t, completions, "alerts") + assert.Contains(t, completions, "apps") assert.Contains(t, completions, "clusters") - assert.Contains(t, completions, "pipelines") assert.Contains(t, completions, "dashboards") assert.Contains(t, completions, "experiments") - assert.Contains(t, completions, "warehouses") + assert.Contains(t, completions, "jobs") + assert.Contains(t, completions, "models") + assert.Contains(t, completions, "model_serving_endpoints") + assert.Contains(t, completions, "notebooks") + assert.Contains(t, completions, "pipelines") assert.Contains(t, completions, "queries") - assert.Contains(t, completions, "apps") - assert.Len(t, completions, 9) + assert.Contains(t, completions, "registered_models") + assert.Contains(t, completions, "warehouses") + assert.Len(t, completions, 13) } func TestWorkspaceOpenCommandCompletionSecondArg(t *testing.T) { @@ -127,7 +131,7 @@ func TestWorkspaceOpenCommandCompletionSecondArg(t *testing.T) { func TestWorkspaceOpenCommandHelpText(t *testing.T) { cmd := newWorkspaceOpenCommand() - assert.Contains(t, cmd.Long, "Supported resource types: apps, clusters, dashboards, experiments, jobs, notebooks, pipelines, queries, warehouses.") + assert.Contains(t, cmd.Long, "Supported resource types: alerts, apps, clusters, dashboards, experiments, jobs, model_serving_endpoints, models, notebooks, pipelines, queries, registered_models, warehouses.") assert.Contains(t, cmd.Long, "databricks experimental open jobs 123456789") assert.Contains(t, cmd.Long, "databricks experimental open notebooks /Users/user@example.com/my-notebook") assert.Contains(t, cmd.Long, "databricks experimental open jobs 123456789 --url") From d773fff2b4f055d23699cb51cae8749dbec5e33e Mon Sep 17 00:00:00 2001 From: simon Date: Fri, 13 Mar 2026 16:04:05 +0100 Subject: [PATCH 10/10] Fix registered_models URL, quoted BROWSER env, and BROWSER=none message Fix three issues in workspace open and browser opener: 1. Convert dot-separated registered_models names (catalog.schema.model) to slash-separated URL paths automatically. Add help text and example. 2. Handle quoted BROWSER env values (e.g. open -a "Google Chrome") by delegating to sh -c instead of naive whitespace splitting. 3. Make the BROWSER=none fallback message configurable via WithDisabledMessage option. Auth callers keep the auth-specific message, workspace open uses a generic resource message. --- cmd/auth/login.go | 4 +- cmd/auth/token.go | 4 +- cmd/experimental/workspace_open.go | 10 +++- cmd/experimental/workspace_open_test.go | 5 +- libs/browser/open.go | 70 ++++++++++++++++++++++--- libs/browser/open_test.go | 67 ++++++++++++++++++++++- libs/workspaceurls/urls.go | 17 ++++++ 7 files changed, 166 insertions(+), 11 deletions(-) diff --git a/cmd/auth/login.go b/cmd/auth/login.go index 6de4ddc304..b40981db84 100644 --- a/cmd/auth/login.go +++ b/cmd/auth/login.go @@ -172,7 +172,9 @@ depends on the existing profiles you have set in your configuration file } persistentAuthOpts := []u2m.PersistentAuthOption{ u2m.WithOAuthArgument(oauthArgument), - u2m.WithBrowser(browser.NewOpener(cmd.Context(), ".")), + u2m.WithBrowser(browser.NewOpener(cmd.Context(), ".", + browser.WithDisabledMessage("Please complete authentication by opening this link in your browser:\n"), + )), } if len(scopesList) > 0 { persistentAuthOpts = append(persistentAuthOpts, u2m.WithScopes(scopesList)) diff --git a/cmd/auth/token.go b/cmd/auth/token.go index 15fc30745b..009dad130f 100644 --- a/cmd/auth/token.go +++ b/cmd/auth/token.go @@ -433,7 +433,9 @@ func runInlineLogin(ctx context.Context, profiler profile.Profiler) (string, *pr } persistentAuthOpts := []u2m.PersistentAuthOption{ u2m.WithOAuthArgument(oauthArgument), - u2m.WithBrowser(browser.NewOpener(ctx, ".")), + u2m.WithBrowser(browser.NewOpener(ctx, ".", + browser.WithDisabledMessage("Please complete authentication by opening this link in your browser:\n"), + )), } if len(scopesList) > 0 { persistentAuthOpts = append(persistentAuthOpts, u2m.WithScopes(scopesList)) diff --git a/cmd/experimental/workspace_open.go b/cmd/experimental/workspace_open.go index bf8b46a035..2a61e62da8 100644 --- a/cmd/experimental/workspace_open.go +++ b/cmd/experimental/workspace_open.go @@ -36,7 +36,10 @@ var currentWorkspaceID = func(ctx context.Context) (int64, error) { } var openWorkspaceURL = func(ctx context.Context, targetURL string) error { - return browser.OpenURL(ctx, ".", targetURL) + opener := browser.NewOpener(ctx, ".", + browser.WithDisabledMessage("Open this URL in your browser to view the resource:\n"), + ) + return opener(targetURL) } func resourceTypeNames() []string { @@ -54,6 +57,7 @@ func buildWorkspaceURL(host, resourceType, id string, workspaceID int64) (string return "", fmt.Errorf("unknown resource type %q, must be one of: %s", resourceType, supportedResourceTypes()) } + id = workspaceurls.NormalizeDotSeparatedID(resourceType, id) return workspaceurls.BuildResourceURL(host, pattern, id, workspaceID) } @@ -67,10 +71,14 @@ func newWorkspaceOpenCommand() *cobra.Command { Supported resource types: %s. +For registered_models, use the dot-separated name (catalog.schema.model) +and it will be converted to the correct URL path automatically. + Examples: databricks experimental open jobs 123456789 databricks experimental open notebooks /Users/user@example.com/my-notebook databricks experimental open clusters 0123-456789-abcdef + databricks experimental open registered_models catalog.schema.my_model databricks experimental open jobs 123456789 --url`, supportedResourceTypes()), Args: cobra.ExactArgs(2), PreRunE: root.MustWorkspaceClient, diff --git a/cmd/experimental/workspace_open_test.go b/cmd/experimental/workspace_open_test.go index 95f74f92c2..a5ed0f0ff8 100644 --- a/cmd/experimental/workspace_open_test.go +++ b/cmd/experimental/workspace_open_test.go @@ -32,10 +32,11 @@ func TestBuildWorkspaceURLPathBasedResources(t *testing.T) { {"queries", "q-1", "https://myworkspace.databricks.com/sql/editor/q-1"}, {"apps", "my-app", "https://myworkspace.databricks.com/apps/my-app"}, {"clusters", "0123-456789-abc", "https://myworkspace.databricks.com/compute/clusters/0123-456789-abc"}, + {"registered_models", "catalog.schema.model", "https://myworkspace.databricks.com/explore/data/models/catalog/schema/model"}, } for _, tt := range tests { - t.Run(tt.resourceType, func(t *testing.T) { + t.Run(tt.resourceType+"/"+tt.id, func(t *testing.T) { got, err := buildWorkspaceURL("https://myworkspace.databricks.com", tt.resourceType, tt.id, 0) require.NoError(t, err) assert.Equal(t, tt.expected, got) @@ -134,7 +135,9 @@ func TestWorkspaceOpenCommandHelpText(t *testing.T) { assert.Contains(t, cmd.Long, "Supported resource types: alerts, apps, clusters, dashboards, experiments, jobs, model_serving_endpoints, models, notebooks, pipelines, queries, registered_models, warehouses.") assert.Contains(t, cmd.Long, "databricks experimental open jobs 123456789") assert.Contains(t, cmd.Long, "databricks experimental open notebooks /Users/user@example.com/my-notebook") + assert.Contains(t, cmd.Long, "databricks experimental open registered_models catalog.schema.my_model") assert.Contains(t, cmd.Long, "databricks experimental open jobs 123456789 --url") + assert.Contains(t, cmd.Long, "dot-separated name") flag := cmd.Flags().Lookup("url") require.NotNil(t, flag) diff --git a/libs/browser/open.go b/libs/browser/open.go index 83e7da22d2..409ebc7f19 100644 --- a/libs/browser/open.go +++ b/libs/browser/open.go @@ -5,6 +5,7 @@ import ( "io" "os" osexec "os/exec" + "runtime" "strings" "github.com/databricks/cli/libs/cmdio" @@ -13,8 +14,9 @@ import ( ) const ( - browserEnvVar = "BROWSER" - disabledBrowser = "none" + browserEnvVar = "BROWSER" + disabledBrowser = "none" + defaultDisabledMessage = "Open this link in your browser:\n" ) var openDefaultBrowserURL = func(targetURL string) error { @@ -36,20 +38,76 @@ var runBrowserCommand = func(ctx context.Context, workingDirectory string, brows return cmd.Run() } +// shellName returns the shell executable name for the current OS. +func shellName() string { + if runtime.GOOS == "windows" { + return "cmd" + } + return "sh" +} + +// shellFlag returns the flag to pass an inline command to the shell. +func shellFlag() string { + if runtime.GOOS == "windows" { + return "/c" + } + return "-c" +} + +// containsQuotes reports whether s contains single or double quote characters. +func containsQuotes(s string) bool { + return strings.ContainsAny(s, `"'`) +} + +// parseBrowserCommand splits the BROWSER env var into a command slice. +// If the value contains quotes it delegates to the system shell so that +// values like `open -a "Google Chrome"` are handled correctly. +func parseBrowserCommand(raw string) []string { + if raw == "" { + return nil + } + if containsQuotes(raw) { + return []string{shellName(), shellFlag(), raw} + } + return strings.Fields(raw) +} + // IsDisabled reports whether browser launching is disabled for the context. func IsDisabled(ctx context.Context) bool { return env.Get(ctx, browserEnvVar) == disabledBrowser } +// OpenerOption configures NewOpener. +type OpenerOption func(*openerConfig) + +type openerConfig struct { + disabledMessage string +} + +// WithDisabledMessage overrides the message printed when BROWSER=none. +func WithDisabledMessage(msg string) OpenerOption { + return func(cfg *openerConfig) { + cfg.disabledMessage = msg + } +} + // NewOpener returns a function that opens URLs in the browser. -func NewOpener(ctx context.Context, workingDirectory string) func(string) error { - browserCommand := strings.Fields(env.Get(ctx, browserEnvVar)) +func NewOpener(ctx context.Context, workingDirectory string, opts ...OpenerOption) func(string) error { + cfg := &openerConfig{ + disabledMessage: defaultDisabledMessage, + } + for _, opt := range opts { + opt(cfg) + } + + raw := env.Get(ctx, browserEnvVar) + browserCommand := parseBrowserCommand(raw) switch { case len(browserCommand) == 0: return openDefaultBrowserURL - case len(browserCommand) == 1 && browserCommand[0] == disabledBrowser: + case raw == disabledBrowser: return func(targetURL string) error { - cmdio.LogString(ctx, "Please complete authentication by opening this link in your browser:\n"+targetURL) + cmdio.LogString(ctx, cfg.disabledMessage+targetURL) return nil } default: diff --git a/libs/browser/open_test.go b/libs/browser/open_test.go index e529dbf726..1a0a8898a8 100644 --- a/libs/browser/open_test.go +++ b/libs/browser/open_test.go @@ -2,6 +2,7 @@ package browser import ( "context" + "runtime" "testing" "github.com/databricks/cli/libs/cmdio" @@ -33,10 +34,20 @@ func TestOpenURLWithDisabledBrowser(t *testing.T) { err := OpenURL(ctx, ".", "https://example.com") require.NoError(t, err) - assert.Contains(t, stderr.String(), "Please complete authentication by opening this link in your browser:") + assert.Contains(t, stderr.String(), "Open this link in your browser:") assert.Contains(t, stderr.String(), "https://example.com") } +func TestOpenURLWithDisabledBrowserCustomMessage(t *testing.T) { + ctx, stderr := cmdio.NewTestContextWithStderr(t.Context()) + ctx = env.Set(ctx, browserEnvVar, disabledBrowser) + + opener := NewOpener(ctx, ".", WithDisabledMessage("Custom message:\n")) + err := opener("https://example.com") + require.NoError(t, err) + assert.Contains(t, stderr.String(), "Custom message:\nhttps://example.com") +} + func TestOpenURLUsesCustomBrowserCommand(t *testing.T) { original := runBrowserCommand t.Cleanup(func() { @@ -65,6 +76,60 @@ func TestOpenURLUsesCustomBrowserCommand(t *testing.T) { assert.Equal(t, "https://example.com", gotURL) } +func TestOpenURLUsesShellForQuotedBrowserCommand(t *testing.T) { + original := runBrowserCommand + t.Cleanup(func() { + runBrowserCommand = original + }) + + ctx := env.Set(t.Context(), browserEnvVar, `open -a "Google Chrome"`) + + var gotCommand []string + runBrowserCommand = func(ctx context.Context, workingDirectory string, browserCommand []string, targetURL string) error { + gotCommand = browserCommand + return nil + } + + err := OpenURL(ctx, ".", "https://example.com") + require.NoError(t, err) + + if runtime.GOOS == "windows" { + assert.Equal(t, []string{"cmd", "/c", `open -a "Google Chrome"`}, gotCommand) + } else { + assert.Equal(t, []string{"sh", "-c", `open -a "Google Chrome"`}, gotCommand) + } +} + +func TestParseBrowserCommand(t *testing.T) { + tests := []struct { + name string + input string + expected []string + }{ + {"empty", "", nil}, + {"simple", "firefox", []string{"firefox"}}, + {"with flags", "firefox --private-window", []string{"firefox", "--private-window"}}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := parseBrowserCommand(tt.input) + assert.Equal(t, tt.expected, got) + }) + } +} + +func TestParseBrowserCommandWithQuotes(t *testing.T) { + got := parseBrowserCommand(`open -a "Google Chrome"`) + shell := "sh" + flag := "-c" + if runtime.GOOS == "windows" { + shell = "cmd" + flag = "/c" + } + assert.Equal(t, []string{shell, flag, `open -a "Google Chrome"`}, got) +} + func TestIsDisabled(t *testing.T) { assert.False(t, IsDisabled(t.Context())) diff --git a/libs/workspaceurls/urls.go b/libs/workspaceurls/urls.go index b9a1892263..4e387d5d77 100644 --- a/libs/workspaceurls/urls.go +++ b/libs/workspaceurls/urls.go @@ -56,6 +56,23 @@ var resourceURLPatterns = map[string]string{ ResourceWarehouses: WarehousePattern, } +// dotSeparatedResources lists resource types where the identifier is commonly +// provided as a dot-separated name (e.g. "catalog.schema.model") but the URL +// requires slash-separated segments. +var dotSeparatedResources = map[string]bool{ + ResourceRegisteredModels: true, +} + +// NormalizeDotSeparatedID converts dots to slashes for resource types that use +// multi-part names (e.g. registered_models: "catalog.schema.model" becomes +// "catalog/schema/model"). +func NormalizeDotSeparatedID(resourceType, id string) string { + if dotSeparatedResources[resourceType] { + return strings.ReplaceAll(id, ".", "/") + } + return id +} + // LookupPattern returns the workspace URL pattern for a resource type. func LookupPattern(resourceType string) (string, bool) { pattern, ok := resourceURLPatterns[resourceType]