From 8922dba1ca01ea5daa4e954c3ce7e254e7623004 Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Fri, 29 Nov 2024 10:49:20 +0100 Subject: [PATCH] Move loadGitDetails mutator to Initialize phase This will require API call and we want to keep Load phase fast. --- bundle/config/mutator/mutator.go | 1 - bundle/phases/initialize.go | 1 + bundle/tests/environment_git_test.go | 5 +++-- bundle/tests/git_test.go | 11 ++--------- bundle/tests/loader.go | 19 +++++++++++++------ 5 files changed, 19 insertions(+), 18 deletions(-) diff --git a/bundle/config/mutator/mutator.go b/bundle/config/mutator/mutator.go index faf50ae6..5fd9f53e 100644 --- a/bundle/config/mutator/mutator.go +++ b/bundle/config/mutator/mutator.go @@ -26,7 +26,6 @@ func DefaultMutators() []bundle.Mutator { ComputeIdToClusterId(), InitializeVariables(), DefineDefaultTarget(), - LoadGitDetails(), pythonmutator.PythonMutator(pythonmutator.PythonMutatorPhaseLoad), // Note: This mutator must run before the target overrides are merged. diff --git a/bundle/phases/initialize.go b/bundle/phases/initialize.go index 3d5ad5e8..33458720 100644 --- a/bundle/phases/initialize.go +++ b/bundle/phases/initialize.go @@ -39,6 +39,7 @@ func Initialize() bundle.Mutator { mutator.MergePipelineClusters(), mutator.InitializeWorkspaceClient(), mutator.PopulateCurrentUser(), + mutator.LoadGitDetails(), mutator.DefineDefaultWorkspaceRoot(), mutator.ExpandWorkspaceRoot(), diff --git a/bundle/tests/environment_git_test.go b/bundle/tests/environment_git_test.go index ad4aec2e..8a37c37e 100644 --- a/bundle/tests/environment_git_test.go +++ b/bundle/tests/environment_git_test.go @@ -5,18 +5,19 @@ import ( "strings" "testing" + "github.com/databricks/cli/bundle/config/mutator" "github.com/stretchr/testify/assert" ) func TestGitAutoLoadWithEnvironment(t *testing.T) { - b := load(t, "./environments_autoload_git") + b := load(t, "./environments_autoload_git", mutator.LoadGitDetails()) assert.True(t, b.Config.Bundle.Git.Inferred) validUrl := strings.Contains(b.Config.Bundle.Git.OriginURL, "/cli") || strings.Contains(b.Config.Bundle.Git.OriginURL, "/bricks") assert.True(t, validUrl, fmt.Sprintf("Expected URL to contain '/cli' or '/bricks', got %s", b.Config.Bundle.Git.OriginURL)) } func TestGitManuallySetBranchWithEnvironment(t *testing.T) { - b := loadTarget(t, "./environments_autoload_git", "production") + b := loadTarget(t, "./environments_autoload_git", "production", mutator.LoadGitDetails()) assert.False(t, b.Config.Bundle.Git.Inferred) assert.Equal(t, "main", b.Config.Bundle.Git.Branch) validUrl := strings.Contains(b.Config.Bundle.Git.OriginURL, "/cli") || strings.Contains(b.Config.Bundle.Git.OriginURL, "/bricks") diff --git a/bundle/tests/git_test.go b/bundle/tests/git_test.go index 21eaaedd..81d45de6 100644 --- a/bundle/tests/git_test.go +++ b/bundle/tests/git_test.go @@ -12,15 +12,8 @@ import ( "github.com/stretchr/testify/assert" ) -func TestGitAutoLoad(t *testing.T) { - b := load(t, "./autoload_git") - assert.True(t, b.Config.Bundle.Git.Inferred) - validUrl := strings.Contains(b.Config.Bundle.Git.OriginURL, "/cli") || strings.Contains(b.Config.Bundle.Git.OriginURL, "/bricks") - assert.True(t, validUrl, fmt.Sprintf("Expected URL to contain '/cli' or '/bricks', got %s", b.Config.Bundle.Git.OriginURL)) -} - func TestGitManuallySetBranch(t *testing.T) { - b := loadTarget(t, "./autoload_git", "production") + b := loadTarget(t, "./autoload_git", "production", mutator.LoadGitDetails()) assert.False(t, b.Config.Bundle.Git.Inferred) assert.Equal(t, "main", b.Config.Bundle.Git.Branch) validUrl := strings.Contains(b.Config.Bundle.Git.OriginURL, "/cli") || strings.Contains(b.Config.Bundle.Git.OriginURL, "/bricks") @@ -33,7 +26,7 @@ func TestGitBundleBranchValidation(t *testing.T) { git.GitDirectoryName = ".git" }) - b := load(t, "./git_branch_validation") + b := load(t, "./git_branch_validation", mutator.LoadGitDetails()) assert.False(t, b.Config.Bundle.Git.Inferred) assert.Equal(t, "feature-a", b.Config.Bundle.Git.Branch) assert.Equal(t, "feature-b", b.Config.Bundle.Git.ActualBranch) diff --git a/bundle/tests/loader.go b/bundle/tests/loader.go index 5c48d81c..a1ae8af4 100644 --- a/bundle/tests/loader.go +++ b/bundle/tests/loader.go @@ -15,29 +15,33 @@ import ( "github.com/stretchr/testify/require" ) -func load(t *testing.T, path string) *bundle.Bundle { +func load(t *testing.T, path string, mutators ...bundle.Mutator) *bundle.Bundle { ctx := context.Background() b, err := bundle.Load(ctx, path) require.NoError(t, err) diags := bundle.Apply(ctx, b, phases.Load()) require.NoError(t, diags.Error()) + for _, m := range mutators { + diags := bundle.Apply(ctx, b, m) + require.NoError(t, diags.Error()) + } return b } -func loadTarget(t *testing.T, path, env string) *bundle.Bundle { - b, diags := loadTargetWithDiags(path, env) +func loadTarget(t *testing.T, path, env string, mutators ...bundle.Mutator) *bundle.Bundle { + b, diags := loadTargetWithDiags(path, env, mutators...) require.NoError(t, diags.Error()) return b } -func loadTargetWithDiags(path, env string) (*bundle.Bundle, diag.Diagnostics) { +func loadTargetWithDiags(path, env string, mutators ...bundle.Mutator) (*bundle.Bundle, diag.Diagnostics) { ctx := context.Background() b, err := bundle.Load(ctx, path) if err != nil { return nil, diag.FromErr(err) } - diags := bundle.Apply(ctx, b, bundle.Seq( + seq := []bundle.Mutator{ phases.LoadNamedTarget(env), mutator.RewriteSyncPaths(), mutator.SyncDefaultPath(), @@ -46,7 +50,10 @@ func loadTargetWithDiags(path, env string) (*bundle.Bundle, diag.Diagnostics) { mutator.MergeJobParameters(), mutator.MergeJobTasks(), mutator.MergePipelineClusters(), - )) + } + seq = append(seq, mutators...) + diags := bundle.Apply(ctx, b, bundle.Seq(seq...)) + return b, diags }