Skip to content

Commit 66289fd

Browse files
committed
Reuse shared ExtractHostQueryParams, remove redundant mutator
Refactor Workspace.NormalizeHostURL() to use auth.ExtractHostQueryParams() instead of duplicating URL query parameter parsing. Remove the NormalizeHostURL mutator since Client() already calls NormalizeHostURL() before building the SDK config, making the Initialize-phase mutator redundant. Addresses review feedback from andrewnester on #4825. Co-authored-by: Isaac
1 parent 3cfbac2 commit 66289fd

File tree

5 files changed

+27
-161
lines changed

5 files changed

+27
-161
lines changed

bundle/config/mutator/normalize_host_url.go

Lines changed: 0 additions & 30 deletions
This file was deleted.

bundle/config/mutator/normalize_host_url_test.go

Lines changed: 0 additions & 94 deletions
This file was deleted.

bundle/config/workspace.go

Lines changed: 5 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
11
package config
22

33
import (
4-
"net/url"
54
"os"
65
"path/filepath"
76

7+
"github.com/databricks/cli/libs/auth"
88
"github.com/databricks/cli/libs/databrickscfg"
99
"github.com/databricks/databricks-sdk-go"
1010
"github.com/databricks/databricks-sdk-go/config"
@@ -146,40 +146,14 @@ func (w *Workspace) Config() *config.Config {
146146
// config. Must be called before Config() so the extracted fields are included
147147
// in the SDK config used for profile resolution and authentication.
148148
func (w *Workspace) NormalizeHostURL() {
149-
if w.Host == "" {
150-
return
151-
}
152-
u, err := url.Parse(w.Host)
153-
if err != nil {
154-
return
155-
}
156-
q := u.Query()
157-
if len(q) == 0 {
158-
return
159-
}
160-
161-
// Extract workspace_id from ?o= or ?workspace_id= if not already set.
149+
params := auth.ExtractHostQueryParams(w.Host)
150+
w.Host = params.Host
162151
if w.WorkspaceID == "" {
163-
if v := q.Get("o"); v != "" {
164-
w.WorkspaceID = v
165-
} else if v := q.Get("workspace_id"); v != "" {
166-
w.WorkspaceID = v
167-
}
152+
w.WorkspaceID = params.WorkspaceID
168153
}
169-
170-
// Extract account_id from ?a= or ?account_id= if not already set.
171154
if w.AccountID == "" {
172-
if v := q.Get("a"); v != "" {
173-
w.AccountID = v
174-
} else if v := q.Get("account_id"); v != "" {
175-
w.AccountID = v
176-
}
155+
w.AccountID = params.AccountID
177156
}
178-
179-
// Strip query parameters from the host.
180-
u.RawQuery = ""
181-
u.Fragment = ""
182-
w.Host = u.String()
183157
}
184158

185159
func (w *Workspace) Client() (*databricks.WorkspaceClient, error) {

bundle/config/workspace_test.go

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -79,20 +79,40 @@ func TestWorkspaceNormalizeHostURL(t *testing.T) {
7979
Host: "https://spog.databricks.com/?o=12345",
8080
}
8181
w.NormalizeHostURL()
82-
assert.Equal(t, "https://spog.databricks.com/", w.Host)
82+
assert.Equal(t, "https://spog.databricks.com", w.Host)
8383
assert.Equal(t, "12345", w.WorkspaceID)
8484
})
8585

86+
t.Run("extracts both workspace_id and account_id", func(t *testing.T) {
87+
w := Workspace{
88+
Host: "https://spog.databricks.com/?o=605&a=abc123",
89+
}
90+
w.NormalizeHostURL()
91+
assert.Equal(t, "https://spog.databricks.com", w.Host)
92+
assert.Equal(t, "605", w.WorkspaceID)
93+
assert.Equal(t, "abc123", w.AccountID)
94+
})
95+
8696
t.Run("explicit workspace_id takes precedence", func(t *testing.T) {
8797
w := Workspace{
8898
Host: "https://spog.databricks.com/?o=999",
8999
WorkspaceID: "explicit",
90100
}
91101
w.NormalizeHostURL()
92-
assert.Equal(t, "https://spog.databricks.com/", w.Host)
102+
assert.Equal(t, "https://spog.databricks.com", w.Host)
93103
assert.Equal(t, "explicit", w.WorkspaceID)
94104
})
95105

106+
t.Run("explicit account_id takes precedence", func(t *testing.T) {
107+
w := Workspace{
108+
Host: "https://spog.databricks.com/?a=from-url",
109+
AccountID: "explicit-account",
110+
}
111+
w.NormalizeHostURL()
112+
assert.Equal(t, "https://spog.databricks.com", w.Host)
113+
assert.Equal(t, "explicit-account", w.AccountID)
114+
})
115+
96116
t.Run("no-op for host without query params", func(t *testing.T) {
97117
w := Workspace{
98118
Host: "https://normal.databricks.com",

bundle/phases/initialize.go

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -34,10 +34,6 @@ func Initialize(ctx context.Context, b *bundle.Bundle) {
3434
validate.ValidateEngine(),
3535
validate.Scripts(),
3636

37-
// Extract query parameters (?o=, ?a=) from workspace host URL and strip them.
38-
// Must run before any mutator that uses the host for authentication or API calls.
39-
mutator.NormalizeHostURL(),
40-
4137
// Updates (dynamic): sync.{paths,include,exclude} (makes them relative to bundle root rather than to definition file)
4238
// Rewrites sync paths to be relative to the bundle root instead of the file they were defined in.
4339
mutator.RewriteSyncPaths(),

0 commit comments

Comments
 (0)