From bf68318a0cc1932d37ed451919adbfeed9c93622 Mon Sep 17 00:00:00 2001 From: Shreyas Goenka Date: Mon, 20 Jan 2025 19:05:16 +0100 Subject: [PATCH 01/13] Set bundle auth configuration in command context --- .../mutator/initialize_workspace_client.go | 26 ------------------- cmd/root/bundle.go | 13 ++++++++++ 2 files changed, 13 insertions(+), 26 deletions(-) delete mode 100644 bundle/config/mutator/initialize_workspace_client.go diff --git a/bundle/config/mutator/initialize_workspace_client.go b/bundle/config/mutator/initialize_workspace_client.go deleted file mode 100644 index 5c905f40c2..0000000000 --- a/bundle/config/mutator/initialize_workspace_client.go +++ /dev/null @@ -1,26 +0,0 @@ -package mutator - -import ( - "context" - - "github.com/databricks/cli/bundle" - "github.com/databricks/cli/libs/diag" -) - -type initializeWorkspaceClient struct{} - -func InitializeWorkspaceClient() bundle.Mutator { - return &initializeWorkspaceClient{} -} - -func (m *initializeWorkspaceClient) Name() string { - return "InitializeWorkspaceClient" -} - -// Apply initializes the workspace client for the bundle. We do this here so -// downstream calls to b.WorkspaceClient() do not panic if there's an error in the -// auth configuration. -func (m *initializeWorkspaceClient) Apply(_ context.Context, b *bundle.Bundle) diag.Diagnostics { - _, err := b.InitializeWorkspaceClient() - return diag.FromErr(err) -} diff --git a/cmd/root/bundle.go b/cmd/root/bundle.go index 8b98f2cf20..36af1c4957 100644 --- a/cmd/root/bundle.go +++ b/cmd/root/bundle.go @@ -81,6 +81,19 @@ func configureBundle(cmd *cobra.Command, b *bundle.Bundle) (*bundle.Bundle, diag // Configure the workspace profile if the flag has been set. diags = diags.Extend(configureProfile(cmd, b)) + if diags.HasError() { + return b, diags + } + + // Set the auth configuration in the command context. This can be used + // downstream to initialize a API client. + client, err := b.InitializeWorkspaceClient() + if err != nil { + return b, diags.Extend(diag.FromErr(err)) + } + ctx = context.WithValue(ctx, &configUsed, client.Config) + cmd.SetContext(ctx) + return b, diags } From e868dbcc22b70d41530ca5913296c8bcca6156cc Mon Sep 17 00:00:00 2001 From: Shreyas Goenka Date: Mon, 20 Jan 2025 19:16:04 +0100 Subject: [PATCH 02/13] fix build --- bundle/phases/initialize.go | 1 - 1 file changed, 1 deletion(-) diff --git a/bundle/phases/initialize.go b/bundle/phases/initialize.go index c5b8751961..afd6def3f5 100644 --- a/bundle/phases/initialize.go +++ b/bundle/phases/initialize.go @@ -34,7 +34,6 @@ func Initialize() bundle.Mutator { // If it is an ancestor, this updates all paths to be relative to the sync root path. mutator.SyncInferRoot(), - mutator.InitializeWorkspaceClient(), mutator.PopulateCurrentUser(), mutator.LoadGitDetails(), From 27f1ae38268ef82f1f79c6fbf6e7e3077fae1a98 Mon Sep 17 00:00:00 2001 From: Shreyas Goenka Date: Mon, 20 Jan 2025 20:06:01 +0100 Subject: [PATCH 03/13] fix tests --- cmd/root/bundle_test.go | 312 ++++++++++++++++++++-------------------- 1 file changed, 157 insertions(+), 155 deletions(-) diff --git a/cmd/root/bundle_test.go b/cmd/root/bundle_test.go index 1998b19e61..bccccd567d 100644 --- a/cmd/root/bundle_test.go +++ b/cmd/root/bundle_test.go @@ -8,7 +8,6 @@ import ( "runtime" "testing" - "github.com/databricks/cli/bundle" "github.com/databricks/cli/internal/testutil" "github.com/spf13/cobra" "github.com/stretchr/testify/assert" @@ -38,7 +37,7 @@ func emptyCommand(t *testing.T) *cobra.Command { return cmd } -func setupWithHost(t *testing.T, cmd *cobra.Command, host string) *bundle.Bundle { +func setupWithHost(t *testing.T, cmd *cobra.Command, host string) { setupDatabricksCfg(t) rootPath := t.TempDir() @@ -50,13 +49,9 @@ workspace: `, host) err := os.WriteFile(filepath.Join(rootPath, "databricks.yml"), []byte(contents), 0o644) require.NoError(t, err) - - b, diags := MustConfigureBundle(cmd) - require.NoError(t, diags.Error()) - return b } -func setupWithProfile(t *testing.T, cmd *cobra.Command, profile string) *bundle.Bundle { +func setupWithProfile(t *testing.T, cmd *cobra.Command, profile string) { setupDatabricksCfg(t) rootPath := t.TempDir() @@ -68,157 +63,164 @@ workspace: `, profile) err := os.WriteFile(filepath.Join(rootPath, "databricks.yml"), []byte(contents), 0o644) require.NoError(t, err) - - b, diags := MustConfigureBundle(cmd) - require.NoError(t, diags.Error()) - return b -} - -func TestBundleConfigureDefault(t *testing.T) { - testutil.CleanupEnvironment(t) - - cmd := emptyCommand(t) - b := setupWithHost(t, cmd, "https://x.com") - - client, err := b.InitializeWorkspaceClient() - require.NoError(t, err) - assert.Equal(t, "https://x.com", client.Config.Host) -} - -func TestBundleConfigureWithMultipleMatches(t *testing.T) { - testutil.CleanupEnvironment(t) - - cmd := emptyCommand(t) - b := setupWithHost(t, cmd, "https://a.com") - - _, err := b.InitializeWorkspaceClient() - assert.ErrorContains(t, err, "multiple profiles matched: PROFILE-1, PROFILE-2") } -func TestBundleConfigureWithNonExistentProfileFlag(t *testing.T) { - testutil.CleanupEnvironment(t) - - cmd := emptyCommand(t) - err := cmd.Flag("profile").Value.Set("NOEXIST") - require.NoError(t, err) - b := setupWithHost(t, cmd, "https://x.com") - - _, err = b.InitializeWorkspaceClient() - assert.ErrorContains(t, err, "has no NOEXIST profile configured") -} - -func TestBundleConfigureWithMismatchedProfile(t *testing.T) { - testutil.CleanupEnvironment(t) - - cmd := emptyCommand(t) - err := cmd.Flag("profile").Value.Set("PROFILE-1") - require.NoError(t, err) - b := setupWithHost(t, cmd, "https://x.com") - - _, err = b.InitializeWorkspaceClient() - assert.ErrorContains(t, err, "config host mismatch: profile uses host https://a.com, but CLI configured to use https://x.com") -} - -func TestBundleConfigureWithCorrectProfile(t *testing.T) { - testutil.CleanupEnvironment(t) - - cmd := emptyCommand(t) - err := cmd.Flag("profile").Value.Set("PROFILE-1") - require.NoError(t, err) - b := setupWithHost(t, cmd, "https://a.com") - - client, err := b.InitializeWorkspaceClient() - require.NoError(t, err) - assert.Equal(t, "https://a.com", client.Config.Host) - assert.Equal(t, "PROFILE-1", client.Config.Profile) -} - -func TestBundleConfigureWithMismatchedProfileEnvVariable(t *testing.T) { - testutil.CleanupEnvironment(t) - - t.Setenv("DATABRICKS_CONFIG_PROFILE", "PROFILE-1") - cmd := emptyCommand(t) - b := setupWithHost(t, cmd, "https://x.com") - - _, err := b.InitializeWorkspaceClient() - assert.ErrorContains(t, err, "config host mismatch: profile uses host https://a.com, but CLI configured to use https://x.com") -} - -func TestBundleConfigureWithProfileFlagAndEnvVariable(t *testing.T) { - testutil.CleanupEnvironment(t) - - t.Setenv("DATABRICKS_CONFIG_PROFILE", "NOEXIST") - cmd := emptyCommand(t) - err := cmd.Flag("profile").Value.Set("PROFILE-1") - require.NoError(t, err) - b := setupWithHost(t, cmd, "https://a.com") - - client, err := b.InitializeWorkspaceClient() - require.NoError(t, err) - assert.Equal(t, "https://a.com", client.Config.Host) - assert.Equal(t, "PROFILE-1", client.Config.Profile) -} - -func TestBundleConfigureProfileDefault(t *testing.T) { - testutil.CleanupEnvironment(t) - - // The profile in the databricks.yml file is used - cmd := emptyCommand(t) - b := setupWithProfile(t, cmd, "PROFILE-1") - - client, err := b.InitializeWorkspaceClient() - require.NoError(t, err) - assert.Equal(t, "https://a.com", client.Config.Host) - assert.Equal(t, "a", client.Config.Token) - assert.Equal(t, "PROFILE-1", client.Config.Profile) -} - -func TestBundleConfigureProfileFlag(t *testing.T) { - testutil.CleanupEnvironment(t) - - // The --profile flag takes precedence over the profile in the databricks.yml file - cmd := emptyCommand(t) - err := cmd.Flag("profile").Value.Set("PROFILE-2") - require.NoError(t, err) - b := setupWithProfile(t, cmd, "PROFILE-1") - - client, err := b.InitializeWorkspaceClient() - require.NoError(t, err) - assert.Equal(t, "https://a.com", client.Config.Host) - assert.Equal(t, "b", client.Config.Token) - assert.Equal(t, "PROFILE-2", client.Config.Profile) -} - -func TestBundleConfigureProfileEnvVariable(t *testing.T) { - testutil.CleanupEnvironment(t) - - // The DATABRICKS_CONFIG_PROFILE environment variable takes precedence over the profile in the databricks.yml file - t.Setenv("DATABRICKS_CONFIG_PROFILE", "PROFILE-2") - cmd := emptyCommand(t) - b := setupWithProfile(t, cmd, "PROFILE-1") - - client, err := b.InitializeWorkspaceClient() - require.NoError(t, err) - assert.Equal(t, "https://a.com", client.Config.Host) - assert.Equal(t, "b", client.Config.Token) - assert.Equal(t, "PROFILE-2", client.Config.Profile) -} - -func TestBundleConfigureProfileFlagAndEnvVariable(t *testing.T) { - testutil.CleanupEnvironment(t) - - // The --profile flag takes precedence over the DATABRICKS_CONFIG_PROFILE environment variable - t.Setenv("DATABRICKS_CONFIG_PROFILE", "NOEXIST") - cmd := emptyCommand(t) - err := cmd.Flag("profile").Value.Set("PROFILE-2") - require.NoError(t, err) - b := setupWithProfile(t, cmd, "PROFILE-1") +func TestBundleConfigureProfile(t *testing.T) { + tcases := []struct { + name string + hostInConfig string + + // --profile flag + profileFlag string + // DATABRICKS_CONFIG_PROFILE environment variable + profileEnvVar string + // profile in config + profileInConfig string + + expectedError string + expectedHost string + expectedProfile string + expectedToken string + }{ + { + name: "no match, keep host", + hostInConfig: "https://x.com", + + expectedHost: "https://x.com", + }, + { + name: "multiple profile matches", + hostInConfig: "https://a.com", + + expectedError: "multiple profiles matched: PROFILE-1, PROFILE-2", + }, + { + name: "non-existent profile", + profileFlag: "NOEXIST", + hostInConfig: "https://x.com", + + expectedError: "has no NOEXIST profile configured", + }, + { + name: "mismatched profile", + hostInConfig: "https://x.com", + profileFlag: "PROFILE-1", + + expectedError: "config host mismatch: profile uses host https://a.com, but CLI configured to use https://x.com", + }, + { + name: "profile flag specified", + hostInConfig: "https://a.com", + profileFlag: "PROFILE-1", + + expectedHost: "https://a.com", + expectedProfile: "PROFILE-1", + }, + { + name: "mismatched profile env variable", + hostInConfig: "https://x.com", + profileEnvVar: "PROFILE-1", + + expectedError: "config host mismatch: profile uses host https://a.com, but CLI configured to use https://x.com", + }, + { + // The --profile flag takes precedence over the DATABRICKS_CONFIG_PROFILE environment variable + name: "(host) profile flag takes precedence over env variable", + hostInConfig: "https://a.com", + profileFlag: "PROFILE-1", + profileEnvVar: "NOEXIST", + + expectedHost: "https://a.com", + expectedProfile: "PROFILE-1", + }, + { + name: "profile from config", + profileInConfig: "PROFILE-1", + + expectedHost: "https://a.com", + expectedProfile: "PROFILE-1", + expectedToken: "a", + }, + { + // The --profile flag takes precedence over the profile in the databricks.yml file + name: "profile flag takes precedence", + profileInConfig: "PROFILE-1", + profileFlag: "PROFILE-2", + + expectedHost: "https://a.com", + expectedProfile: "PROFILE-2", + expectedToken: "b", + }, + { + // The DATABRICKS_CONFIG_PROFILE environment variable takes precedence over the profile in the databricks.yml file + name: "profile env variable takes precedence", + profileInConfig: "PROFILE-1", + profileEnvVar: "PROFILE-2", + + expectedHost: "https://a.com", + expectedProfile: "PROFILE-2", + expectedToken: "b", + }, + { + // The --profile flag takes precedence over the DATABRICKS_CONFIG_PROFILE environment variable + name: "profile flag takes precedence over env variable", + profileInConfig: "PROFILE-1", + profileFlag: "PROFILE-2", + profileEnvVar: "NOEXIST", + + expectedHost: "https://a.com", + expectedProfile: "PROFILE-2", + expectedToken: "b", + }, + } - client, err := b.InitializeWorkspaceClient() - require.NoError(t, err) - assert.Equal(t, "https://a.com", client.Config.Host) - assert.Equal(t, "b", client.Config.Token) - assert.Equal(t, "PROFILE-2", client.Config.Profile) + for _, tc := range tcases { + t.Run(tc.name, func(t *testing.T) { + testutil.CleanupEnvironment(t) + + cmd := emptyCommand(t) + + // Set up host in databricks.yml + if tc.hostInConfig != "" { + setupWithHost(t, cmd, tc.hostInConfig) + } + + // Set up profile in databricks.yml + if tc.profileInConfig != "" { + setupWithProfile(t, cmd, tc.profileInConfig) + } + + // Set --profile flag + if tc.profileFlag != "" { + err := cmd.Flag("profile").Value.Set(tc.profileFlag) + require.NoError(t, err) + } + + // Set DATABRICKS_CONFIG_PROFILE environment variable + if tc.profileEnvVar != "" { + t.Setenv("DATABRICKS_CONFIG_PROFILE", tc.profileEnvVar) + } + + _, diags := MustConfigureBundle(cmd) + + if tc.expectedError != "" { + assert.ErrorContains(t, diags.Error(), tc.expectedError) + } else { + assert.NoError(t, diags.Error()) + } + + // Assert on the resolved configuration values + if tc.expectedHost != "" { + assert.Equal(t, tc.expectedHost, ConfigUsed(cmd.Context()).Host) + } + if tc.expectedProfile != "" { + assert.Equal(t, tc.expectedProfile, ConfigUsed(cmd.Context()).Profile) + } + if tc.expectedToken != "" { + assert.Equal(t, tc.expectedToken, ConfigUsed(cmd.Context()).Token) + } + }) + } } func TestTargetFlagFull(t *testing.T) { From 9cff7c66195ac8c7cd04360d457c4617b890d956 Mon Sep 17 00:00:00 2001 From: Shreyas Goenka Date: Mon, 20 Jan 2025 20:10:41 +0100 Subject: [PATCH 04/13] Revert "fix tests" This reverts commit 27f1ae38268ef82f1f79c6fbf6e7e3077fae1a98. --- cmd/root/bundle_test.go | 312 ++++++++++++++++++++-------------------- 1 file changed, 155 insertions(+), 157 deletions(-) diff --git a/cmd/root/bundle_test.go b/cmd/root/bundle_test.go index bccccd567d..1998b19e61 100644 --- a/cmd/root/bundle_test.go +++ b/cmd/root/bundle_test.go @@ -8,6 +8,7 @@ import ( "runtime" "testing" + "github.com/databricks/cli/bundle" "github.com/databricks/cli/internal/testutil" "github.com/spf13/cobra" "github.com/stretchr/testify/assert" @@ -37,7 +38,7 @@ func emptyCommand(t *testing.T) *cobra.Command { return cmd } -func setupWithHost(t *testing.T, cmd *cobra.Command, host string) { +func setupWithHost(t *testing.T, cmd *cobra.Command, host string) *bundle.Bundle { setupDatabricksCfg(t) rootPath := t.TempDir() @@ -49,9 +50,13 @@ workspace: `, host) err := os.WriteFile(filepath.Join(rootPath, "databricks.yml"), []byte(contents), 0o644) require.NoError(t, err) + + b, diags := MustConfigureBundle(cmd) + require.NoError(t, diags.Error()) + return b } -func setupWithProfile(t *testing.T, cmd *cobra.Command, profile string) { +func setupWithProfile(t *testing.T, cmd *cobra.Command, profile string) *bundle.Bundle { setupDatabricksCfg(t) rootPath := t.TempDir() @@ -63,164 +68,157 @@ workspace: `, profile) err := os.WriteFile(filepath.Join(rootPath, "databricks.yml"), []byte(contents), 0o644) require.NoError(t, err) + + b, diags := MustConfigureBundle(cmd) + require.NoError(t, diags.Error()) + return b } -func TestBundleConfigureProfile(t *testing.T) { - tcases := []struct { - name string - hostInConfig string - - // --profile flag - profileFlag string - // DATABRICKS_CONFIG_PROFILE environment variable - profileEnvVar string - // profile in config - profileInConfig string - - expectedError string - expectedHost string - expectedProfile string - expectedToken string - }{ - { - name: "no match, keep host", - hostInConfig: "https://x.com", - - expectedHost: "https://x.com", - }, - { - name: "multiple profile matches", - hostInConfig: "https://a.com", - - expectedError: "multiple profiles matched: PROFILE-1, PROFILE-2", - }, - { - name: "non-existent profile", - profileFlag: "NOEXIST", - hostInConfig: "https://x.com", - - expectedError: "has no NOEXIST profile configured", - }, - { - name: "mismatched profile", - hostInConfig: "https://x.com", - profileFlag: "PROFILE-1", - - expectedError: "config host mismatch: profile uses host https://a.com, but CLI configured to use https://x.com", - }, - { - name: "profile flag specified", - hostInConfig: "https://a.com", - profileFlag: "PROFILE-1", - - expectedHost: "https://a.com", - expectedProfile: "PROFILE-1", - }, - { - name: "mismatched profile env variable", - hostInConfig: "https://x.com", - profileEnvVar: "PROFILE-1", - - expectedError: "config host mismatch: profile uses host https://a.com, but CLI configured to use https://x.com", - }, - { - // The --profile flag takes precedence over the DATABRICKS_CONFIG_PROFILE environment variable - name: "(host) profile flag takes precedence over env variable", - hostInConfig: "https://a.com", - profileFlag: "PROFILE-1", - profileEnvVar: "NOEXIST", - - expectedHost: "https://a.com", - expectedProfile: "PROFILE-1", - }, - { - name: "profile from config", - profileInConfig: "PROFILE-1", - - expectedHost: "https://a.com", - expectedProfile: "PROFILE-1", - expectedToken: "a", - }, - { - // The --profile flag takes precedence over the profile in the databricks.yml file - name: "profile flag takes precedence", - profileInConfig: "PROFILE-1", - profileFlag: "PROFILE-2", - - expectedHost: "https://a.com", - expectedProfile: "PROFILE-2", - expectedToken: "b", - }, - { - // The DATABRICKS_CONFIG_PROFILE environment variable takes precedence over the profile in the databricks.yml file - name: "profile env variable takes precedence", - profileInConfig: "PROFILE-1", - profileEnvVar: "PROFILE-2", - - expectedHost: "https://a.com", - expectedProfile: "PROFILE-2", - expectedToken: "b", - }, - { - // The --profile flag takes precedence over the DATABRICKS_CONFIG_PROFILE environment variable - name: "profile flag takes precedence over env variable", - profileInConfig: "PROFILE-1", - profileFlag: "PROFILE-2", - profileEnvVar: "NOEXIST", - - expectedHost: "https://a.com", - expectedProfile: "PROFILE-2", - expectedToken: "b", - }, - } +func TestBundleConfigureDefault(t *testing.T) { + testutil.CleanupEnvironment(t) - for _, tc := range tcases { - t.Run(tc.name, func(t *testing.T) { - testutil.CleanupEnvironment(t) - - cmd := emptyCommand(t) - - // Set up host in databricks.yml - if tc.hostInConfig != "" { - setupWithHost(t, cmd, tc.hostInConfig) - } - - // Set up profile in databricks.yml - if tc.profileInConfig != "" { - setupWithProfile(t, cmd, tc.profileInConfig) - } - - // Set --profile flag - if tc.profileFlag != "" { - err := cmd.Flag("profile").Value.Set(tc.profileFlag) - require.NoError(t, err) - } - - // Set DATABRICKS_CONFIG_PROFILE environment variable - if tc.profileEnvVar != "" { - t.Setenv("DATABRICKS_CONFIG_PROFILE", tc.profileEnvVar) - } - - _, diags := MustConfigureBundle(cmd) - - if tc.expectedError != "" { - assert.ErrorContains(t, diags.Error(), tc.expectedError) - } else { - assert.NoError(t, diags.Error()) - } - - // Assert on the resolved configuration values - if tc.expectedHost != "" { - assert.Equal(t, tc.expectedHost, ConfigUsed(cmd.Context()).Host) - } - if tc.expectedProfile != "" { - assert.Equal(t, tc.expectedProfile, ConfigUsed(cmd.Context()).Profile) - } - if tc.expectedToken != "" { - assert.Equal(t, tc.expectedToken, ConfigUsed(cmd.Context()).Token) - } - }) - } + cmd := emptyCommand(t) + b := setupWithHost(t, cmd, "https://x.com") + + client, err := b.InitializeWorkspaceClient() + require.NoError(t, err) + assert.Equal(t, "https://x.com", client.Config.Host) +} + +func TestBundleConfigureWithMultipleMatches(t *testing.T) { + testutil.CleanupEnvironment(t) + + cmd := emptyCommand(t) + b := setupWithHost(t, cmd, "https://a.com") + + _, err := b.InitializeWorkspaceClient() + assert.ErrorContains(t, err, "multiple profiles matched: PROFILE-1, PROFILE-2") +} + +func TestBundleConfigureWithNonExistentProfileFlag(t *testing.T) { + testutil.CleanupEnvironment(t) + + cmd := emptyCommand(t) + err := cmd.Flag("profile").Value.Set("NOEXIST") + require.NoError(t, err) + b := setupWithHost(t, cmd, "https://x.com") + + _, err = b.InitializeWorkspaceClient() + assert.ErrorContains(t, err, "has no NOEXIST profile configured") +} + +func TestBundleConfigureWithMismatchedProfile(t *testing.T) { + testutil.CleanupEnvironment(t) + + cmd := emptyCommand(t) + err := cmd.Flag("profile").Value.Set("PROFILE-1") + require.NoError(t, err) + b := setupWithHost(t, cmd, "https://x.com") + + _, err = b.InitializeWorkspaceClient() + assert.ErrorContains(t, err, "config host mismatch: profile uses host https://a.com, but CLI configured to use https://x.com") +} + +func TestBundleConfigureWithCorrectProfile(t *testing.T) { + testutil.CleanupEnvironment(t) + + cmd := emptyCommand(t) + err := cmd.Flag("profile").Value.Set("PROFILE-1") + require.NoError(t, err) + b := setupWithHost(t, cmd, "https://a.com") + + client, err := b.InitializeWorkspaceClient() + require.NoError(t, err) + assert.Equal(t, "https://a.com", client.Config.Host) + assert.Equal(t, "PROFILE-1", client.Config.Profile) +} + +func TestBundleConfigureWithMismatchedProfileEnvVariable(t *testing.T) { + testutil.CleanupEnvironment(t) + + t.Setenv("DATABRICKS_CONFIG_PROFILE", "PROFILE-1") + cmd := emptyCommand(t) + b := setupWithHost(t, cmd, "https://x.com") + + _, err := b.InitializeWorkspaceClient() + assert.ErrorContains(t, err, "config host mismatch: profile uses host https://a.com, but CLI configured to use https://x.com") +} + +func TestBundleConfigureWithProfileFlagAndEnvVariable(t *testing.T) { + testutil.CleanupEnvironment(t) + + t.Setenv("DATABRICKS_CONFIG_PROFILE", "NOEXIST") + cmd := emptyCommand(t) + err := cmd.Flag("profile").Value.Set("PROFILE-1") + require.NoError(t, err) + b := setupWithHost(t, cmd, "https://a.com") + + client, err := b.InitializeWorkspaceClient() + require.NoError(t, err) + assert.Equal(t, "https://a.com", client.Config.Host) + assert.Equal(t, "PROFILE-1", client.Config.Profile) +} + +func TestBundleConfigureProfileDefault(t *testing.T) { + testutil.CleanupEnvironment(t) + + // The profile in the databricks.yml file is used + cmd := emptyCommand(t) + b := setupWithProfile(t, cmd, "PROFILE-1") + + client, err := b.InitializeWorkspaceClient() + require.NoError(t, err) + assert.Equal(t, "https://a.com", client.Config.Host) + assert.Equal(t, "a", client.Config.Token) + assert.Equal(t, "PROFILE-1", client.Config.Profile) +} + +func TestBundleConfigureProfileFlag(t *testing.T) { + testutil.CleanupEnvironment(t) + + // The --profile flag takes precedence over the profile in the databricks.yml file + cmd := emptyCommand(t) + err := cmd.Flag("profile").Value.Set("PROFILE-2") + require.NoError(t, err) + b := setupWithProfile(t, cmd, "PROFILE-1") + + client, err := b.InitializeWorkspaceClient() + require.NoError(t, err) + assert.Equal(t, "https://a.com", client.Config.Host) + assert.Equal(t, "b", client.Config.Token) + assert.Equal(t, "PROFILE-2", client.Config.Profile) +} + +func TestBundleConfigureProfileEnvVariable(t *testing.T) { + testutil.CleanupEnvironment(t) + + // The DATABRICKS_CONFIG_PROFILE environment variable takes precedence over the profile in the databricks.yml file + t.Setenv("DATABRICKS_CONFIG_PROFILE", "PROFILE-2") + cmd := emptyCommand(t) + b := setupWithProfile(t, cmd, "PROFILE-1") + + client, err := b.InitializeWorkspaceClient() + require.NoError(t, err) + assert.Equal(t, "https://a.com", client.Config.Host) + assert.Equal(t, "b", client.Config.Token) + assert.Equal(t, "PROFILE-2", client.Config.Profile) +} + +func TestBundleConfigureProfileFlagAndEnvVariable(t *testing.T) { + testutil.CleanupEnvironment(t) + + // The --profile flag takes precedence over the DATABRICKS_CONFIG_PROFILE environment variable + t.Setenv("DATABRICKS_CONFIG_PROFILE", "NOEXIST") + cmd := emptyCommand(t) + err := cmd.Flag("profile").Value.Set("PROFILE-2") + require.NoError(t, err) + b := setupWithProfile(t, cmd, "PROFILE-1") + + client, err := b.InitializeWorkspaceClient() + require.NoError(t, err) + assert.Equal(t, "https://a.com", client.Config.Host) + assert.Equal(t, "b", client.Config.Token) + assert.Equal(t, "PROFILE-2", client.Config.Profile) } func TestTargetFlagFull(t *testing.T) { From 506a4bd3f8b3fd9466001cedd3eb1cfa0fab1654 Mon Sep 17 00:00:00 2001 From: Shreyas Goenka Date: Mon, 20 Jan 2025 20:18:42 +0100 Subject: [PATCH 05/13] fix tests easy to review version --- cmd/root/bundle_test.go | 85 +++++++++++++++++------------------------ 1 file changed, 35 insertions(+), 50 deletions(-) diff --git a/cmd/root/bundle_test.go b/cmd/root/bundle_test.go index 1998b19e61..3517b02e46 100644 --- a/cmd/root/bundle_test.go +++ b/cmd/root/bundle_test.go @@ -8,7 +8,6 @@ import ( "runtime" "testing" - "github.com/databricks/cli/bundle" "github.com/databricks/cli/internal/testutil" "github.com/spf13/cobra" "github.com/stretchr/testify/assert" @@ -38,7 +37,7 @@ func emptyCommand(t *testing.T) *cobra.Command { return cmd } -func setupWithHost(t *testing.T, cmd *cobra.Command, host string) *bundle.Bundle { +func setupWithHost(t *testing.T, cmd *cobra.Command, host string) error { setupDatabricksCfg(t) rootPath := t.TempDir() @@ -51,12 +50,11 @@ workspace: err := os.WriteFile(filepath.Join(rootPath, "databricks.yml"), []byte(contents), 0o644) require.NoError(t, err) - b, diags := MustConfigureBundle(cmd) - require.NoError(t, diags.Error()) - return b + _, diags := MustConfigureBundle(cmd) + return diags.Error() } -func setupWithProfile(t *testing.T, cmd *cobra.Command, profile string) *bundle.Bundle { +func setupWithProfile(t *testing.T, cmd *cobra.Command, profile string) error { setupDatabricksCfg(t) rootPath := t.TempDir() @@ -69,29 +67,25 @@ workspace: err := os.WriteFile(filepath.Join(rootPath, "databricks.yml"), []byte(contents), 0o644) require.NoError(t, err) - b, diags := MustConfigureBundle(cmd) - require.NoError(t, diags.Error()) - return b + _, diags := MustConfigureBundle(cmd) + return diags.Error() } func TestBundleConfigureDefault(t *testing.T) { testutil.CleanupEnvironment(t) cmd := emptyCommand(t) - b := setupWithHost(t, cmd, "https://x.com") - - client, err := b.InitializeWorkspaceClient() + err := setupWithHost(t, cmd, "https://x.com") require.NoError(t, err) - assert.Equal(t, "https://x.com", client.Config.Host) + + assert.Equal(t, "https://x.com", ConfigUsed(cmd.Context()).Host) } func TestBundleConfigureWithMultipleMatches(t *testing.T) { testutil.CleanupEnvironment(t) cmd := emptyCommand(t) - b := setupWithHost(t, cmd, "https://a.com") - - _, err := b.InitializeWorkspaceClient() + err := setupWithHost(t, cmd, "https://a.com") assert.ErrorContains(t, err, "multiple profiles matched: PROFILE-1, PROFILE-2") } @@ -101,9 +95,8 @@ func TestBundleConfigureWithNonExistentProfileFlag(t *testing.T) { cmd := emptyCommand(t) err := cmd.Flag("profile").Value.Set("NOEXIST") require.NoError(t, err) - b := setupWithHost(t, cmd, "https://x.com") - _, err = b.InitializeWorkspaceClient() + err = setupWithHost(t, cmd, "https://x.com") assert.ErrorContains(t, err, "has no NOEXIST profile configured") } @@ -113,9 +106,8 @@ func TestBundleConfigureWithMismatchedProfile(t *testing.T) { cmd := emptyCommand(t) err := cmd.Flag("profile").Value.Set("PROFILE-1") require.NoError(t, err) - b := setupWithHost(t, cmd, "https://x.com") - _, err = b.InitializeWorkspaceClient() + err = setupWithHost(t, cmd, "https://x.com") assert.ErrorContains(t, err, "config host mismatch: profile uses host https://a.com, but CLI configured to use https://x.com") } @@ -125,12 +117,11 @@ func TestBundleConfigureWithCorrectProfile(t *testing.T) { cmd := emptyCommand(t) err := cmd.Flag("profile").Value.Set("PROFILE-1") require.NoError(t, err) - b := setupWithHost(t, cmd, "https://a.com") + err = setupWithHost(t, cmd, "https://a.com") - client, err := b.InitializeWorkspaceClient() require.NoError(t, err) - assert.Equal(t, "https://a.com", client.Config.Host) - assert.Equal(t, "PROFILE-1", client.Config.Profile) + assert.Equal(t, "https://a.com", ConfigUsed(cmd.Context()).Host) + assert.Equal(t, "PROFILE-1", ConfigUsed(cmd.Context()).Profile) } func TestBundleConfigureWithMismatchedProfileEnvVariable(t *testing.T) { @@ -138,9 +129,8 @@ func TestBundleConfigureWithMismatchedProfileEnvVariable(t *testing.T) { t.Setenv("DATABRICKS_CONFIG_PROFILE", "PROFILE-1") cmd := emptyCommand(t) - b := setupWithHost(t, cmd, "https://x.com") - _, err := b.InitializeWorkspaceClient() + err := setupWithHost(t, cmd, "https://x.com") assert.ErrorContains(t, err, "config host mismatch: profile uses host https://a.com, but CLI configured to use https://x.com") } @@ -151,12 +141,11 @@ func TestBundleConfigureWithProfileFlagAndEnvVariable(t *testing.T) { cmd := emptyCommand(t) err := cmd.Flag("profile").Value.Set("PROFILE-1") require.NoError(t, err) - b := setupWithHost(t, cmd, "https://a.com") - client, err := b.InitializeWorkspaceClient() + err = setupWithHost(t, cmd, "https://a.com") require.NoError(t, err) - assert.Equal(t, "https://a.com", client.Config.Host) - assert.Equal(t, "PROFILE-1", client.Config.Profile) + assert.Equal(t, "https://a.com", ConfigUsed(cmd.Context()).Host) + assert.Equal(t, "PROFILE-1", ConfigUsed(cmd.Context()).Profile) } func TestBundleConfigureProfileDefault(t *testing.T) { @@ -164,13 +153,12 @@ func TestBundleConfigureProfileDefault(t *testing.T) { // The profile in the databricks.yml file is used cmd := emptyCommand(t) - b := setupWithProfile(t, cmd, "PROFILE-1") - client, err := b.InitializeWorkspaceClient() + err := setupWithProfile(t, cmd, "PROFILE-1") require.NoError(t, err) - assert.Equal(t, "https://a.com", client.Config.Host) - assert.Equal(t, "a", client.Config.Token) - assert.Equal(t, "PROFILE-1", client.Config.Profile) + assert.Equal(t, "https://a.com", ConfigUsed(cmd.Context()).Host) + assert.Equal(t, "a", ConfigUsed(cmd.Context()).Token) + assert.Equal(t, "PROFILE-1", ConfigUsed(cmd.Context()).Profile) } func TestBundleConfigureProfileFlag(t *testing.T) { @@ -180,13 +168,12 @@ func TestBundleConfigureProfileFlag(t *testing.T) { cmd := emptyCommand(t) err := cmd.Flag("profile").Value.Set("PROFILE-2") require.NoError(t, err) - b := setupWithProfile(t, cmd, "PROFILE-1") - client, err := b.InitializeWorkspaceClient() + err = setupWithProfile(t, cmd, "PROFILE-1") require.NoError(t, err) - assert.Equal(t, "https://a.com", client.Config.Host) - assert.Equal(t, "b", client.Config.Token) - assert.Equal(t, "PROFILE-2", client.Config.Profile) + assert.Equal(t, "https://a.com", ConfigUsed(cmd.Context()).Host) + assert.Equal(t, "b", ConfigUsed(cmd.Context()).Token) + assert.Equal(t, "PROFILE-2", ConfigUsed(cmd.Context()).Profile) } func TestBundleConfigureProfileEnvVariable(t *testing.T) { @@ -195,13 +182,12 @@ func TestBundleConfigureProfileEnvVariable(t *testing.T) { // The DATABRICKS_CONFIG_PROFILE environment variable takes precedence over the profile in the databricks.yml file t.Setenv("DATABRICKS_CONFIG_PROFILE", "PROFILE-2") cmd := emptyCommand(t) - b := setupWithProfile(t, cmd, "PROFILE-1") - client, err := b.InitializeWorkspaceClient() + err := setupWithProfile(t, cmd, "PROFILE-1") require.NoError(t, err) - assert.Equal(t, "https://a.com", client.Config.Host) - assert.Equal(t, "b", client.Config.Token) - assert.Equal(t, "PROFILE-2", client.Config.Profile) + assert.Equal(t, "https://a.com", ConfigUsed(cmd.Context()).Host) + assert.Equal(t, "b", ConfigUsed(cmd.Context()).Token) + assert.Equal(t, "PROFILE-2", ConfigUsed(cmd.Context()).Profile) } func TestBundleConfigureProfileFlagAndEnvVariable(t *testing.T) { @@ -212,13 +198,12 @@ func TestBundleConfigureProfileFlagAndEnvVariable(t *testing.T) { cmd := emptyCommand(t) err := cmd.Flag("profile").Value.Set("PROFILE-2") require.NoError(t, err) - b := setupWithProfile(t, cmd, "PROFILE-1") - client, err := b.InitializeWorkspaceClient() + err = setupWithProfile(t, cmd, "PROFILE-1") require.NoError(t, err) - assert.Equal(t, "https://a.com", client.Config.Host) - assert.Equal(t, "b", client.Config.Token) - assert.Equal(t, "PROFILE-2", client.Config.Profile) + assert.Equal(t, "https://a.com", ConfigUsed(cmd.Context()).Host) + assert.Equal(t, "b", ConfigUsed(cmd.Context()).Token) + assert.Equal(t, "PROFILE-2", ConfigUsed(cmd.Context()).Profile) } func TestTargetFlagFull(t *testing.T) { From 8cc50c2a19ccfc878ef8da96f668240cb537868a Mon Sep 17 00:00:00 2001 From: Shreyas Goenka Date: Tue, 21 Jan 2025 16:28:25 +0100 Subject: [PATCH 06/13] set client during initialization --- bundle/bundle.go | 12 +++--------- bundle/deploy/terraform/init_test.go | 5 ++++- bundle/deploy/terraform/load_test.go | 5 ++++- cmd/root/bundle.go | 1 + libs/template/renderer_test.go | 5 ++++- 5 files changed, 16 insertions(+), 12 deletions(-) diff --git a/bundle/bundle.go b/bundle/bundle.go index 3bf4ffb629..5d19b52e4b 100644 --- a/bundle/bundle.go +++ b/bundle/bundle.go @@ -143,20 +143,14 @@ func (b *Bundle) InitializeWorkspaceClient() (*databricks.WorkspaceClient, error } func (b *Bundle) WorkspaceClient() *databricks.WorkspaceClient { - b.clientOnce.Do(func() { - var err error - b.client, err = b.InitializeWorkspaceClient() - if err != nil { - panic(err) - } - }) + if b.client == nil { + panic("workspace client not initialized yet. This is a bug in the Databricks CLI.") + } return b.client } // SetWorkpaceClient sets the workspace client for this bundle. -// This is used to inject a mock client for testing. func (b *Bundle) SetWorkpaceClient(w *databricks.WorkspaceClient) { - b.clientOnce.Do(func() {}) b.client = w } diff --git a/bundle/deploy/terraform/init_test.go b/bundle/deploy/terraform/init_test.go index c7a4ffe4ae..ac48fd4841 100644 --- a/bundle/deploy/terraform/init_test.go +++ b/bundle/deploy/terraform/init_test.go @@ -48,7 +48,10 @@ func TestInitEnvironmentVariables(t *testing.T) { // TODO(pietern): create test fixture that initializes a mocked client. t.Setenv("DATABRICKS_HOST", "https://x") t.Setenv("DATABRICKS_TOKEN", "foobar") - b.WorkspaceClient() + + client, err := b.InitializeWorkspaceClient() + require.NoError(t, err) + b.SetWorkpaceClient(client) diags := bundle.Apply(context.Background(), b, Initialize()) require.NoError(t, diags.Error()) diff --git a/bundle/deploy/terraform/load_test.go b/bundle/deploy/terraform/load_test.go index b7243ca191..e353ad48eb 100644 --- a/bundle/deploy/terraform/load_test.go +++ b/bundle/deploy/terraform/load_test.go @@ -30,7 +30,10 @@ func TestLoadWithNoState(t *testing.T) { t.Setenv("DATABRICKS_HOST", "https://x") t.Setenv("DATABRICKS_TOKEN", "foobar") - b.WorkspaceClient() + + client, err := b.InitializeWorkspaceClient() + require.NoError(t, err) + b.SetWorkpaceClient(client) diags := bundle.Apply(context.Background(), b, bundle.Seq( Initialize(), diff --git a/cmd/root/bundle.go b/cmd/root/bundle.go index 36af1c4957..bf20a421f4 100644 --- a/cmd/root/bundle.go +++ b/cmd/root/bundle.go @@ -93,6 +93,7 @@ func configureBundle(cmd *cobra.Command, b *bundle.Bundle) (*bundle.Bundle, diag } ctx = context.WithValue(ctx, &configUsed, client.Config) cmd.SetContext(ctx) + b.SetWorkpaceClient(client) return b, diags } diff --git a/libs/template/renderer_test.go b/libs/template/renderer_test.go index b2ec388bdd..d5fe9fffeb 100644 --- a/libs/template/renderer_test.go +++ b/libs/template/renderer_test.go @@ -91,7 +91,10 @@ func assertBuiltinTemplateValid(t *testing.T, template string, settings map[stri }) b.Tagging = tags.ForCloud(w.Config) - b.WorkspaceClient() + + client, err := b.InitializeWorkspaceClient() + require.NoError(t, err) + b.SetWorkpaceClient(client) diags = bundle.Apply(ctx, b, bundle.Seq( phases.Initialize(), From c0cf3f85357fc5f32c6c0fb2ebc765f0a74d9a03 Mon Sep 17 00:00:00 2001 From: Shreyas Goenka Date: Tue, 21 Jan 2025 16:30:21 +0100 Subject: [PATCH 07/13] remove unused parameter --- bundle/bundle.go | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/bundle/bundle.go b/bundle/bundle.go index 5d19b52e4b..11ad068501 100644 --- a/bundle/bundle.go +++ b/bundle/bundle.go @@ -12,7 +12,6 @@ import ( "fmt" "os" "path/filepath" - "sync" "github.com/databricks/cli/bundle/config" "github.com/databricks/cli/bundle/env" @@ -69,9 +68,7 @@ type Bundle struct { Metadata metadata.Metadata // Store a pointer to the workspace client. - // It can be initialized on demand after loading the configuration. - clientOnce sync.Once - client *databricks.WorkspaceClient + client *databricks.WorkspaceClient // Files that are synced to the workspace.file_path Files []fileset.File From 79a52dc59223c4e035197ce9feba203ceb6347cb Mon Sep 17 00:00:00 2001 From: Shreyas Goenka Date: Tue, 21 Jan 2025 16:43:18 +0100 Subject: [PATCH 08/13] Revert "remove unused parameter" This reverts commit c0cf3f85357fc5f32c6c0fb2ebc765f0a74d9a03. --- bundle/bundle.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/bundle/bundle.go b/bundle/bundle.go index 11ad068501..5d19b52e4b 100644 --- a/bundle/bundle.go +++ b/bundle/bundle.go @@ -12,6 +12,7 @@ import ( "fmt" "os" "path/filepath" + "sync" "github.com/databricks/cli/bundle/config" "github.com/databricks/cli/bundle/env" @@ -68,7 +69,9 @@ type Bundle struct { Metadata metadata.Metadata // Store a pointer to the workspace client. - client *databricks.WorkspaceClient + // It can be initialized on demand after loading the configuration. + clientOnce sync.Once + client *databricks.WorkspaceClient // Files that are synced to the workspace.file_path Files []fileset.File From c07f1370d244bd396cb20905ba269ade4d1e8c38 Mon Sep 17 00:00:00 2001 From: Shreyas Goenka Date: Tue, 21 Jan 2025 16:43:20 +0100 Subject: [PATCH 09/13] Revert "set client during initialization" This reverts commit 8cc50c2a19ccfc878ef8da96f668240cb537868a. --- bundle/bundle.go | 12 +++++++++--- bundle/deploy/terraform/init_test.go | 5 +---- bundle/deploy/terraform/load_test.go | 5 +---- cmd/root/bundle.go | 1 - libs/template/renderer_test.go | 5 +---- 5 files changed, 12 insertions(+), 16 deletions(-) diff --git a/bundle/bundle.go b/bundle/bundle.go index 5d19b52e4b..3bf4ffb629 100644 --- a/bundle/bundle.go +++ b/bundle/bundle.go @@ -143,14 +143,20 @@ func (b *Bundle) InitializeWorkspaceClient() (*databricks.WorkspaceClient, error } func (b *Bundle) WorkspaceClient() *databricks.WorkspaceClient { - if b.client == nil { - panic("workspace client not initialized yet. This is a bug in the Databricks CLI.") - } + b.clientOnce.Do(func() { + var err error + b.client, err = b.InitializeWorkspaceClient() + if err != nil { + panic(err) + } + }) return b.client } // SetWorkpaceClient sets the workspace client for this bundle. +// This is used to inject a mock client for testing. func (b *Bundle) SetWorkpaceClient(w *databricks.WorkspaceClient) { + b.clientOnce.Do(func() {}) b.client = w } diff --git a/bundle/deploy/terraform/init_test.go b/bundle/deploy/terraform/init_test.go index ac48fd4841..c7a4ffe4ae 100644 --- a/bundle/deploy/terraform/init_test.go +++ b/bundle/deploy/terraform/init_test.go @@ -48,10 +48,7 @@ func TestInitEnvironmentVariables(t *testing.T) { // TODO(pietern): create test fixture that initializes a mocked client. t.Setenv("DATABRICKS_HOST", "https://x") t.Setenv("DATABRICKS_TOKEN", "foobar") - - client, err := b.InitializeWorkspaceClient() - require.NoError(t, err) - b.SetWorkpaceClient(client) + b.WorkspaceClient() diags := bundle.Apply(context.Background(), b, Initialize()) require.NoError(t, diags.Error()) diff --git a/bundle/deploy/terraform/load_test.go b/bundle/deploy/terraform/load_test.go index e353ad48eb..b7243ca191 100644 --- a/bundle/deploy/terraform/load_test.go +++ b/bundle/deploy/terraform/load_test.go @@ -30,10 +30,7 @@ func TestLoadWithNoState(t *testing.T) { t.Setenv("DATABRICKS_HOST", "https://x") t.Setenv("DATABRICKS_TOKEN", "foobar") - - client, err := b.InitializeWorkspaceClient() - require.NoError(t, err) - b.SetWorkpaceClient(client) + b.WorkspaceClient() diags := bundle.Apply(context.Background(), b, bundle.Seq( Initialize(), diff --git a/cmd/root/bundle.go b/cmd/root/bundle.go index bf20a421f4..36af1c4957 100644 --- a/cmd/root/bundle.go +++ b/cmd/root/bundle.go @@ -93,7 +93,6 @@ func configureBundle(cmd *cobra.Command, b *bundle.Bundle) (*bundle.Bundle, diag } ctx = context.WithValue(ctx, &configUsed, client.Config) cmd.SetContext(ctx) - b.SetWorkpaceClient(client) return b, diags } diff --git a/libs/template/renderer_test.go b/libs/template/renderer_test.go index d5fe9fffeb..b2ec388bdd 100644 --- a/libs/template/renderer_test.go +++ b/libs/template/renderer_test.go @@ -91,10 +91,7 @@ func assertBuiltinTemplateValid(t *testing.T, template string, settings map[stri }) b.Tagging = tags.ForCloud(w.Config) - - client, err := b.InitializeWorkspaceClient() - require.NoError(t, err) - b.SetWorkpaceClient(client) + b.WorkspaceClient() diags = bundle.Apply(ctx, b, bundle.Seq( phases.Initialize(), From 7855d9159783b70f21a54e0dcce6b6cddf61d192 Mon Sep 17 00:00:00 2001 From: Shreyas Goenka Date: Tue, 21 Jan 2025 16:49:51 +0100 Subject: [PATCH 10/13] modify approach --- bundle/bundle.go | 27 +++++++++++++++------------ cmd/root/auth.go | 2 +- cmd/root/bundle.go | 2 +- 3 files changed, 17 insertions(+), 14 deletions(-) diff --git a/bundle/bundle.go b/bundle/bundle.go index 3bf4ffb629..acbefc19ac 100644 --- a/bundle/bundle.go +++ b/bundle/bundle.go @@ -72,6 +72,7 @@ type Bundle struct { // It can be initialized on demand after loading the configuration. clientOnce sync.Once client *databricks.WorkspaceClient + clientErr error // Files that are synced to the workspace.file_path Files []fileset.File @@ -134,23 +135,25 @@ func TryLoad(ctx context.Context) (*Bundle, error) { return Load(ctx, root) } -func (b *Bundle) InitializeWorkspaceClient() (*databricks.WorkspaceClient, error) { - client, err := b.Config.Workspace.Client() - if err != nil { - return nil, fmt.Errorf("cannot resolve bundle auth configuration: %w", err) - } - return client, nil -} - -func (b *Bundle) WorkspaceClient() *databricks.WorkspaceClient { +func (b *Bundle) WorkspaceClientE() (*databricks.WorkspaceClient, error) { b.clientOnce.Do(func() { var err error - b.client, err = b.InitializeWorkspaceClient() + b.client, err = b.WorkspaceClientE() if err != nil { - panic(err) + b.clientErr = fmt.Errorf("cannot resolve bundle auth configuration: %w", err) } }) - return b.client + + return b.client, b.clientErr +} + +func (b *Bundle) WorkspaceClient() *databricks.WorkspaceClient { + client, err := b.WorkspaceClientE() + if err != nil { + panic(err) + } + + return client } // SetWorkpaceClient sets the workspace client for this bundle. diff --git a/cmd/root/auth.go b/cmd/root/auth.go index 49abfd4140..4fcfbb4d87 100644 --- a/cmd/root/auth.go +++ b/cmd/root/auth.go @@ -209,7 +209,7 @@ func MustWorkspaceClient(cmd *cobra.Command, args []string) error { if b != nil { ctx = context.WithValue(ctx, &configUsed, b.Config.Workspace.Config()) cmd.SetContext(ctx) - client, err := b.InitializeWorkspaceClient() + client, err := b.WorkspaceClientE() if err != nil { return err } diff --git a/cmd/root/bundle.go b/cmd/root/bundle.go index 36af1c4957..6414fbe3fb 100644 --- a/cmd/root/bundle.go +++ b/cmd/root/bundle.go @@ -87,7 +87,7 @@ func configureBundle(cmd *cobra.Command, b *bundle.Bundle) (*bundle.Bundle, diag // Set the auth configuration in the command context. This can be used // downstream to initialize a API client. - client, err := b.InitializeWorkspaceClient() + client, err := b.WorkspaceClientE() if err != nil { return b, diags.Extend(diag.FromErr(err)) } From d14730831de602f2051fff08a081837327934698 Mon Sep 17 00:00:00 2001 From: Shreyas Goenka Date: Wed, 22 Jan 2025 13:47:14 +0100 Subject: [PATCH 11/13] fix infinite recursion --- bundle/bundle.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bundle/bundle.go b/bundle/bundle.go index 050ba83ff0..9cb8916f51 100644 --- a/bundle/bundle.go +++ b/bundle/bundle.go @@ -138,7 +138,7 @@ func TryLoad(ctx context.Context) (*Bundle, error) { func (b *Bundle) WorkspaceClientE() (*databricks.WorkspaceClient, error) { b.clientOnce.Do(func() { var err error - b.client, err = b.WorkspaceClientE() + b.client, err = b.Config.Workspace.Client() if err != nil { b.clientErr = fmt.Errorf("cannot resolve bundle auth configuration: %w", err) } From 334ceda99bf087fadf07216844bd45704c4bcda2 Mon Sep 17 00:00:00 2001 From: Shreyas Goenka Date: Tue, 28 Jan 2025 12:02:17 +0100 Subject: [PATCH 12/13] add comment --- cmd/root/bundle.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/cmd/root/bundle.go b/cmd/root/bundle.go index 6414fbe3fb..5e20666f7d 100644 --- a/cmd/root/bundle.go +++ b/cmd/root/bundle.go @@ -87,6 +87,9 @@ func configureBundle(cmd *cobra.Command, b *bundle.Bundle) (*bundle.Bundle, diag // Set the auth configuration in the command context. This can be used // downstream to initialize a API client. + // + // Note that just initializing a workspace client and loading auth configuration + // is a fast operation. No network i/o is perform just yet for auth types like OAuth. client, err := b.WorkspaceClientE() if err != nil { return b, diags.Extend(diag.FromErr(err)) From 7f33e953b6fdadd0ab93ae3239134a007d9d0cfa Mon Sep 17 00:00:00 2001 From: shreyas-goenka <88374338+shreyas-goenka@users.noreply.github.com> Date: Wed, 29 Jan 2025 16:13:36 +0530 Subject: [PATCH 13/13] Update cmd/root/bundle.go Co-authored-by: Pieter Noordhuis --- cmd/root/bundle.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/root/bundle.go b/cmd/root/bundle.go index 5e20666f7d..5842526f3d 100644 --- a/cmd/root/bundle.go +++ b/cmd/root/bundle.go @@ -89,7 +89,7 @@ func configureBundle(cmd *cobra.Command, b *bundle.Bundle) (*bundle.Bundle, diag // downstream to initialize a API client. // // Note that just initializing a workspace client and loading auth configuration - // is a fast operation. No network i/o is perform just yet for auth types like OAuth. + // is a fast operation. It does not perform network I/O or invoke processes (for example the Azure CLI). client, err := b.WorkspaceClientE() if err != nil { return b, diags.Extend(diag.FromErr(err))