From 2adc677c9eca0da3d0d9ff1ef2a208e2e7babb2c Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Tue, 26 Mar 2024 17:18:52 +0100 Subject: [PATCH 01/15] Move path field to bundle type The bundle path was previously stored on the `config.Root` type under the assumption that the first configuration file being loaded would set it. This is slightly counterintuitive and we know what the path is upon construction of the bundle. The new location for this property reflects this. --- bundle/artifacts/build.go | 2 +- bundle/artifacts/upload_test.go | 4 +-- bundle/artifacts/whl/autodetect.go | 6 ++-- bundle/artifacts/whl/from_libraries.go | 2 +- bundle/bundle.go | 12 ++++--- bundle/bundle_test.go | 4 +-- .../expand_pipeline_glob_paths_test.go | 2 +- bundle/config/mutator/load_git_details.go | 4 +-- bundle/config/mutator/process_include_test.go | 4 +-- .../config/mutator/process_root_includes.go | 8 ++--- .../mutator/process_root_includes_test.go | 34 ++++++++----------- bundle/config/mutator/rewrite_sync_paths.go | 4 +-- .../config/mutator/rewrite_sync_paths_test.go | 10 +++--- bundle/config/mutator/trampoline.go | 2 +- bundle/config/mutator/trampoline_test.go | 2 +- bundle/config/mutator/translate_paths.go | 2 +- bundle/config/mutator/translate_paths_test.go | 24 ++++++------- bundle/config/root.go | 11 +----- bundle/deploy/files/sync.go | 2 +- bundle/deploy/metadata/compute.go | 2 +- bundle/deploy/state_pull.go | 2 +- bundle/deploy/state_pull_test.go | 10 +++--- bundle/deploy/state_push_test.go | 2 +- bundle/deploy/state_update_test.go | 12 +++---- bundle/deploy/terraform/init_test.go | 14 ++++---- bundle/deploy/terraform/load_test.go | 2 +- bundle/deploy/terraform/state_pull_test.go | 2 +- bundle/deploy/terraform/state_push_test.go | 2 +- bundle/libraries/libraries.go | 2 +- bundle/libraries/libraries_test.go | 2 +- bundle/python/conditional_transform_test.go | 6 ++-- bundle/python/transform_test.go | 2 +- bundle/root_test.go | 4 +-- bundle/scripts/scripts.go | 2 +- bundle/scripts/scripts_test.go | 2 +- bundle/tests/python_wheel_test.go | 4 +-- cmd/bundle/generate/generate_test.go | 9 ++--- cmd/sync/sync_test.go | 3 +- internal/bundle/artifacts_test.go | 2 +- 39 files changed, 102 insertions(+), 123 deletions(-) diff --git a/bundle/artifacts/build.go b/bundle/artifacts/build.go index f3ee097c28..5f7e111528 100644 --- a/bundle/artifacts/build.go +++ b/bundle/artifacts/build.go @@ -46,7 +46,7 @@ func (m *build) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { // If artifact path is not provided, use bundle root dir if artifact.Path == "" { - artifact.Path = b.Config.Path + artifact.Path = b.Path } if !filepath.IsAbs(artifact.Path) { diff --git a/bundle/artifacts/upload_test.go b/bundle/artifacts/upload_test.go index ec71100958..ec7ee73a17 100644 --- a/bundle/artifacts/upload_test.go +++ b/bundle/artifacts/upload_test.go @@ -36,8 +36,8 @@ func TestExpandGlobFilesSource(t *testing.T) { t2.Close(t) b := &bundle.Bundle{ + Path: rootPath, Config: config.Root{ - Path: rootPath, Artifacts: map[string]*config.Artifact{ "test": { Type: "custom", @@ -72,8 +72,8 @@ func TestExpandGlobFilesSourceWithNoMatches(t *testing.T) { require.NoError(t, err) b := &bundle.Bundle{ + Path: rootPath, Config: config.Root{ - Path: rootPath, Artifacts: map[string]*config.Artifact{ "test": { Type: "custom", diff --git a/bundle/artifacts/whl/autodetect.go b/bundle/artifacts/whl/autodetect.go index d11db83110..ddb1797f43 100644 --- a/bundle/artifacts/whl/autodetect.go +++ b/bundle/artifacts/whl/autodetect.go @@ -35,21 +35,21 @@ func (m *detectPkg) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostic log.Infof(ctx, "Detecting Python wheel project...") // checking if there is setup.py in the bundle root - setupPy := filepath.Join(b.Config.Path, "setup.py") + setupPy := filepath.Join(b.Path, "setup.py") _, err := os.Stat(setupPy) if err != nil { log.Infof(ctx, "No Python wheel project found at bundle root folder") return nil } - log.Infof(ctx, fmt.Sprintf("Found Python wheel project at %s", b.Config.Path)) + log.Infof(ctx, fmt.Sprintf("Found Python wheel project at %s", b.Path)) module := extractModuleName(setupPy) if b.Config.Artifacts == nil { b.Config.Artifacts = make(map[string]*config.Artifact) } - pkgPath, err := filepath.Abs(b.Config.Path) + pkgPath, err := filepath.Abs(b.Path) if err != nil { return diag.FromErr(err) } diff --git a/bundle/artifacts/whl/from_libraries.go b/bundle/artifacts/whl/from_libraries.go index a2045aaf8c..f3660bbae9 100644 --- a/bundle/artifacts/whl/from_libraries.go +++ b/bundle/artifacts/whl/from_libraries.go @@ -30,7 +30,7 @@ func (*fromLibraries) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnost tasks := libraries.FindAllWheelTasksWithLocalLibraries(b) for _, task := range tasks { for _, lib := range task.Libraries { - matches, err := filepath.Glob(filepath.Join(b.Config.Path, lib.Whl)) + matches, err := filepath.Glob(filepath.Join(b.Path, lib.Whl)) // File referenced from libraries section does not exists, skipping if err != nil { continue diff --git a/bundle/bundle.go b/bundle/bundle.go index a178ea090b..b1c514751e 100644 --- a/bundle/bundle.go +++ b/bundle/bundle.go @@ -30,6 +30,10 @@ import ( const internalFolder = ".internal" type Bundle struct { + // Path contains the directory path to the root of the bundle. + // It is set when we instantiate a new bundle instance. + Path string + Config config.Root // Metadata about the bundle deployment. This is the interface Databricks services @@ -74,8 +78,8 @@ func Load(ctx context.Context, path string) (*Bundle, error) { _, hasIncludesEnv := env.Includes(ctx) if hasRootEnv && hasIncludesEnv && stat.IsDir() { log.Debugf(ctx, "No bundle configuration; using bundle root: %s", path) + b.Path = path b.Config = config.Root{ - Path: path, Bundle: config.Bundle{ Name: filepath.Base(path), }, @@ -158,7 +162,7 @@ func (b *Bundle) CacheDir(ctx context.Context, paths ...string) (string, error) if !exists || cacheDirName == "" { cacheDirName = filepath.Join( // Anchor at bundle root directory. - b.Config.Path, + b.Path, // Static cache directory. ".databricks", "bundle", @@ -210,7 +214,7 @@ func (b *Bundle) GetSyncIncludePatterns(ctx context.Context) ([]string, error) { if err != nil { return nil, err } - internalDirRel, err := filepath.Rel(b.Config.Path, internalDir) + internalDirRel, err := filepath.Rel(b.Path, internalDir) if err != nil { return nil, err } @@ -218,7 +222,7 @@ func (b *Bundle) GetSyncIncludePatterns(ctx context.Context) ([]string, error) { } func (b *Bundle) GitRepository() (*git.Repository, error) { - rootPath, err := folders.FindDirWithLeaf(b.Config.Path, ".git") + rootPath, err := folders.FindDirWithLeaf(b.Path, ".git") if err != nil { return nil, fmt.Errorf("unable to locate repository root: %w", err) } diff --git a/bundle/bundle_test.go b/bundle/bundle_test.go index 887a4ee83f..0ebc4c9be8 100644 --- a/bundle/bundle_test.go +++ b/bundle/bundle_test.go @@ -77,7 +77,7 @@ func TestBundleMustLoadSuccess(t *testing.T) { t.Setenv(env.RootVariable, "./tests/basic") b, err := MustLoad(context.Background()) require.NoError(t, err) - assert.Equal(t, "tests/basic", filepath.ToSlash(b.Config.Path)) + assert.Equal(t, "tests/basic", filepath.ToSlash(b.Path)) } func TestBundleMustLoadFailureWithEnv(t *testing.T) { @@ -96,7 +96,7 @@ func TestBundleTryLoadSuccess(t *testing.T) { t.Setenv(env.RootVariable, "./tests/basic") b, err := TryLoad(context.Background()) require.NoError(t, err) - assert.Equal(t, "tests/basic", filepath.ToSlash(b.Config.Path)) + assert.Equal(t, "tests/basic", filepath.ToSlash(b.Path)) } func TestBundleTryLoadFailureWithEnv(t *testing.T) { diff --git a/bundle/config/mutator/expand_pipeline_glob_paths_test.go b/bundle/config/mutator/expand_pipeline_glob_paths_test.go index db80be028a..effa6f244c 100644 --- a/bundle/config/mutator/expand_pipeline_glob_paths_test.go +++ b/bundle/config/mutator/expand_pipeline_glob_paths_test.go @@ -41,8 +41,8 @@ func TestExpandGlobPathsInPipelines(t *testing.T) { touchEmptyFile(t, filepath.Join(dir, "skip/test7.py")) b := &bundle.Bundle{ + Path: dir, Config: config.Root{ - Path: dir, Resources: config.Resources{ Pipelines: map[string]*resources.Pipeline{ "pipeline": { diff --git a/bundle/config/mutator/load_git_details.go b/bundle/config/mutator/load_git_details.go index 6ff9aad622..4d354b8d70 100644 --- a/bundle/config/mutator/load_git_details.go +++ b/bundle/config/mutator/load_git_details.go @@ -22,7 +22,7 @@ func (m *loadGitDetails) Name() string { func (m *loadGitDetails) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { // Load relevant git repository - repo, err := git.NewRepository(b.Config.Path) + repo, err := git.NewRepository(b.Path) if err != nil { return diag.FromErr(err) } @@ -56,7 +56,7 @@ func (m *loadGitDetails) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagn } // Compute relative path of the bundle root from the Git repo root. - absBundlePath, err := filepath.Abs(b.Config.Path) + absBundlePath, err := filepath.Abs(b.Path) if err != nil { return diag.FromErr(err) } diff --git a/bundle/config/mutator/process_include_test.go b/bundle/config/mutator/process_include_test.go index 0e5351b634..7103d5074b 100644 --- a/bundle/config/mutator/process_include_test.go +++ b/bundle/config/mutator/process_include_test.go @@ -16,8 +16,8 @@ import ( func TestProcessInclude(t *testing.T) { b := &bundle.Bundle{ + Path: t.TempDir(), Config: config.Root{ - Path: t.TempDir(), Workspace: config.Workspace{ Host: "foo", }, @@ -25,7 +25,7 @@ func TestProcessInclude(t *testing.T) { } relPath := "./file.yml" - fullPath := filepath.Join(b.Config.Path, relPath) + fullPath := filepath.Join(b.Path, relPath) f, err := os.Create(fullPath) require.NoError(t, err) fmt.Fprint(f, "workspace:\n host: bar\n") diff --git a/bundle/config/mutator/process_root_includes.go b/bundle/config/mutator/process_root_includes.go index dbf99f2dc6..e4edabc532 100644 --- a/bundle/config/mutator/process_root_includes.go +++ b/bundle/config/mutator/process_root_includes.go @@ -51,7 +51,7 @@ func (m *processRootIncludes) Apply(ctx context.Context, b *bundle.Bundle) diag. // Converts extra include paths from environment variable to relative paths for _, extraIncludePath := range getExtraIncludePaths(ctx) { if filepath.IsAbs(extraIncludePath) { - rel, err := filepath.Rel(b.Config.Path, extraIncludePath) + rel, err := filepath.Rel(b.Path, extraIncludePath) if err != nil { return diag.Errorf("unable to include file '%s': %v", extraIncludePath, err) } @@ -70,7 +70,7 @@ func (m *processRootIncludes) Apply(ctx context.Context, b *bundle.Bundle) diag. } // Anchor includes to the bundle root path. - matches, err := filepath.Glob(filepath.Join(b.Config.Path, entry)) + matches, err := filepath.Glob(filepath.Join(b.Path, entry)) if err != nil { return diag.FromErr(err) } @@ -84,7 +84,7 @@ func (m *processRootIncludes) Apply(ctx context.Context, b *bundle.Bundle) diag. // Filter matches to ones we haven't seen yet. var includes []string for _, match := range matches { - rel, err := filepath.Rel(b.Config.Path, match) + rel, err := filepath.Rel(b.Path, match) if err != nil { return diag.FromErr(err) } @@ -99,7 +99,7 @@ func (m *processRootIncludes) Apply(ctx context.Context, b *bundle.Bundle) diag. slices.Sort(includes) files = append(files, includes...) for _, include := range includes { - out = append(out, ProcessInclude(filepath.Join(b.Config.Path, include), include)) + out = append(out, ProcessInclude(filepath.Join(b.Path, include), include)) } } diff --git a/bundle/config/mutator/process_root_includes_test.go b/bundle/config/mutator/process_root_includes_test.go index 7b21945539..c356628635 100644 --- a/bundle/config/mutator/process_root_includes_test.go +++ b/bundle/config/mutator/process_root_includes_test.go @@ -19,9 +19,7 @@ import ( func TestProcessRootIncludesEmpty(t *testing.T) { b := &bundle.Bundle{ - Config: config.Root{ - Path: ".", - }, + Path: ".", } diags := bundle.Apply(context.Background(), b, mutator.ProcessRootIncludes()) require.NoError(t, diags.Error()) @@ -36,8 +34,8 @@ func TestProcessRootIncludesAbs(t *testing.T) { } b := &bundle.Bundle{ + Path: ".", Config: config.Root{ - Path: ".", Include: []string{ "/tmp/*.yml", }, @@ -50,17 +48,17 @@ func TestProcessRootIncludesAbs(t *testing.T) { func TestProcessRootIncludesSingleGlob(t *testing.T) { b := &bundle.Bundle{ + Path: t.TempDir(), Config: config.Root{ - Path: t.TempDir(), Include: []string{ "*.yml", }, }, } - testutil.Touch(t, b.Config.Path, "databricks.yml") - testutil.Touch(t, b.Config.Path, "a.yml") - testutil.Touch(t, b.Config.Path, "b.yml") + testutil.Touch(t, b.Path, "databricks.yml") + testutil.Touch(t, b.Path, "a.yml") + testutil.Touch(t, b.Path, "b.yml") diags := bundle.Apply(context.Background(), b, mutator.ProcessRootIncludes()) require.NoError(t, diags.Error()) @@ -69,8 +67,8 @@ func TestProcessRootIncludesSingleGlob(t *testing.T) { func TestProcessRootIncludesMultiGlob(t *testing.T) { b := &bundle.Bundle{ + Path: t.TempDir(), Config: config.Root{ - Path: t.TempDir(), Include: []string{ "a*.yml", "b*.yml", @@ -78,8 +76,8 @@ func TestProcessRootIncludesMultiGlob(t *testing.T) { }, } - testutil.Touch(t, b.Config.Path, "a1.yml") - testutil.Touch(t, b.Config.Path, "b1.yml") + testutil.Touch(t, b.Path, "a1.yml") + testutil.Touch(t, b.Path, "b1.yml") diags := bundle.Apply(context.Background(), b, mutator.ProcessRootIncludes()) require.NoError(t, diags.Error()) @@ -88,8 +86,8 @@ func TestProcessRootIncludesMultiGlob(t *testing.T) { func TestProcessRootIncludesRemoveDups(t *testing.T) { b := &bundle.Bundle{ + Path: t.TempDir(), Config: config.Root{ - Path: t.TempDir(), Include: []string{ "*.yml", "*.yml", @@ -97,7 +95,7 @@ func TestProcessRootIncludesRemoveDups(t *testing.T) { }, } - testutil.Touch(t, b.Config.Path, "a.yml") + testutil.Touch(t, b.Path, "a.yml") diags := bundle.Apply(context.Background(), b, mutator.ProcessRootIncludes()) require.NoError(t, diags.Error()) @@ -106,8 +104,8 @@ func TestProcessRootIncludesRemoveDups(t *testing.T) { func TestProcessRootIncludesNotExists(t *testing.T) { b := &bundle.Bundle{ + Path: t.TempDir(), Config: config.Root{ - Path: t.TempDir(), Include: []string{ "notexist.yml", }, @@ -125,9 +123,7 @@ func TestProcessRootIncludesExtrasFromEnvVar(t *testing.T) { t.Setenv(env.IncludesVariable, path.Join(rootPath, testYamlName)) b := &bundle.Bundle{ - Config: config.Root{ - Path: rootPath, - }, + Path: rootPath, } diags := bundle.Apply(context.Background(), b, mutator.ProcessRootIncludes()) @@ -148,9 +144,7 @@ func TestProcessRootIncludesDedupExtrasFromEnvVar(t *testing.T) { )) b := &bundle.Bundle{ - Config: config.Root{ - Path: rootPath, - }, + Path: rootPath, } diags := bundle.Apply(context.Background(), b, mutator.ProcessRootIncludes()) diff --git a/bundle/config/mutator/rewrite_sync_paths.go b/bundle/config/mutator/rewrite_sync_paths.go index 0785c64300..ea56b7afe9 100644 --- a/bundle/config/mutator/rewrite_sync_paths.go +++ b/bundle/config/mutator/rewrite_sync_paths.go @@ -45,11 +45,11 @@ func (m *rewriteSyncPaths) makeRelativeTo(root string) dyn.MapFunc { func (m *rewriteSyncPaths) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { err := b.Config.Mutate(func(v dyn.Value) (dyn.Value, error) { return dyn.Map(v, "sync", func(_ dyn.Path, v dyn.Value) (nv dyn.Value, err error) { - v, err = dyn.Map(v, "include", dyn.Foreach(m.makeRelativeTo(b.Config.Path))) + v, err = dyn.Map(v, "include", dyn.Foreach(m.makeRelativeTo(b.Path))) if err != nil { return dyn.NilValue, err } - v, err = dyn.Map(v, "exclude", dyn.Foreach(m.makeRelativeTo(b.Config.Path))) + v, err = dyn.Map(v, "exclude", dyn.Foreach(m.makeRelativeTo(b.Path))) if err != nil { return dyn.NilValue, err } diff --git a/bundle/config/mutator/rewrite_sync_paths_test.go b/bundle/config/mutator/rewrite_sync_paths_test.go index 667f811ac9..55b9a826bf 100644 --- a/bundle/config/mutator/rewrite_sync_paths_test.go +++ b/bundle/config/mutator/rewrite_sync_paths_test.go @@ -14,8 +14,8 @@ import ( func TestRewriteSyncPathsRelative(t *testing.T) { b := &bundle.Bundle{ + Path: ".", Config: config.Root{ - Path: ".", Sync: config.Sync{ Include: []string{ "foo", @@ -45,8 +45,8 @@ func TestRewriteSyncPathsRelative(t *testing.T) { func TestRewriteSyncPathsAbsolute(t *testing.T) { b := &bundle.Bundle{ + Path: "/tmp/dir", Config: config.Root{ - Path: "/tmp/dir", Sync: config.Sync{ Include: []string{ "foo", @@ -77,9 +77,7 @@ func TestRewriteSyncPathsAbsolute(t *testing.T) { func TestRewriteSyncPathsErrorPaths(t *testing.T) { t.Run("no sync block", func(t *testing.T) { b := &bundle.Bundle{ - Config: config.Root{ - Path: ".", - }, + Path: ".", } diags := bundle.Apply(context.Background(), b, mutator.RewriteSyncPaths()) @@ -88,8 +86,8 @@ func TestRewriteSyncPathsErrorPaths(t *testing.T) { t.Run("empty include/exclude blocks", func(t *testing.T) { b := &bundle.Bundle{ + Path: ".", Config: config.Root{ - Path: ".", Sync: config.Sync{ Include: []string{}, Exclude: []string{}, diff --git a/bundle/config/mutator/trampoline.go b/bundle/config/mutator/trampoline.go index 72c053b594..f7e20ff98b 100644 --- a/bundle/config/mutator/trampoline.go +++ b/bundle/config/mutator/trampoline.go @@ -82,7 +82,7 @@ func (m *trampoline) generateNotebookWrapper(ctx context.Context, b *bundle.Bund return err } - internalDirRel, err := filepath.Rel(b.Config.Path, internalDir) + internalDirRel, err := filepath.Rel(b.Path, internalDir) if err != nil { return err } diff --git a/bundle/config/mutator/trampoline_test.go b/bundle/config/mutator/trampoline_test.go index 8a375aa9ba..088885a2be 100644 --- a/bundle/config/mutator/trampoline_test.go +++ b/bundle/config/mutator/trampoline_test.go @@ -57,8 +57,8 @@ func TestGenerateTrampoline(t *testing.T) { } b := &bundle.Bundle{ + Path: tmpDir, Config: config.Root{ - Path: tmpDir, Bundle: config.Bundle{ Target: "development", }, diff --git a/bundle/config/mutator/translate_paths.go b/bundle/config/mutator/translate_paths.go index af6896ee0d..768253e255 100644 --- a/bundle/config/mutator/translate_paths.go +++ b/bundle/config/mutator/translate_paths.go @@ -85,7 +85,7 @@ func (m *translatePaths) rewritePath( } // Remote path must be relative to the bundle root. - localRelPath, err := filepath.Rel(b.Config.Path, localPath) + localRelPath, err := filepath.Rel(b.Path, localPath) if err != nil { return err } diff --git a/bundle/config/mutator/translate_paths_test.go b/bundle/config/mutator/translate_paths_test.go index bd2ec809ba..701a0b9c8b 100644 --- a/bundle/config/mutator/translate_paths_test.go +++ b/bundle/config/mutator/translate_paths_test.go @@ -36,8 +36,8 @@ func touchEmptyFile(t *testing.T, path string) { func TestTranslatePathsSkippedWithGitSource(t *testing.T) { dir := t.TempDir() b := &bundle.Bundle{ + Path: dir, Config: config.Root{ - Path: dir, Workspace: config.Workspace{ FilePath: "/bundle", }, @@ -106,8 +106,8 @@ func TestTranslatePaths(t *testing.T) { touchEmptyFile(t, filepath.Join(dir, "dist", "task.jar")) b := &bundle.Bundle{ + Path: dir, Config: config.Root{ - Path: dir, Workspace: config.Workspace{ FilePath: "/bundle", }, @@ -273,8 +273,8 @@ func TestTranslatePathsInSubdirectories(t *testing.T) { touchEmptyFile(t, filepath.Join(dir, "job", "my_dbt_project", "dbt_project.yml")) b := &bundle.Bundle{ + Path: dir, Config: config.Root{ - Path: dir, Workspace: config.Workspace{ FilePath: "/bundle", }, @@ -367,8 +367,8 @@ func TestTranslatePathsOutsideBundleRoot(t *testing.T) { dir := t.TempDir() b := &bundle.Bundle{ + Path: dir, Config: config.Root{ - Path: dir, Workspace: config.Workspace{ FilePath: "/bundle", }, @@ -400,8 +400,8 @@ func TestJobNotebookDoesNotExistError(t *testing.T) { dir := t.TempDir() b := &bundle.Bundle{ + Path: dir, Config: config.Root{ - Path: dir, Resources: config.Resources{ Jobs: map[string]*resources.Job{ "job": { @@ -430,8 +430,8 @@ func TestJobFileDoesNotExistError(t *testing.T) { dir := t.TempDir() b := &bundle.Bundle{ + Path: dir, Config: config.Root{ - Path: dir, Resources: config.Resources{ Jobs: map[string]*resources.Job{ "job": { @@ -460,8 +460,8 @@ func TestPipelineNotebookDoesNotExistError(t *testing.T) { dir := t.TempDir() b := &bundle.Bundle{ + Path: dir, Config: config.Root{ - Path: dir, Resources: config.Resources{ Pipelines: map[string]*resources.Pipeline{ "pipeline": { @@ -490,8 +490,8 @@ func TestPipelineFileDoesNotExistError(t *testing.T) { dir := t.TempDir() b := &bundle.Bundle{ + Path: dir, Config: config.Root{ - Path: dir, Resources: config.Resources{ Pipelines: map[string]*resources.Pipeline{ "pipeline": { @@ -521,8 +521,8 @@ func TestJobSparkPythonTaskWithNotebookSourceError(t *testing.T) { touchNotebookFile(t, filepath.Join(dir, "my_notebook.py")) b := &bundle.Bundle{ + Path: dir, Config: config.Root{ - Path: dir, Workspace: config.Workspace{ FilePath: "/bundle", }, @@ -555,8 +555,8 @@ func TestJobNotebookTaskWithFileSourceError(t *testing.T) { touchEmptyFile(t, filepath.Join(dir, "my_file.py")) b := &bundle.Bundle{ + Path: dir, Config: config.Root{ - Path: dir, Workspace: config.Workspace{ FilePath: "/bundle", }, @@ -589,8 +589,8 @@ func TestPipelineNotebookLibraryWithFileSourceError(t *testing.T) { touchEmptyFile(t, filepath.Join(dir, "my_file.py")) b := &bundle.Bundle{ + Path: dir, Config: config.Root{ - Path: dir, Workspace: config.Workspace{ FilePath: "/bundle", }, @@ -623,8 +623,8 @@ func TestPipelineFileLibraryWithNotebookSourceError(t *testing.T) { touchNotebookFile(t, filepath.Join(dir, "my_notebook.py")) b := &bundle.Bundle{ + Path: dir, Config: config.Root{ - Path: dir, Workspace: config.Workspace{ FilePath: "/bundle", }, diff --git a/bundle/config/root.go b/bundle/config/root.go index 8e1ff65077..a3dd0d28bb 100644 --- a/bundle/config/root.go +++ b/bundle/config/root.go @@ -5,7 +5,6 @@ import ( "context" "fmt" "os" - "path/filepath" "strings" "github.com/databricks/cli/bundle/config/resources" @@ -24,10 +23,6 @@ type Root struct { diags diag.Diagnostics depth int - // Path contains the directory path to the root of the bundle. - // It is set when loading `databricks.yml`. - Path string `json:"-" bundle:"readonly"` - // Contains user defined variables Variables map[string]*variable.Variable `json:"variables,omitempty"` @@ -80,9 +75,7 @@ func Load(path string) (*Root, error) { return nil, err } - r := Root{ - Path: filepath.Dir(path), - } + r := Root{} // Load configuration tree from YAML. v, err := yamlloader.LoadYAML(path, bytes.NewBuffer(raw)) @@ -135,12 +128,10 @@ func (r *Root) updateWithDynamicValue(nv dyn.Value) error { // the configuration equals nil (happens in tests). diags := r.diags depth := r.depth - path := r.Path defer func() { r.diags = diags r.depth = depth - r.Path = path }() // Convert normalized configuration tree to typed configuration. diff --git a/bundle/deploy/files/sync.go b/bundle/deploy/files/sync.go index 8de80c22fa..c74e41090d 100644 --- a/bundle/deploy/files/sync.go +++ b/bundle/deploy/files/sync.go @@ -28,7 +28,7 @@ func GetSyncOptions(ctx context.Context, b *bundle.Bundle) (*sync.SyncOptions, e } opts := &sync.SyncOptions{ - LocalPath: b.Config.Path, + LocalPath: b.Path, RemotePath: b.Config.Workspace.FilePath, Include: includes, Exclude: b.Config.Sync.Exclude, diff --git a/bundle/deploy/metadata/compute.go b/bundle/deploy/metadata/compute.go index 5a46cd67f8..d10ed1e4c7 100644 --- a/bundle/deploy/metadata/compute.go +++ b/bundle/deploy/metadata/compute.go @@ -39,7 +39,7 @@ func (m *compute) Apply(_ context.Context, b *bundle.Bundle) diag.Diagnostics { for name, job := range b.Config.Resources.Jobs { // Compute config file path the job is defined in, relative to the bundle // root - relativePath, err := filepath.Rel(b.Config.Path, job.ConfigFilePath) + relativePath, err := filepath.Rel(b.Path, job.ConfigFilePath) if err != nil { return diag.Errorf("failed to compute relative path for job %s: %v", name, err) } diff --git a/bundle/deploy/state_pull.go b/bundle/deploy/state_pull.go index 61f5426a09..6f8591d6bd 100644 --- a/bundle/deploy/state_pull.go +++ b/bundle/deploy/state_pull.go @@ -85,7 +85,7 @@ func (s *statePull) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostic } log.Infof(ctx, "Creating new snapshot") - snapshot, err := sync.NewSnapshot(state.Files.ToSlice(b.Config.Path), opts) + snapshot, err := sync.NewSnapshot(state.Files.ToSlice(b.Path), opts) if err != nil { return diag.FromErr(err) } diff --git a/bundle/deploy/state_pull_test.go b/bundle/deploy/state_pull_test.go index 9716a1e04b..a0ac8eeb42 100644 --- a/bundle/deploy/state_pull_test.go +++ b/bundle/deploy/state_pull_test.go @@ -59,8 +59,8 @@ func testStatePull(t *testing.T, opts statePullOpts) { }} b := &bundle.Bundle{ + Path: t.TempDir(), Config: config.Root{ - Path: t.TempDir(), Bundle: config.Bundle{ Target: "default", }, @@ -77,11 +77,11 @@ func testStatePull(t *testing.T, opts statePullOpts) { ctx := context.Background() for _, file := range opts.localFiles { - testutil.Touch(t, filepath.Join(b.Config.Path, "bar"), file) + testutil.Touch(t, filepath.Join(b.Path, "bar"), file) } for _, file := range opts.localNotebooks { - testutil.TouchNotebook(t, filepath.Join(b.Config.Path, "bar"), file) + testutil.TouchNotebook(t, filepath.Join(b.Path, "bar"), file) } if opts.withExistingSnapshot { @@ -251,8 +251,8 @@ func TestStatePullNoState(t *testing.T) { }} b := &bundle.Bundle{ + Path: t.TempDir(), Config: config.Root{ - Path: t.TempDir(), Bundle: config.Bundle{ Target: "default", }, @@ -439,8 +439,8 @@ func TestStatePullNewerDeploymentStateVersion(t *testing.T) { }} b := &bundle.Bundle{ + Path: t.TempDir(), Config: config.Root{ - Path: t.TempDir(), Bundle: config.Bundle{ Target: "default", }, diff --git a/bundle/deploy/state_push_test.go b/bundle/deploy/state_push_test.go index c6d9f88f5a..39d6366ddd 100644 --- a/bundle/deploy/state_push_test.go +++ b/bundle/deploy/state_push_test.go @@ -45,8 +45,8 @@ func TestStatePush(t *testing.T) { }} b := &bundle.Bundle{ + Path: t.TempDir(), Config: config.Root{ - Path: t.TempDir(), Bundle: config.Bundle{ Target: "default", }, diff --git a/bundle/deploy/state_update_test.go b/bundle/deploy/state_update_test.go index 73b7fe4b34..1127331735 100644 --- a/bundle/deploy/state_update_test.go +++ b/bundle/deploy/state_update_test.go @@ -22,8 +22,8 @@ func TestStateUpdate(t *testing.T) { s := &stateUpdate{} b := &bundle.Bundle{ + Path: t.TempDir(), Config: config.Root{ - Path: t.TempDir(), Bundle: config.Bundle{ Target: "default", }, @@ -39,8 +39,8 @@ func TestStateUpdate(t *testing.T) { }, } - testutil.Touch(t, b.Config.Path, "test1.py") - testutil.Touch(t, b.Config.Path, "test2.py") + testutil.Touch(t, b.Path, "test1.py") + testutil.Touch(t, b.Path, "test2.py") m := mocks.NewMockWorkspaceClient(t) m.WorkspaceClient.Config = &databrickscfg.Config{ @@ -82,8 +82,8 @@ func TestStateUpdateWithExistingState(t *testing.T) { s := &stateUpdate{} b := &bundle.Bundle{ + Path: t.TempDir(), Config: config.Root{ - Path: t.TempDir(), Bundle: config.Bundle{ Target: "default", }, @@ -99,8 +99,8 @@ func TestStateUpdateWithExistingState(t *testing.T) { }, } - testutil.Touch(t, b.Config.Path, "test1.py") - testutil.Touch(t, b.Config.Path, "test2.py") + testutil.Touch(t, b.Path, "test1.py") + testutil.Touch(t, b.Path, "test2.py") m := mocks.NewMockWorkspaceClient(t) m.WorkspaceClient.Config = &databrickscfg.Config{ diff --git a/bundle/deploy/terraform/init_test.go b/bundle/deploy/terraform/init_test.go index bbef7f0f79..e1a04c31d8 100644 --- a/bundle/deploy/terraform/init_test.go +++ b/bundle/deploy/terraform/init_test.go @@ -28,8 +28,8 @@ func TestInitEnvironmentVariables(t *testing.T) { } b := &bundle.Bundle{ + Path: t.TempDir(), Config: config.Root{ - Path: t.TempDir(), Bundle: config.Bundle{ Target: "whatever", Terraform: &config.Terraform{ @@ -55,8 +55,8 @@ func TestSetTempDirEnvVarsForUnixWithTmpDirSet(t *testing.T) { } b := &bundle.Bundle{ + Path: t.TempDir(), Config: config.Root{ - Path: t.TempDir(), Bundle: config.Bundle{ Target: "whatever", }, @@ -83,8 +83,8 @@ func TestSetTempDirEnvVarsForUnixWithTmpDirNotSet(t *testing.T) { } b := &bundle.Bundle{ + Path: t.TempDir(), Config: config.Root{ - Path: t.TempDir(), Bundle: config.Bundle{ Target: "whatever", }, @@ -109,8 +109,8 @@ func TestSetTempDirEnvVarsForWindowWithAllTmpDirEnvVarsSet(t *testing.T) { } b := &bundle.Bundle{ + Path: t.TempDir(), Config: config.Root{ - Path: t.TempDir(), Bundle: config.Bundle{ Target: "whatever", }, @@ -139,8 +139,8 @@ func TestSetTempDirEnvVarsForWindowWithUserProfileAndTempSet(t *testing.T) { } b := &bundle.Bundle{ + Path: t.TempDir(), Config: config.Root{ - Path: t.TempDir(), Bundle: config.Bundle{ Target: "whatever", }, @@ -169,8 +169,8 @@ func TestSetTempDirEnvVarsForWindowsWithoutAnyTempDirEnvVarsSet(t *testing.T) { } b := &bundle.Bundle{ + Path: t.TempDir(), Config: config.Root{ - Path: t.TempDir(), Bundle: config.Bundle{ Target: "whatever", }, @@ -197,8 +197,8 @@ func TestSetTempDirEnvVarsForWindowsWithoutAnyTempDirEnvVarsSet(t *testing.T) { func TestSetProxyEnvVars(t *testing.T) { b := &bundle.Bundle{ + Path: t.TempDir(), Config: config.Root{ - Path: t.TempDir(), Bundle: config.Bundle{ Target: "whatever", }, diff --git a/bundle/deploy/terraform/load_test.go b/bundle/deploy/terraform/load_test.go index a912c52133..d18ac05e96 100644 --- a/bundle/deploy/terraform/load_test.go +++ b/bundle/deploy/terraform/load_test.go @@ -17,8 +17,8 @@ func TestLoadWithNoState(t *testing.T) { } b := &bundle.Bundle{ + Path: t.TempDir(), Config: config.Root{ - Path: t.TempDir(), Bundle: config.Bundle{ Target: "whatever", Terraform: &config.Terraform{ diff --git a/bundle/deploy/terraform/state_pull_test.go b/bundle/deploy/terraform/state_pull_test.go index 805b5af0fc..f4e53762f7 100644 --- a/bundle/deploy/terraform/state_pull_test.go +++ b/bundle/deploy/terraform/state_pull_test.go @@ -32,11 +32,11 @@ func mockStateFilerForPull(t *testing.T, contents map[string]int, merr error) fi func statePullTestBundle(t *testing.T) *bundle.Bundle { return &bundle.Bundle{ + Path: t.TempDir(), Config: config.Root{ Bundle: config.Bundle{ Target: "default", }, - Path: t.TempDir(), }, } } diff --git a/bundle/deploy/terraform/state_push_test.go b/bundle/deploy/terraform/state_push_test.go index 41d3849000..b1ddd23840 100644 --- a/bundle/deploy/terraform/state_push_test.go +++ b/bundle/deploy/terraform/state_push_test.go @@ -29,11 +29,11 @@ func mockStateFilerForPush(t *testing.T, fn func(body io.Reader)) filer.Filer { func statePushTestBundle(t *testing.T) *bundle.Bundle { return &bundle.Bundle{ + Path: t.TempDir(), Config: config.Root{ Bundle: config.Bundle{ Target: "default", }, - Path: t.TempDir(), }, } } diff --git a/bundle/libraries/libraries.go b/bundle/libraries/libraries.go index e0cb3fa382..971f6e0f90 100644 --- a/bundle/libraries/libraries.go +++ b/bundle/libraries/libraries.go @@ -65,7 +65,7 @@ func findLibraryMatches(lib *compute.Library, b *bundle.Bundle) ([]string, error return nil, nil } - fullPath := filepath.Join(b.Config.Path, path) + fullPath := filepath.Join(b.Path, path) return filepath.Glob(fullPath) } diff --git a/bundle/libraries/libraries_test.go b/bundle/libraries/libraries_test.go index 0bec2c6d01..873b322a3e 100644 --- a/bundle/libraries/libraries_test.go +++ b/bundle/libraries/libraries_test.go @@ -15,8 +15,8 @@ import ( func TestMapFilesToTaskLibrariesNoGlob(t *testing.T) { b := &bundle.Bundle{ + Path: "testdata", Config: config.Root{ - Path: "testdata", Resources: config.Resources{ Jobs: map[string]*resources.Job{ "job1": { diff --git a/bundle/python/conditional_transform_test.go b/bundle/python/conditional_transform_test.go index b4d7f9edb6..39ddd46334 100644 --- a/bundle/python/conditional_transform_test.go +++ b/bundle/python/conditional_transform_test.go @@ -18,8 +18,8 @@ func TestNoTransformByDefault(t *testing.T) { tmpDir := t.TempDir() b := &bundle.Bundle{ + Path: tmpDir, Config: config.Root{ - Path: tmpDir, Bundle: config.Bundle{ Target: "development", }, @@ -63,8 +63,8 @@ func TestTransformWithExperimentalSettingSetToTrue(t *testing.T) { tmpDir := t.TempDir() b := &bundle.Bundle{ + Path: tmpDir, Config: config.Root{ - Path: tmpDir, Bundle: config.Bundle{ Target: "development", }, @@ -106,7 +106,7 @@ func TestTransformWithExperimentalSettingSetToTrue(t *testing.T) { dir, err := b.InternalDir(context.Background()) require.NoError(t, err) - internalDirRel, err := filepath.Rel(b.Config.Path, dir) + internalDirRel, err := filepath.Rel(b.Path, dir) require.NoError(t, err) require.Equal(t, path.Join(filepath.ToSlash(internalDirRel), "notebook_job1_key1"), task.NotebookTask.NotebookPath) diff --git a/bundle/python/transform_test.go b/bundle/python/transform_test.go index 729efe1a97..d49bea59a4 100644 --- a/bundle/python/transform_test.go +++ b/bundle/python/transform_test.go @@ -116,8 +116,8 @@ func TestTransformFiltersWheelTasksOnly(t *testing.T) { func TestNoPanicWithNoPythonWheelTasks(t *testing.T) { tmpDir := t.TempDir() b := &bundle.Bundle{ + Path: tmpDir, Config: config.Root{ - Path: tmpDir, Bundle: config.Bundle{ Target: "development", }, diff --git a/bundle/root_test.go b/bundle/root_test.go index e6c53e8249..d9365f3599 100644 --- a/bundle/root_test.go +++ b/bundle/root_test.go @@ -106,7 +106,7 @@ func TestLoadYamlWhenIncludesEnvPresent(t *testing.T) { cwd, err := os.Getwd() assert.NoError(t, err) - assert.Equal(t, cwd, bundle.Config.Path) + assert.Equal(t, cwd, bundle.Path) } func TestLoadDefautlBundleWhenNoYamlAndRootAndIncludesEnvPresent(t *testing.T) { @@ -118,7 +118,7 @@ func TestLoadDefautlBundleWhenNoYamlAndRootAndIncludesEnvPresent(t *testing.T) { bundle, err := MustLoad(ctx) assert.NoError(t, err) - assert.Equal(t, dir, bundle.Config.Path) + assert.Equal(t, dir, bundle.Path) } func TestErrorIfNoYamlNoRootEnvAndIncludesEnvPresent(t *testing.T) { diff --git a/bundle/scripts/scripts.go b/bundle/scripts/scripts.go index f8ed7d6a38..e59e6a6415 100644 --- a/bundle/scripts/scripts.go +++ b/bundle/scripts/scripts.go @@ -30,7 +30,7 @@ func (m *script) Name() string { } func (m *script) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { - executor, err := exec.NewCommandExecutor(b.Config.Path) + executor, err := exec.NewCommandExecutor(b.Path) if err != nil { return diag.FromErr(err) } diff --git a/bundle/scripts/scripts_test.go b/bundle/scripts/scripts_test.go index fa5c239701..2034202e0d 100644 --- a/bundle/scripts/scripts_test.go +++ b/bundle/scripts/scripts_test.go @@ -23,7 +23,7 @@ func TestExecutesHook(t *testing.T) { }, } - executor, err := exec.NewCommandExecutor(b.Config.Path) + executor, err := exec.NewCommandExecutor(b.Path) require.NoError(t, err) _, out, err := executeHook(context.Background(), executor, b, config.ScriptPreBuild) require.NoError(t, err) diff --git a/bundle/tests/python_wheel_test.go b/bundle/tests/python_wheel_test.go index c44e80a578..655b3df6b9 100644 --- a/bundle/tests/python_wheel_test.go +++ b/bundle/tests/python_wheel_test.go @@ -79,9 +79,7 @@ func TestPythonWheelBuildNoBuildJustUpload(t *testing.T) { artifact := b.Config.Artifacts["my_test_code-0.0.1-py3-none-any.whl"] require.NotNil(t, artifact) require.Empty(t, artifact.BuildCommand) - require.Contains(t, artifact.Files[0].Source, filepath.Join( - b.Config.Path, - "package", + require.Contains(t, artifact.Files[0].Source, filepath.Join(b.Path, "package", "my_test_code-0.0.1-py3-none-any.whl", )) } diff --git a/cmd/bundle/generate/generate_test.go b/cmd/bundle/generate/generate_test.go index b71f1edfde..691110668a 100644 --- a/cmd/bundle/generate/generate_test.go +++ b/cmd/bundle/generate/generate_test.go @@ -10,7 +10,6 @@ import ( "testing" "github.com/databricks/cli/bundle" - "github.com/databricks/cli/bundle/config" "github.com/databricks/databricks-sdk-go/experimental/mocks" "github.com/databricks/databricks-sdk-go/service/compute" "github.com/databricks/databricks-sdk-go/service/jobs" @@ -25,9 +24,7 @@ func TestGeneratePipelineCommand(t *testing.T) { root := t.TempDir() b := &bundle.Bundle{ - Config: config.Root{ - Path: root, - }, + Path: root, } m := mocks.NewMockWorkspaceClient(t) @@ -125,9 +122,7 @@ func TestGenerateJobCommand(t *testing.T) { root := t.TempDir() b := &bundle.Bundle{ - Config: config.Root{ - Path: root, - }, + Path: root, } m := mocks.NewMockWorkspaceClient(t) diff --git a/cmd/sync/sync_test.go b/cmd/sync/sync_test.go index 827c4d5097..e8754ef560 100644 --- a/cmd/sync/sync_test.go +++ b/cmd/sync/sync_test.go @@ -16,9 +16,8 @@ import ( func TestSyncOptionsFromBundle(t *testing.T) { tempDir := t.TempDir() b := &bundle.Bundle{ + Path: tempDir, Config: config.Root{ - Path: tempDir, - Bundle: config.Bundle{ Target: "default", }, diff --git a/internal/bundle/artifacts_test.go b/internal/bundle/artifacts_test.go index 2ced12fdd1..8f6d117d00 100644 --- a/internal/bundle/artifacts_test.go +++ b/internal/bundle/artifacts_test.go @@ -36,8 +36,8 @@ func TestAccUploadArtifactFileToCorrectRemotePath(t *testing.T) { wsDir := internal.TemporaryWorkspaceDir(t, w) b := &bundle.Bundle{ + Path: dir, Config: config.Root{ - Path: dir, Bundle: config.Bundle{ Target: "whatever", }, From f48cf03579d6406d22f4c069f6ac27ffafefe8cb Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Tue, 26 Mar 2024 17:21:36 +0100 Subject: [PATCH 02/15] Path -> RootPath --- bundle/artifacts/build.go | 2 +- bundle/artifacts/upload_test.go | 4 +-- bundle/artifacts/whl/autodetect.go | 6 ++-- bundle/artifacts/whl/from_libraries.go | 2 +- bundle/bundle.go | 12 ++++---- bundle/bundle_test.go | 4 +-- .../expand_pipeline_glob_paths_test.go | 2 +- bundle/config/mutator/load_git_details.go | 4 +-- bundle/config/mutator/process_include_test.go | 4 +-- .../config/mutator/process_root_includes.go | 8 +++--- .../mutator/process_root_includes_test.go | 28 +++++++++---------- bundle/config/mutator/rewrite_sync_paths.go | 4 +-- .../config/mutator/rewrite_sync_paths_test.go | 8 +++--- bundle/config/mutator/trampoline.go | 2 +- bundle/config/mutator/trampoline_test.go | 2 +- bundle/config/mutator/translate_paths.go | 2 +- bundle/config/mutator/translate_paths_test.go | 24 ++++++++-------- bundle/deploy/files/sync.go | 2 +- bundle/deploy/metadata/compute.go | 2 +- bundle/deploy/state_pull.go | 2 +- bundle/deploy/state_pull_test.go | 10 +++---- bundle/deploy/state_push_test.go | 2 +- bundle/deploy/state_update_test.go | 12 ++++---- bundle/deploy/terraform/init_test.go | 14 +++++----- bundle/deploy/terraform/load_test.go | 2 +- bundle/deploy/terraform/state_pull_test.go | 2 +- bundle/deploy/terraform/state_push_test.go | 2 +- bundle/libraries/libraries.go | 2 +- bundle/libraries/libraries_test.go | 2 +- bundle/python/conditional_transform_test.go | 6 ++-- bundle/python/transform_test.go | 2 +- bundle/root_test.go | 4 +-- bundle/scripts/scripts.go | 2 +- bundle/scripts/scripts_test.go | 2 +- bundle/tests/python_wheel_test.go | 2 +- cmd/bundle/generate/generate_test.go | 4 +-- cmd/sync/sync_test.go | 2 +- internal/bundle/artifacts_test.go | 2 +- 38 files changed, 99 insertions(+), 99 deletions(-) diff --git a/bundle/artifacts/build.go b/bundle/artifacts/build.go index 5f7e111528..349b1ff898 100644 --- a/bundle/artifacts/build.go +++ b/bundle/artifacts/build.go @@ -46,7 +46,7 @@ func (m *build) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { // If artifact path is not provided, use bundle root dir if artifact.Path == "" { - artifact.Path = b.Path + artifact.Path = b.RootPath } if !filepath.IsAbs(artifact.Path) { diff --git a/bundle/artifacts/upload_test.go b/bundle/artifacts/upload_test.go index ec7ee73a17..687d73b4a8 100644 --- a/bundle/artifacts/upload_test.go +++ b/bundle/artifacts/upload_test.go @@ -36,7 +36,7 @@ func TestExpandGlobFilesSource(t *testing.T) { t2.Close(t) b := &bundle.Bundle{ - Path: rootPath, + RootPath: rootPath, Config: config.Root{ Artifacts: map[string]*config.Artifact{ "test": { @@ -72,7 +72,7 @@ func TestExpandGlobFilesSourceWithNoMatches(t *testing.T) { require.NoError(t, err) b := &bundle.Bundle{ - Path: rootPath, + RootPath: rootPath, Config: config.Root{ Artifacts: map[string]*config.Artifact{ "test": { diff --git a/bundle/artifacts/whl/autodetect.go b/bundle/artifacts/whl/autodetect.go index ddb1797f43..ee77fff01b 100644 --- a/bundle/artifacts/whl/autodetect.go +++ b/bundle/artifacts/whl/autodetect.go @@ -35,21 +35,21 @@ func (m *detectPkg) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostic log.Infof(ctx, "Detecting Python wheel project...") // checking if there is setup.py in the bundle root - setupPy := filepath.Join(b.Path, "setup.py") + setupPy := filepath.Join(b.RootPath, "setup.py") _, err := os.Stat(setupPy) if err != nil { log.Infof(ctx, "No Python wheel project found at bundle root folder") return nil } - log.Infof(ctx, fmt.Sprintf("Found Python wheel project at %s", b.Path)) + log.Infof(ctx, fmt.Sprintf("Found Python wheel project at %s", b.RootPath)) module := extractModuleName(setupPy) if b.Config.Artifacts == nil { b.Config.Artifacts = make(map[string]*config.Artifact) } - pkgPath, err := filepath.Abs(b.Path) + pkgPath, err := filepath.Abs(b.RootPath) if err != nil { return diag.FromErr(err) } diff --git a/bundle/artifacts/whl/from_libraries.go b/bundle/artifacts/whl/from_libraries.go index f3660bbae9..84ef712acb 100644 --- a/bundle/artifacts/whl/from_libraries.go +++ b/bundle/artifacts/whl/from_libraries.go @@ -30,7 +30,7 @@ func (*fromLibraries) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnost tasks := libraries.FindAllWheelTasksWithLocalLibraries(b) for _, task := range tasks { for _, lib := range task.Libraries { - matches, err := filepath.Glob(filepath.Join(b.Path, lib.Whl)) + matches, err := filepath.Glob(filepath.Join(b.RootPath, lib.Whl)) // File referenced from libraries section does not exists, skipping if err != nil { continue diff --git a/bundle/bundle.go b/bundle/bundle.go index b1c514751e..d5673b493e 100644 --- a/bundle/bundle.go +++ b/bundle/bundle.go @@ -30,9 +30,9 @@ import ( const internalFolder = ".internal" type Bundle struct { - // Path contains the directory path to the root of the bundle. + // RootPath contains the directory path to the root of the bundle. // It is set when we instantiate a new bundle instance. - Path string + RootPath string Config config.Root @@ -78,7 +78,7 @@ func Load(ctx context.Context, path string) (*Bundle, error) { _, hasIncludesEnv := env.Includes(ctx) if hasRootEnv && hasIncludesEnv && stat.IsDir() { log.Debugf(ctx, "No bundle configuration; using bundle root: %s", path) - b.Path = path + b.RootPath = path b.Config = config.Root{ Bundle: config.Bundle{ Name: filepath.Base(path), @@ -162,7 +162,7 @@ func (b *Bundle) CacheDir(ctx context.Context, paths ...string) (string, error) if !exists || cacheDirName == "" { cacheDirName = filepath.Join( // Anchor at bundle root directory. - b.Path, + b.RootPath, // Static cache directory. ".databricks", "bundle", @@ -214,7 +214,7 @@ func (b *Bundle) GetSyncIncludePatterns(ctx context.Context) ([]string, error) { if err != nil { return nil, err } - internalDirRel, err := filepath.Rel(b.Path, internalDir) + internalDirRel, err := filepath.Rel(b.RootPath, internalDir) if err != nil { return nil, err } @@ -222,7 +222,7 @@ func (b *Bundle) GetSyncIncludePatterns(ctx context.Context) ([]string, error) { } func (b *Bundle) GitRepository() (*git.Repository, error) { - rootPath, err := folders.FindDirWithLeaf(b.Path, ".git") + rootPath, err := folders.FindDirWithLeaf(b.RootPath, ".git") if err != nil { return nil, fmt.Errorf("unable to locate repository root: %w", err) } diff --git a/bundle/bundle_test.go b/bundle/bundle_test.go index 0ebc4c9be8..be716a40ac 100644 --- a/bundle/bundle_test.go +++ b/bundle/bundle_test.go @@ -77,7 +77,7 @@ func TestBundleMustLoadSuccess(t *testing.T) { t.Setenv(env.RootVariable, "./tests/basic") b, err := MustLoad(context.Background()) require.NoError(t, err) - assert.Equal(t, "tests/basic", filepath.ToSlash(b.Path)) + assert.Equal(t, "tests/basic", filepath.ToSlash(b.RootPath)) } func TestBundleMustLoadFailureWithEnv(t *testing.T) { @@ -96,7 +96,7 @@ func TestBundleTryLoadSuccess(t *testing.T) { t.Setenv(env.RootVariable, "./tests/basic") b, err := TryLoad(context.Background()) require.NoError(t, err) - assert.Equal(t, "tests/basic", filepath.ToSlash(b.Path)) + assert.Equal(t, "tests/basic", filepath.ToSlash(b.RootPath)) } func TestBundleTryLoadFailureWithEnv(t *testing.T) { diff --git a/bundle/config/mutator/expand_pipeline_glob_paths_test.go b/bundle/config/mutator/expand_pipeline_glob_paths_test.go index effa6f244c..d1671c256f 100644 --- a/bundle/config/mutator/expand_pipeline_glob_paths_test.go +++ b/bundle/config/mutator/expand_pipeline_glob_paths_test.go @@ -41,7 +41,7 @@ func TestExpandGlobPathsInPipelines(t *testing.T) { touchEmptyFile(t, filepath.Join(dir, "skip/test7.py")) b := &bundle.Bundle{ - Path: dir, + RootPath: dir, Config: config.Root{ Resources: config.Resources{ Pipelines: map[string]*resources.Pipeline{ diff --git a/bundle/config/mutator/load_git_details.go b/bundle/config/mutator/load_git_details.go index 4d354b8d70..7ce8476f1f 100644 --- a/bundle/config/mutator/load_git_details.go +++ b/bundle/config/mutator/load_git_details.go @@ -22,7 +22,7 @@ func (m *loadGitDetails) Name() string { func (m *loadGitDetails) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { // Load relevant git repository - repo, err := git.NewRepository(b.Path) + repo, err := git.NewRepository(b.RootPath) if err != nil { return diag.FromErr(err) } @@ -56,7 +56,7 @@ func (m *loadGitDetails) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagn } // Compute relative path of the bundle root from the Git repo root. - absBundlePath, err := filepath.Abs(b.Path) + absBundlePath, err := filepath.Abs(b.RootPath) if err != nil { return diag.FromErr(err) } diff --git a/bundle/config/mutator/process_include_test.go b/bundle/config/mutator/process_include_test.go index 7103d5074b..b4fa3ccda5 100644 --- a/bundle/config/mutator/process_include_test.go +++ b/bundle/config/mutator/process_include_test.go @@ -16,7 +16,7 @@ import ( func TestProcessInclude(t *testing.T) { b := &bundle.Bundle{ - Path: t.TempDir(), + RootPath: t.TempDir(), Config: config.Root{ Workspace: config.Workspace{ Host: "foo", @@ -25,7 +25,7 @@ func TestProcessInclude(t *testing.T) { } relPath := "./file.yml" - fullPath := filepath.Join(b.Path, relPath) + fullPath := filepath.Join(b.RootPath, relPath) f, err := os.Create(fullPath) require.NoError(t, err) fmt.Fprint(f, "workspace:\n host: bar\n") diff --git a/bundle/config/mutator/process_root_includes.go b/bundle/config/mutator/process_root_includes.go index e4edabc532..4e4aeef43c 100644 --- a/bundle/config/mutator/process_root_includes.go +++ b/bundle/config/mutator/process_root_includes.go @@ -51,7 +51,7 @@ func (m *processRootIncludes) Apply(ctx context.Context, b *bundle.Bundle) diag. // Converts extra include paths from environment variable to relative paths for _, extraIncludePath := range getExtraIncludePaths(ctx) { if filepath.IsAbs(extraIncludePath) { - rel, err := filepath.Rel(b.Path, extraIncludePath) + rel, err := filepath.Rel(b.RootPath, extraIncludePath) if err != nil { return diag.Errorf("unable to include file '%s': %v", extraIncludePath, err) } @@ -70,7 +70,7 @@ func (m *processRootIncludes) Apply(ctx context.Context, b *bundle.Bundle) diag. } // Anchor includes to the bundle root path. - matches, err := filepath.Glob(filepath.Join(b.Path, entry)) + matches, err := filepath.Glob(filepath.Join(b.RootPath, entry)) if err != nil { return diag.FromErr(err) } @@ -84,7 +84,7 @@ func (m *processRootIncludes) Apply(ctx context.Context, b *bundle.Bundle) diag. // Filter matches to ones we haven't seen yet. var includes []string for _, match := range matches { - rel, err := filepath.Rel(b.Path, match) + rel, err := filepath.Rel(b.RootPath, match) if err != nil { return diag.FromErr(err) } @@ -99,7 +99,7 @@ func (m *processRootIncludes) Apply(ctx context.Context, b *bundle.Bundle) diag. slices.Sort(includes) files = append(files, includes...) for _, include := range includes { - out = append(out, ProcessInclude(filepath.Join(b.Path, include), include)) + out = append(out, ProcessInclude(filepath.Join(b.RootPath, include), include)) } } diff --git a/bundle/config/mutator/process_root_includes_test.go b/bundle/config/mutator/process_root_includes_test.go index c356628635..d3aaa974d6 100644 --- a/bundle/config/mutator/process_root_includes_test.go +++ b/bundle/config/mutator/process_root_includes_test.go @@ -19,7 +19,7 @@ import ( func TestProcessRootIncludesEmpty(t *testing.T) { b := &bundle.Bundle{ - Path: ".", + RootPath: ".", } diags := bundle.Apply(context.Background(), b, mutator.ProcessRootIncludes()) require.NoError(t, diags.Error()) @@ -34,7 +34,7 @@ func TestProcessRootIncludesAbs(t *testing.T) { } b := &bundle.Bundle{ - Path: ".", + RootPath: ".", Config: config.Root{ Include: []string{ "/tmp/*.yml", @@ -48,7 +48,7 @@ func TestProcessRootIncludesAbs(t *testing.T) { func TestProcessRootIncludesSingleGlob(t *testing.T) { b := &bundle.Bundle{ - Path: t.TempDir(), + RootPath: t.TempDir(), Config: config.Root{ Include: []string{ "*.yml", @@ -56,9 +56,9 @@ func TestProcessRootIncludesSingleGlob(t *testing.T) { }, } - testutil.Touch(t, b.Path, "databricks.yml") - testutil.Touch(t, b.Path, "a.yml") - testutil.Touch(t, b.Path, "b.yml") + testutil.Touch(t, b.RootPath, "databricks.yml") + testutil.Touch(t, b.RootPath, "a.yml") + testutil.Touch(t, b.RootPath, "b.yml") diags := bundle.Apply(context.Background(), b, mutator.ProcessRootIncludes()) require.NoError(t, diags.Error()) @@ -67,7 +67,7 @@ func TestProcessRootIncludesSingleGlob(t *testing.T) { func TestProcessRootIncludesMultiGlob(t *testing.T) { b := &bundle.Bundle{ - Path: t.TempDir(), + RootPath: t.TempDir(), Config: config.Root{ Include: []string{ "a*.yml", @@ -76,8 +76,8 @@ func TestProcessRootIncludesMultiGlob(t *testing.T) { }, } - testutil.Touch(t, b.Path, "a1.yml") - testutil.Touch(t, b.Path, "b1.yml") + testutil.Touch(t, b.RootPath, "a1.yml") + testutil.Touch(t, b.RootPath, "b1.yml") diags := bundle.Apply(context.Background(), b, mutator.ProcessRootIncludes()) require.NoError(t, diags.Error()) @@ -86,7 +86,7 @@ func TestProcessRootIncludesMultiGlob(t *testing.T) { func TestProcessRootIncludesRemoveDups(t *testing.T) { b := &bundle.Bundle{ - Path: t.TempDir(), + RootPath: t.TempDir(), Config: config.Root{ Include: []string{ "*.yml", @@ -95,7 +95,7 @@ func TestProcessRootIncludesRemoveDups(t *testing.T) { }, } - testutil.Touch(t, b.Path, "a.yml") + testutil.Touch(t, b.RootPath, "a.yml") diags := bundle.Apply(context.Background(), b, mutator.ProcessRootIncludes()) require.NoError(t, diags.Error()) @@ -104,7 +104,7 @@ func TestProcessRootIncludesRemoveDups(t *testing.T) { func TestProcessRootIncludesNotExists(t *testing.T) { b := &bundle.Bundle{ - Path: t.TempDir(), + RootPath: t.TempDir(), Config: config.Root{ Include: []string{ "notexist.yml", @@ -123,7 +123,7 @@ func TestProcessRootIncludesExtrasFromEnvVar(t *testing.T) { t.Setenv(env.IncludesVariable, path.Join(rootPath, testYamlName)) b := &bundle.Bundle{ - Path: rootPath, + RootPath: rootPath, } diags := bundle.Apply(context.Background(), b, mutator.ProcessRootIncludes()) @@ -144,7 +144,7 @@ func TestProcessRootIncludesDedupExtrasFromEnvVar(t *testing.T) { )) b := &bundle.Bundle{ - Path: rootPath, + RootPath: rootPath, } diags := bundle.Apply(context.Background(), b, mutator.ProcessRootIncludes()) diff --git a/bundle/config/mutator/rewrite_sync_paths.go b/bundle/config/mutator/rewrite_sync_paths.go index ea56b7afe9..710190230e 100644 --- a/bundle/config/mutator/rewrite_sync_paths.go +++ b/bundle/config/mutator/rewrite_sync_paths.go @@ -45,11 +45,11 @@ func (m *rewriteSyncPaths) makeRelativeTo(root string) dyn.MapFunc { func (m *rewriteSyncPaths) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { err := b.Config.Mutate(func(v dyn.Value) (dyn.Value, error) { return dyn.Map(v, "sync", func(_ dyn.Path, v dyn.Value) (nv dyn.Value, err error) { - v, err = dyn.Map(v, "include", dyn.Foreach(m.makeRelativeTo(b.Path))) + v, err = dyn.Map(v, "include", dyn.Foreach(m.makeRelativeTo(b.RootPath))) if err != nil { return dyn.NilValue, err } - v, err = dyn.Map(v, "exclude", dyn.Foreach(m.makeRelativeTo(b.Path))) + v, err = dyn.Map(v, "exclude", dyn.Foreach(m.makeRelativeTo(b.RootPath))) if err != nil { return dyn.NilValue, err } diff --git a/bundle/config/mutator/rewrite_sync_paths_test.go b/bundle/config/mutator/rewrite_sync_paths_test.go index 55b9a826bf..56ada19e67 100644 --- a/bundle/config/mutator/rewrite_sync_paths_test.go +++ b/bundle/config/mutator/rewrite_sync_paths_test.go @@ -14,7 +14,7 @@ import ( func TestRewriteSyncPathsRelative(t *testing.T) { b := &bundle.Bundle{ - Path: ".", + RootPath: ".", Config: config.Root{ Sync: config.Sync{ Include: []string{ @@ -45,7 +45,7 @@ func TestRewriteSyncPathsRelative(t *testing.T) { func TestRewriteSyncPathsAbsolute(t *testing.T) { b := &bundle.Bundle{ - Path: "/tmp/dir", + RootPath: "/tmp/dir", Config: config.Root{ Sync: config.Sync{ Include: []string{ @@ -77,7 +77,7 @@ func TestRewriteSyncPathsAbsolute(t *testing.T) { func TestRewriteSyncPathsErrorPaths(t *testing.T) { t.Run("no sync block", func(t *testing.T) { b := &bundle.Bundle{ - Path: ".", + RootPath: ".", } diags := bundle.Apply(context.Background(), b, mutator.RewriteSyncPaths()) @@ -86,7 +86,7 @@ func TestRewriteSyncPathsErrorPaths(t *testing.T) { t.Run("empty include/exclude blocks", func(t *testing.T) { b := &bundle.Bundle{ - Path: ".", + RootPath: ".", Config: config.Root{ Sync: config.Sync{ Include: []string{}, diff --git a/bundle/config/mutator/trampoline.go b/bundle/config/mutator/trampoline.go index f7e20ff98b..dde9a299eb 100644 --- a/bundle/config/mutator/trampoline.go +++ b/bundle/config/mutator/trampoline.go @@ -82,7 +82,7 @@ func (m *trampoline) generateNotebookWrapper(ctx context.Context, b *bundle.Bund return err } - internalDirRel, err := filepath.Rel(b.Path, internalDir) + internalDirRel, err := filepath.Rel(b.RootPath, internalDir) if err != nil { return err } diff --git a/bundle/config/mutator/trampoline_test.go b/bundle/config/mutator/trampoline_test.go index 088885a2be..e39076647f 100644 --- a/bundle/config/mutator/trampoline_test.go +++ b/bundle/config/mutator/trampoline_test.go @@ -57,7 +57,7 @@ func TestGenerateTrampoline(t *testing.T) { } b := &bundle.Bundle{ - Path: tmpDir, + RootPath: tmpDir, Config: config.Root{ Bundle: config.Bundle{ Target: "development", diff --git a/bundle/config/mutator/translate_paths.go b/bundle/config/mutator/translate_paths.go index 768253e255..8fab3abb38 100644 --- a/bundle/config/mutator/translate_paths.go +++ b/bundle/config/mutator/translate_paths.go @@ -85,7 +85,7 @@ func (m *translatePaths) rewritePath( } // Remote path must be relative to the bundle root. - localRelPath, err := filepath.Rel(b.Path, localPath) + localRelPath, err := filepath.Rel(b.RootPath, localPath) if err != nil { return err } diff --git a/bundle/config/mutator/translate_paths_test.go b/bundle/config/mutator/translate_paths_test.go index 701a0b9c8b..9650ae8ba9 100644 --- a/bundle/config/mutator/translate_paths_test.go +++ b/bundle/config/mutator/translate_paths_test.go @@ -36,7 +36,7 @@ func touchEmptyFile(t *testing.T, path string) { func TestTranslatePathsSkippedWithGitSource(t *testing.T) { dir := t.TempDir() b := &bundle.Bundle{ - Path: dir, + RootPath: dir, Config: config.Root{ Workspace: config.Workspace{ FilePath: "/bundle", @@ -106,7 +106,7 @@ func TestTranslatePaths(t *testing.T) { touchEmptyFile(t, filepath.Join(dir, "dist", "task.jar")) b := &bundle.Bundle{ - Path: dir, + RootPath: dir, Config: config.Root{ Workspace: config.Workspace{ FilePath: "/bundle", @@ -273,7 +273,7 @@ func TestTranslatePathsInSubdirectories(t *testing.T) { touchEmptyFile(t, filepath.Join(dir, "job", "my_dbt_project", "dbt_project.yml")) b := &bundle.Bundle{ - Path: dir, + RootPath: dir, Config: config.Root{ Workspace: config.Workspace{ FilePath: "/bundle", @@ -367,7 +367,7 @@ func TestTranslatePathsOutsideBundleRoot(t *testing.T) { dir := t.TempDir() b := &bundle.Bundle{ - Path: dir, + RootPath: dir, Config: config.Root{ Workspace: config.Workspace{ FilePath: "/bundle", @@ -400,7 +400,7 @@ func TestJobNotebookDoesNotExistError(t *testing.T) { dir := t.TempDir() b := &bundle.Bundle{ - Path: dir, + RootPath: dir, Config: config.Root{ Resources: config.Resources{ Jobs: map[string]*resources.Job{ @@ -430,7 +430,7 @@ func TestJobFileDoesNotExistError(t *testing.T) { dir := t.TempDir() b := &bundle.Bundle{ - Path: dir, + RootPath: dir, Config: config.Root{ Resources: config.Resources{ Jobs: map[string]*resources.Job{ @@ -460,7 +460,7 @@ func TestPipelineNotebookDoesNotExistError(t *testing.T) { dir := t.TempDir() b := &bundle.Bundle{ - Path: dir, + RootPath: dir, Config: config.Root{ Resources: config.Resources{ Pipelines: map[string]*resources.Pipeline{ @@ -490,7 +490,7 @@ func TestPipelineFileDoesNotExistError(t *testing.T) { dir := t.TempDir() b := &bundle.Bundle{ - Path: dir, + RootPath: dir, Config: config.Root{ Resources: config.Resources{ Pipelines: map[string]*resources.Pipeline{ @@ -521,7 +521,7 @@ func TestJobSparkPythonTaskWithNotebookSourceError(t *testing.T) { touchNotebookFile(t, filepath.Join(dir, "my_notebook.py")) b := &bundle.Bundle{ - Path: dir, + RootPath: dir, Config: config.Root{ Workspace: config.Workspace{ FilePath: "/bundle", @@ -555,7 +555,7 @@ func TestJobNotebookTaskWithFileSourceError(t *testing.T) { touchEmptyFile(t, filepath.Join(dir, "my_file.py")) b := &bundle.Bundle{ - Path: dir, + RootPath: dir, Config: config.Root{ Workspace: config.Workspace{ FilePath: "/bundle", @@ -589,7 +589,7 @@ func TestPipelineNotebookLibraryWithFileSourceError(t *testing.T) { touchEmptyFile(t, filepath.Join(dir, "my_file.py")) b := &bundle.Bundle{ - Path: dir, + RootPath: dir, Config: config.Root{ Workspace: config.Workspace{ FilePath: "/bundle", @@ -623,7 +623,7 @@ func TestPipelineFileLibraryWithNotebookSourceError(t *testing.T) { touchNotebookFile(t, filepath.Join(dir, "my_notebook.py")) b := &bundle.Bundle{ - Path: dir, + RootPath: dir, Config: config.Root{ Workspace: config.Workspace{ FilePath: "/bundle", diff --git a/bundle/deploy/files/sync.go b/bundle/deploy/files/sync.go index c74e41090d..e8c54c6332 100644 --- a/bundle/deploy/files/sync.go +++ b/bundle/deploy/files/sync.go @@ -28,7 +28,7 @@ func GetSyncOptions(ctx context.Context, b *bundle.Bundle) (*sync.SyncOptions, e } opts := &sync.SyncOptions{ - LocalPath: b.Path, + LocalPath: b.RootPath, RemotePath: b.Config.Workspace.FilePath, Include: includes, Exclude: b.Config.Sync.Exclude, diff --git a/bundle/deploy/metadata/compute.go b/bundle/deploy/metadata/compute.go index d10ed1e4c7..0347654848 100644 --- a/bundle/deploy/metadata/compute.go +++ b/bundle/deploy/metadata/compute.go @@ -39,7 +39,7 @@ func (m *compute) Apply(_ context.Context, b *bundle.Bundle) diag.Diagnostics { for name, job := range b.Config.Resources.Jobs { // Compute config file path the job is defined in, relative to the bundle // root - relativePath, err := filepath.Rel(b.Path, job.ConfigFilePath) + relativePath, err := filepath.Rel(b.RootPath, job.ConfigFilePath) if err != nil { return diag.Errorf("failed to compute relative path for job %s: %v", name, err) } diff --git a/bundle/deploy/state_pull.go b/bundle/deploy/state_pull.go index 6f8591d6bd..bae457ea09 100644 --- a/bundle/deploy/state_pull.go +++ b/bundle/deploy/state_pull.go @@ -85,7 +85,7 @@ func (s *statePull) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostic } log.Infof(ctx, "Creating new snapshot") - snapshot, err := sync.NewSnapshot(state.Files.ToSlice(b.Path), opts) + snapshot, err := sync.NewSnapshot(state.Files.ToSlice(b.RootPath), opts) if err != nil { return diag.FromErr(err) } diff --git a/bundle/deploy/state_pull_test.go b/bundle/deploy/state_pull_test.go index a0ac8eeb42..80acb254f4 100644 --- a/bundle/deploy/state_pull_test.go +++ b/bundle/deploy/state_pull_test.go @@ -59,7 +59,7 @@ func testStatePull(t *testing.T, opts statePullOpts) { }} b := &bundle.Bundle{ - Path: t.TempDir(), + RootPath: t.TempDir(), Config: config.Root{ Bundle: config.Bundle{ Target: "default", @@ -77,11 +77,11 @@ func testStatePull(t *testing.T, opts statePullOpts) { ctx := context.Background() for _, file := range opts.localFiles { - testutil.Touch(t, filepath.Join(b.Path, "bar"), file) + testutil.Touch(t, filepath.Join(b.RootPath, "bar"), file) } for _, file := range opts.localNotebooks { - testutil.TouchNotebook(t, filepath.Join(b.Path, "bar"), file) + testutil.TouchNotebook(t, filepath.Join(b.RootPath, "bar"), file) } if opts.withExistingSnapshot { @@ -251,7 +251,7 @@ func TestStatePullNoState(t *testing.T) { }} b := &bundle.Bundle{ - Path: t.TempDir(), + RootPath: t.TempDir(), Config: config.Root{ Bundle: config.Bundle{ Target: "default", @@ -439,7 +439,7 @@ func TestStatePullNewerDeploymentStateVersion(t *testing.T) { }} b := &bundle.Bundle{ - Path: t.TempDir(), + RootPath: t.TempDir(), Config: config.Root{ Bundle: config.Bundle{ Target: "default", diff --git a/bundle/deploy/state_push_test.go b/bundle/deploy/state_push_test.go index 39d6366ddd..39e4d13a56 100644 --- a/bundle/deploy/state_push_test.go +++ b/bundle/deploy/state_push_test.go @@ -45,7 +45,7 @@ func TestStatePush(t *testing.T) { }} b := &bundle.Bundle{ - Path: t.TempDir(), + RootPath: t.TempDir(), Config: config.Root{ Bundle: config.Bundle{ Target: "default", diff --git a/bundle/deploy/state_update_test.go b/bundle/deploy/state_update_test.go index 1127331735..dd8a1336ec 100644 --- a/bundle/deploy/state_update_test.go +++ b/bundle/deploy/state_update_test.go @@ -22,7 +22,7 @@ func TestStateUpdate(t *testing.T) { s := &stateUpdate{} b := &bundle.Bundle{ - Path: t.TempDir(), + RootPath: t.TempDir(), Config: config.Root{ Bundle: config.Bundle{ Target: "default", @@ -39,8 +39,8 @@ func TestStateUpdate(t *testing.T) { }, } - testutil.Touch(t, b.Path, "test1.py") - testutil.Touch(t, b.Path, "test2.py") + testutil.Touch(t, b.RootPath, "test1.py") + testutil.Touch(t, b.RootPath, "test2.py") m := mocks.NewMockWorkspaceClient(t) m.WorkspaceClient.Config = &databrickscfg.Config{ @@ -82,7 +82,7 @@ func TestStateUpdateWithExistingState(t *testing.T) { s := &stateUpdate{} b := &bundle.Bundle{ - Path: t.TempDir(), + RootPath: t.TempDir(), Config: config.Root{ Bundle: config.Bundle{ Target: "default", @@ -99,8 +99,8 @@ func TestStateUpdateWithExistingState(t *testing.T) { }, } - testutil.Touch(t, b.Path, "test1.py") - testutil.Touch(t, b.Path, "test2.py") + testutil.Touch(t, b.RootPath, "test1.py") + testutil.Touch(t, b.RootPath, "test2.py") m := mocks.NewMockWorkspaceClient(t) m.WorkspaceClient.Config = &databrickscfg.Config{ diff --git a/bundle/deploy/terraform/init_test.go b/bundle/deploy/terraform/init_test.go index e1a04c31d8..29bd80a3e0 100644 --- a/bundle/deploy/terraform/init_test.go +++ b/bundle/deploy/terraform/init_test.go @@ -28,7 +28,7 @@ func TestInitEnvironmentVariables(t *testing.T) { } b := &bundle.Bundle{ - Path: t.TempDir(), + RootPath: t.TempDir(), Config: config.Root{ Bundle: config.Bundle{ Target: "whatever", @@ -55,7 +55,7 @@ func TestSetTempDirEnvVarsForUnixWithTmpDirSet(t *testing.T) { } b := &bundle.Bundle{ - Path: t.TempDir(), + RootPath: t.TempDir(), Config: config.Root{ Bundle: config.Bundle{ Target: "whatever", @@ -83,7 +83,7 @@ func TestSetTempDirEnvVarsForUnixWithTmpDirNotSet(t *testing.T) { } b := &bundle.Bundle{ - Path: t.TempDir(), + RootPath: t.TempDir(), Config: config.Root{ Bundle: config.Bundle{ Target: "whatever", @@ -109,7 +109,7 @@ func TestSetTempDirEnvVarsForWindowWithAllTmpDirEnvVarsSet(t *testing.T) { } b := &bundle.Bundle{ - Path: t.TempDir(), + RootPath: t.TempDir(), Config: config.Root{ Bundle: config.Bundle{ Target: "whatever", @@ -139,7 +139,7 @@ func TestSetTempDirEnvVarsForWindowWithUserProfileAndTempSet(t *testing.T) { } b := &bundle.Bundle{ - Path: t.TempDir(), + RootPath: t.TempDir(), Config: config.Root{ Bundle: config.Bundle{ Target: "whatever", @@ -169,7 +169,7 @@ func TestSetTempDirEnvVarsForWindowsWithoutAnyTempDirEnvVarsSet(t *testing.T) { } b := &bundle.Bundle{ - Path: t.TempDir(), + RootPath: t.TempDir(), Config: config.Root{ Bundle: config.Bundle{ Target: "whatever", @@ -197,7 +197,7 @@ func TestSetTempDirEnvVarsForWindowsWithoutAnyTempDirEnvVarsSet(t *testing.T) { func TestSetProxyEnvVars(t *testing.T) { b := &bundle.Bundle{ - Path: t.TempDir(), + RootPath: t.TempDir(), Config: config.Root{ Bundle: config.Bundle{ Target: "whatever", diff --git a/bundle/deploy/terraform/load_test.go b/bundle/deploy/terraform/load_test.go index d18ac05e96..c62217187d 100644 --- a/bundle/deploy/terraform/load_test.go +++ b/bundle/deploy/terraform/load_test.go @@ -17,7 +17,7 @@ func TestLoadWithNoState(t *testing.T) { } b := &bundle.Bundle{ - Path: t.TempDir(), + RootPath: t.TempDir(), Config: config.Root{ Bundle: config.Bundle{ Target: "whatever", diff --git a/bundle/deploy/terraform/state_pull_test.go b/bundle/deploy/terraform/state_pull_test.go index f4e53762f7..26297bfcbe 100644 --- a/bundle/deploy/terraform/state_pull_test.go +++ b/bundle/deploy/terraform/state_pull_test.go @@ -32,7 +32,7 @@ func mockStateFilerForPull(t *testing.T, contents map[string]int, merr error) fi func statePullTestBundle(t *testing.T) *bundle.Bundle { return &bundle.Bundle{ - Path: t.TempDir(), + RootPath: t.TempDir(), Config: config.Root{ Bundle: config.Bundle{ Target: "default", diff --git a/bundle/deploy/terraform/state_push_test.go b/bundle/deploy/terraform/state_push_test.go index b1ddd23840..e054773f31 100644 --- a/bundle/deploy/terraform/state_push_test.go +++ b/bundle/deploy/terraform/state_push_test.go @@ -29,7 +29,7 @@ func mockStateFilerForPush(t *testing.T, fn func(body io.Reader)) filer.Filer { func statePushTestBundle(t *testing.T) *bundle.Bundle { return &bundle.Bundle{ - Path: t.TempDir(), + RootPath: t.TempDir(), Config: config.Root{ Bundle: config.Bundle{ Target: "default", diff --git a/bundle/libraries/libraries.go b/bundle/libraries/libraries.go index 971f6e0f90..8dd63a75a6 100644 --- a/bundle/libraries/libraries.go +++ b/bundle/libraries/libraries.go @@ -65,7 +65,7 @@ func findLibraryMatches(lib *compute.Library, b *bundle.Bundle) ([]string, error return nil, nil } - fullPath := filepath.Join(b.Path, path) + fullPath := filepath.Join(b.RootPath, path) return filepath.Glob(fullPath) } diff --git a/bundle/libraries/libraries_test.go b/bundle/libraries/libraries_test.go index 873b322a3e..3da10d47bb 100644 --- a/bundle/libraries/libraries_test.go +++ b/bundle/libraries/libraries_test.go @@ -15,7 +15,7 @@ import ( func TestMapFilesToTaskLibrariesNoGlob(t *testing.T) { b := &bundle.Bundle{ - Path: "testdata", + RootPath: "testdata", Config: config.Root{ Resources: config.Resources{ Jobs: map[string]*resources.Job{ diff --git a/bundle/python/conditional_transform_test.go b/bundle/python/conditional_transform_test.go index 39ddd46334..677970d70a 100644 --- a/bundle/python/conditional_transform_test.go +++ b/bundle/python/conditional_transform_test.go @@ -18,7 +18,7 @@ func TestNoTransformByDefault(t *testing.T) { tmpDir := t.TempDir() b := &bundle.Bundle{ - Path: tmpDir, + RootPath: tmpDir, Config: config.Root{ Bundle: config.Bundle{ Target: "development", @@ -63,7 +63,7 @@ func TestTransformWithExperimentalSettingSetToTrue(t *testing.T) { tmpDir := t.TempDir() b := &bundle.Bundle{ - Path: tmpDir, + RootPath: tmpDir, Config: config.Root{ Bundle: config.Bundle{ Target: "development", @@ -106,7 +106,7 @@ func TestTransformWithExperimentalSettingSetToTrue(t *testing.T) { dir, err := b.InternalDir(context.Background()) require.NoError(t, err) - internalDirRel, err := filepath.Rel(b.Path, dir) + internalDirRel, err := filepath.Rel(b.RootPath, dir) require.NoError(t, err) require.Equal(t, path.Join(filepath.ToSlash(internalDirRel), "notebook_job1_key1"), task.NotebookTask.NotebookPath) diff --git a/bundle/python/transform_test.go b/bundle/python/transform_test.go index d49bea59a4..c15feb4241 100644 --- a/bundle/python/transform_test.go +++ b/bundle/python/transform_test.go @@ -116,7 +116,7 @@ func TestTransformFiltersWheelTasksOnly(t *testing.T) { func TestNoPanicWithNoPythonWheelTasks(t *testing.T) { tmpDir := t.TempDir() b := &bundle.Bundle{ - Path: tmpDir, + RootPath: tmpDir, Config: config.Root{ Bundle: config.Bundle{ Target: "development", diff --git a/bundle/root_test.go b/bundle/root_test.go index d9365f3599..a83f36ace7 100644 --- a/bundle/root_test.go +++ b/bundle/root_test.go @@ -106,7 +106,7 @@ func TestLoadYamlWhenIncludesEnvPresent(t *testing.T) { cwd, err := os.Getwd() assert.NoError(t, err) - assert.Equal(t, cwd, bundle.Path) + assert.Equal(t, cwd, bundle.RootPath) } func TestLoadDefautlBundleWhenNoYamlAndRootAndIncludesEnvPresent(t *testing.T) { @@ -118,7 +118,7 @@ func TestLoadDefautlBundleWhenNoYamlAndRootAndIncludesEnvPresent(t *testing.T) { bundle, err := MustLoad(ctx) assert.NoError(t, err) - assert.Equal(t, dir, bundle.Path) + assert.Equal(t, dir, bundle.RootPath) } func TestErrorIfNoYamlNoRootEnvAndIncludesEnvPresent(t *testing.T) { diff --git a/bundle/scripts/scripts.go b/bundle/scripts/scripts.go index e59e6a6415..38d204f99e 100644 --- a/bundle/scripts/scripts.go +++ b/bundle/scripts/scripts.go @@ -30,7 +30,7 @@ func (m *script) Name() string { } func (m *script) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { - executor, err := exec.NewCommandExecutor(b.Path) + executor, err := exec.NewCommandExecutor(b.RootPath) if err != nil { return diag.FromErr(err) } diff --git a/bundle/scripts/scripts_test.go b/bundle/scripts/scripts_test.go index 2034202e0d..1bc216b610 100644 --- a/bundle/scripts/scripts_test.go +++ b/bundle/scripts/scripts_test.go @@ -23,7 +23,7 @@ func TestExecutesHook(t *testing.T) { }, } - executor, err := exec.NewCommandExecutor(b.Path) + executor, err := exec.NewCommandExecutor(b.RootPath) require.NoError(t, err) _, out, err := executeHook(context.Background(), executor, b, config.ScriptPreBuild) require.NoError(t, err) diff --git a/bundle/tests/python_wheel_test.go b/bundle/tests/python_wheel_test.go index 655b3df6b9..412b507fe9 100644 --- a/bundle/tests/python_wheel_test.go +++ b/bundle/tests/python_wheel_test.go @@ -79,7 +79,7 @@ func TestPythonWheelBuildNoBuildJustUpload(t *testing.T) { artifact := b.Config.Artifacts["my_test_code-0.0.1-py3-none-any.whl"] require.NotNil(t, artifact) require.Empty(t, artifact.BuildCommand) - require.Contains(t, artifact.Files[0].Source, filepath.Join(b.Path, "package", + require.Contains(t, artifact.Files[0].Source, filepath.Join(b.RootPath, "package", "my_test_code-0.0.1-py3-none-any.whl", )) } diff --git a/cmd/bundle/generate/generate_test.go b/cmd/bundle/generate/generate_test.go index 691110668a..69ef639ae0 100644 --- a/cmd/bundle/generate/generate_test.go +++ b/cmd/bundle/generate/generate_test.go @@ -24,7 +24,7 @@ func TestGeneratePipelineCommand(t *testing.T) { root := t.TempDir() b := &bundle.Bundle{ - Path: root, + RootPath: root, } m := mocks.NewMockWorkspaceClient(t) @@ -122,7 +122,7 @@ func TestGenerateJobCommand(t *testing.T) { root := t.TempDir() b := &bundle.Bundle{ - Path: root, + RootPath: root, } m := mocks.NewMockWorkspaceClient(t) diff --git a/cmd/sync/sync_test.go b/cmd/sync/sync_test.go index e8754ef560..026d840f73 100644 --- a/cmd/sync/sync_test.go +++ b/cmd/sync/sync_test.go @@ -16,7 +16,7 @@ import ( func TestSyncOptionsFromBundle(t *testing.T) { tempDir := t.TempDir() b := &bundle.Bundle{ - Path: tempDir, + RootPath: tempDir, Config: config.Root{ Bundle: config.Bundle{ Target: "default", diff --git a/internal/bundle/artifacts_test.go b/internal/bundle/artifacts_test.go index 8f6d117d00..866a1f6e9e 100644 --- a/internal/bundle/artifacts_test.go +++ b/internal/bundle/artifacts_test.go @@ -36,7 +36,7 @@ func TestAccUploadArtifactFileToCorrectRemotePath(t *testing.T) { wsDir := internal.TemporaryWorkspaceDir(t, w) b := &bundle.Bundle{ - Path: dir, + RootPath: dir, Config: config.Root{ Bundle: config.Bundle{ Target: "whatever", From bd14db077d338364e675fe44c76c8b99afb0f338 Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Tue, 26 Mar 2024 17:27:38 +0100 Subject: [PATCH 03/15] Fix --- bundle/bundle.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/bundle/bundle.go b/bundle/bundle.go index d5673b493e..0aa44df0b9 100644 --- a/bundle/bundle.go +++ b/bundle/bundle.go @@ -67,7 +67,9 @@ type Bundle struct { } func Load(ctx context.Context, path string) (*Bundle, error) { - b := &Bundle{} + b := &Bundle{ + RootPath: filepath.Clean(path), + } stat, err := os.Stat(path) if err != nil { return nil, err @@ -78,7 +80,6 @@ func Load(ctx context.Context, path string) (*Bundle, error) { _, hasIncludesEnv := env.Includes(ctx) if hasRootEnv && hasIncludesEnv && stat.IsDir() { log.Debugf(ctx, "No bundle configuration; using bundle root: %s", path) - b.RootPath = path b.Config = config.Root{ Bundle: config.Bundle{ Name: filepath.Base(path), From 99f027ef9cdf43d74107473e0f4a2c02faaef5e7 Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Tue, 26 Mar 2024 19:39:38 +0100 Subject: [PATCH 04/15] Remove support for DATABRICKS_BUNDLE_INCLUDES PR #604 added functionality to load a bundle without a `databricks.yml` if both the `DATABRICKS_BUNDLE_ROOT` and `DATABRICKS_BUNDLE_INCLUDES` environment variables were set. We never ended up using this in downstream tools so this can be removed. --- bundle/bundle.go | 15 ------ .../config/mutator/process_root_includes.go | 23 --------- .../mutator/process_root_includes_test.go | 40 ---------------- bundle/env/includes.go | 14 ------ bundle/env/includes_test.go | 28 ----------- bundle/root_test.go | 47 ------------------- 6 files changed, 167 deletions(-) delete mode 100644 bundle/env/includes.go delete mode 100644 bundle/env/includes_test.go diff --git a/bundle/bundle.go b/bundle/bundle.go index 0aa44df0b9..2e193bbf39 100644 --- a/bundle/bundle.go +++ b/bundle/bundle.go @@ -70,23 +70,8 @@ func Load(ctx context.Context, path string) (*Bundle, error) { b := &Bundle{ RootPath: filepath.Clean(path), } - stat, err := os.Stat(path) - if err != nil { - return nil, err - } configFile, err := config.FileNames.FindInPath(path) if err != nil { - _, hasRootEnv := env.Root(ctx) - _, hasIncludesEnv := env.Includes(ctx) - if hasRootEnv && hasIncludesEnv && stat.IsDir() { - log.Debugf(ctx, "No bundle configuration; using bundle root: %s", path) - b.Config = config.Root{ - Bundle: config.Bundle{ - Name: filepath.Base(path), - }, - } - return b, nil - } return nil, err } log.Debugf(ctx, "Loading bundle configuration from: %s", configFile) diff --git a/bundle/config/mutator/process_root_includes.go b/bundle/config/mutator/process_root_includes.go index 4e4aeef43c..c5e0a22c5e 100644 --- a/bundle/config/mutator/process_root_includes.go +++ b/bundle/config/mutator/process_root_includes.go @@ -2,26 +2,15 @@ package mutator import ( "context" - "os" "path/filepath" "slices" "strings" "github.com/databricks/cli/bundle" "github.com/databricks/cli/bundle/config" - "github.com/databricks/cli/bundle/env" "github.com/databricks/cli/libs/diag" ) -// Get extra include paths from environment variable -func getExtraIncludePaths(ctx context.Context) []string { - value, exists := env.Includes(ctx) - if !exists { - return nil - } - return strings.Split(value, string(os.PathListSeparator)) -} - type processRootIncludes struct{} // ProcessRootIncludes expands the patterns in the configuration's include list @@ -48,18 +37,6 @@ func (m *processRootIncludes) Apply(ctx context.Context, b *bundle.Bundle) diag. // This is stored in the bundle configuration for observability. var files []string - // Converts extra include paths from environment variable to relative paths - for _, extraIncludePath := range getExtraIncludePaths(ctx) { - if filepath.IsAbs(extraIncludePath) { - rel, err := filepath.Rel(b.RootPath, extraIncludePath) - if err != nil { - return diag.Errorf("unable to include file '%s': %v", extraIncludePath, err) - } - extraIncludePath = rel - } - b.Config.Include = append(b.Config.Include, extraIncludePath) - } - // For each glob, find all files to load. // Ordering of the list of globs is maintained in the output. // For matches that appear in multiple globs, only the first is kept. diff --git a/bundle/config/mutator/process_root_includes_test.go b/bundle/config/mutator/process_root_includes_test.go index d3aaa974d6..675dd9acfb 100644 --- a/bundle/config/mutator/process_root_includes_test.go +++ b/bundle/config/mutator/process_root_includes_test.go @@ -2,16 +2,12 @@ package mutator_test import ( "context" - "os" - "path" "runtime" - "strings" "testing" "github.com/databricks/cli/bundle" "github.com/databricks/cli/bundle/config" "github.com/databricks/cli/bundle/config/mutator" - "github.com/databricks/cli/bundle/env" "github.com/databricks/cli/internal/testutil" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -115,39 +111,3 @@ func TestProcessRootIncludesNotExists(t *testing.T) { require.True(t, diags.HasError()) assert.ErrorContains(t, diags.Error(), "notexist.yml defined in 'include' section does not match any files") } - -func TestProcessRootIncludesExtrasFromEnvVar(t *testing.T) { - rootPath := t.TempDir() - testYamlName := "extra_include_path.yml" - testutil.Touch(t, rootPath, testYamlName) - t.Setenv(env.IncludesVariable, path.Join(rootPath, testYamlName)) - - b := &bundle.Bundle{ - RootPath: rootPath, - } - - diags := bundle.Apply(context.Background(), b, mutator.ProcessRootIncludes()) - require.NoError(t, diags.Error()) - assert.Contains(t, b.Config.Include, testYamlName) -} - -func TestProcessRootIncludesDedupExtrasFromEnvVar(t *testing.T) { - rootPath := t.TempDir() - testYamlName := "extra_include_path.yml" - testutil.Touch(t, rootPath, testYamlName) - t.Setenv(env.IncludesVariable, strings.Join( - []string{ - path.Join(rootPath, testYamlName), - path.Join(rootPath, testYamlName), - }, - string(os.PathListSeparator), - )) - - b := &bundle.Bundle{ - RootPath: rootPath, - } - - diags := bundle.Apply(context.Background(), b, mutator.ProcessRootIncludes()) - require.NoError(t, diags.Error()) - assert.Equal(t, []string{testYamlName}, b.Config.Include) -} diff --git a/bundle/env/includes.go b/bundle/env/includes.go deleted file mode 100644 index 4ade018776..0000000000 --- a/bundle/env/includes.go +++ /dev/null @@ -1,14 +0,0 @@ -package env - -import "context" - -// IncludesVariable names the environment variable that holds additional configuration paths to include -// during bundle configuration loading. Also see `bundle/config/mutator/process_root_includes.go`. -const IncludesVariable = "DATABRICKS_BUNDLE_INCLUDES" - -// Includes returns the bundle Includes environment variable. -func Includes(ctx context.Context) (string, bool) { - return get(ctx, []string{ - IncludesVariable, - }) -} diff --git a/bundle/env/includes_test.go b/bundle/env/includes_test.go deleted file mode 100644 index d9366a59ff..0000000000 --- a/bundle/env/includes_test.go +++ /dev/null @@ -1,28 +0,0 @@ -package env - -import ( - "context" - "testing" - - "github.com/databricks/cli/internal/testutil" - "github.com/stretchr/testify/assert" -) - -func TestIncludes(t *testing.T) { - ctx := context.Background() - - testutil.CleanupEnvironment(t) - - t.Run("set", func(t *testing.T) { - t.Setenv("DATABRICKS_BUNDLE_INCLUDES", "foo") - includes, ok := Includes(ctx) - assert.True(t, ok) - assert.Equal(t, "foo", includes) - }) - - t.Run("not set", func(t *testing.T) { - includes, ok := Includes(ctx) - assert.False(t, ok) - assert.Equal(t, "", includes) - }) -} diff --git a/bundle/root_test.go b/bundle/root_test.go index a83f36ace7..99bf58a00a 100644 --- a/bundle/root_test.go +++ b/bundle/root_test.go @@ -9,7 +9,6 @@ import ( "github.com/databricks/cli/bundle/config" "github.com/databricks/cli/bundle/env" "github.com/databricks/cli/internal/testutil" - "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -94,49 +93,3 @@ func TestRootLookupError(t *testing.T) { _, err := mustGetRoot(ctx) require.ErrorContains(t, err, "unable to locate bundle root") } - -func TestLoadYamlWhenIncludesEnvPresent(t *testing.T) { - ctx := context.Background() - testutil.Chdir(t, filepath.Join(".", "tests", "basic")) - t.Setenv(env.IncludesVariable, "test") - - bundle, err := MustLoad(ctx) - assert.NoError(t, err) - assert.Equal(t, "basic", bundle.Config.Bundle.Name) - - cwd, err := os.Getwd() - assert.NoError(t, err) - assert.Equal(t, cwd, bundle.RootPath) -} - -func TestLoadDefautlBundleWhenNoYamlAndRootAndIncludesEnvPresent(t *testing.T) { - ctx := context.Background() - dir := t.TempDir() - testutil.Chdir(t, dir) - t.Setenv(env.RootVariable, dir) - t.Setenv(env.IncludesVariable, "test") - - bundle, err := MustLoad(ctx) - assert.NoError(t, err) - assert.Equal(t, dir, bundle.RootPath) -} - -func TestErrorIfNoYamlNoRootEnvAndIncludesEnvPresent(t *testing.T) { - ctx := context.Background() - dir := t.TempDir() - testutil.Chdir(t, dir) - t.Setenv(env.IncludesVariable, "test") - - _, err := MustLoad(ctx) - assert.Error(t, err) -} - -func TestErrorIfNoYamlNoIncludesEnvAndRootEnvPresent(t *testing.T) { - ctx := context.Background() - dir := t.TempDir() - testutil.Chdir(t, dir) - t.Setenv(env.RootVariable, dir) - - _, err := MustLoad(ctx) - assert.Error(t, err) -} From 503e3e46d6f7de1d1adbe7ba8baa1c77a59eccec Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Tue, 26 Mar 2024 19:34:27 +0100 Subject: [PATCH 05/15] Load bundle entry point from mutator --- bundle/config/loader/entry_point.go | 34 +++++++++++++++++++ bundle/config/loader/entry_point_test.go | 26 ++++++++++++++ .../{mutator => loader}/process_include.go | 2 +- .../process_include_test.go | 21 +++++------- .../process_root_includes.go | 2 +- .../process_root_includes_test.go | 16 ++++----- bundle/config/loader/testdata/databricks.yml | 2 ++ bundle/config/loader/testdata/host.yml | 2 ++ bundle/config/mutator/mutator.go | 3 +- 9 files changed, 85 insertions(+), 23 deletions(-) create mode 100644 bundle/config/loader/entry_point.go create mode 100644 bundle/config/loader/entry_point_test.go rename bundle/config/{mutator => loader}/process_include.go (98%) rename bundle/config/{mutator => loader}/process_include_test.go (54%) rename bundle/config/{mutator => loader}/process_root_includes.go (99%) rename bundle/config/{mutator => loader}/process_root_includes_test.go (80%) create mode 100644 bundle/config/loader/testdata/databricks.yml create mode 100644 bundle/config/loader/testdata/host.yml diff --git a/bundle/config/loader/entry_point.go b/bundle/config/loader/entry_point.go new file mode 100644 index 0000000000..24ba2f068e --- /dev/null +++ b/bundle/config/loader/entry_point.go @@ -0,0 +1,34 @@ +package loader + +import ( + "context" + + "github.com/databricks/cli/bundle" + "github.com/databricks/cli/bundle/config" + "github.com/databricks/cli/libs/diag" +) + +type entryPoint struct{} + +// EntryPoint loads the entry point configuration. +func EntryPoint() bundle.Mutator { + return &entryPoint{} +} + +func (m *entryPoint) Name() string { + return "EntryPoint" +} + +func (m *entryPoint) Apply(_ context.Context, b *bundle.Bundle) diag.Diagnostics { + path, err := config.FileNames.FindInPath(b.RootPath) + if err != nil { + return diag.FromErr(err) + } + this, err := config.Load(path) + if err != nil { + return diag.FromErr(err) + } + // TODO: Return actual warnings. + err = b.Config.Merge(this) + return diag.FromErr(err) +} diff --git a/bundle/config/loader/entry_point_test.go b/bundle/config/loader/entry_point_test.go new file mode 100644 index 0000000000..80271f0b74 --- /dev/null +++ b/bundle/config/loader/entry_point_test.go @@ -0,0 +1,26 @@ +package loader_test + +import ( + "context" + "testing" + + "github.com/databricks/cli/bundle" + "github.com/databricks/cli/bundle/config/loader" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestEntryPointNoRootPath(t *testing.T) { + b := &bundle.Bundle{} + diags := bundle.Apply(context.Background(), b, loader.EntryPoint()) + require.Error(t, diags.Error()) +} + +func TestEntryPoint(t *testing.T) { + b := &bundle.Bundle{ + RootPath: "testdata", + } + diags := bundle.Apply(context.Background(), b, loader.EntryPoint()) + require.NoError(t, diags.Error()) + assert.Equal(t, "loader_test", b.Config.Bundle.Name) +} diff --git a/bundle/config/mutator/process_include.go b/bundle/config/loader/process_include.go similarity index 98% rename from bundle/config/mutator/process_include.go rename to bundle/config/loader/process_include.go index 23acdf12a0..328f4eacf3 100644 --- a/bundle/config/mutator/process_include.go +++ b/bundle/config/loader/process_include.go @@ -1,4 +1,4 @@ -package mutator +package loader import ( "context" diff --git a/bundle/config/mutator/process_include_test.go b/bundle/config/loader/process_include_test.go similarity index 54% rename from bundle/config/mutator/process_include_test.go rename to bundle/config/loader/process_include_test.go index b4fa3ccda5..da4da9ff66 100644 --- a/bundle/config/mutator/process_include_test.go +++ b/bundle/config/loader/process_include_test.go @@ -1,22 +1,20 @@ -package mutator_test +package loader_test import ( "context" - "fmt" - "os" "path/filepath" "testing" "github.com/databricks/cli/bundle" "github.com/databricks/cli/bundle/config" - "github.com/databricks/cli/bundle/config/mutator" + "github.com/databricks/cli/bundle/config/loader" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) func TestProcessInclude(t *testing.T) { b := &bundle.Bundle{ - RootPath: t.TempDir(), + RootPath: "testdata", Config: config.Root{ Workspace: config.Workspace{ Host: "foo", @@ -24,15 +22,14 @@ func TestProcessInclude(t *testing.T) { }, } - relPath := "./file.yml" - fullPath := filepath.Join(b.RootPath, relPath) - f, err := os.Create(fullPath) - require.NoError(t, err) - fmt.Fprint(f, "workspace:\n host: bar\n") - f.Close() + m := loader.ProcessInclude(filepath.Join(b.RootPath, "host.yml"), "host.yml") + assert.Equal(t, "ProcessInclude(host.yml)", m.Name()) + // Assert the host value prior to applying the mutator assert.Equal(t, "foo", b.Config.Workspace.Host) - diags := bundle.Apply(context.Background(), b, mutator.ProcessInclude(fullPath, relPath)) + + // Apply the mutator and assert that the host value has been updated + diags := bundle.Apply(context.Background(), b, m) require.NoError(t, diags.Error()) assert.Equal(t, "bar", b.Config.Workspace.Host) } diff --git a/bundle/config/mutator/process_root_includes.go b/bundle/config/loader/process_root_includes.go similarity index 99% rename from bundle/config/mutator/process_root_includes.go rename to bundle/config/loader/process_root_includes.go index c5e0a22c5e..25f284fd3a 100644 --- a/bundle/config/mutator/process_root_includes.go +++ b/bundle/config/loader/process_root_includes.go @@ -1,4 +1,4 @@ -package mutator +package loader import ( "context" diff --git a/bundle/config/mutator/process_root_includes_test.go b/bundle/config/loader/process_root_includes_test.go similarity index 80% rename from bundle/config/mutator/process_root_includes_test.go rename to bundle/config/loader/process_root_includes_test.go index 675dd9acfb..737dbbefd1 100644 --- a/bundle/config/mutator/process_root_includes_test.go +++ b/bundle/config/loader/process_root_includes_test.go @@ -1,4 +1,4 @@ -package mutator_test +package loader_test import ( "context" @@ -7,7 +7,7 @@ import ( "github.com/databricks/cli/bundle" "github.com/databricks/cli/bundle/config" - "github.com/databricks/cli/bundle/config/mutator" + "github.com/databricks/cli/bundle/config/loader" "github.com/databricks/cli/internal/testutil" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -17,7 +17,7 @@ func TestProcessRootIncludesEmpty(t *testing.T) { b := &bundle.Bundle{ RootPath: ".", } - diags := bundle.Apply(context.Background(), b, mutator.ProcessRootIncludes()) + diags := bundle.Apply(context.Background(), b, loader.ProcessRootIncludes()) require.NoError(t, diags.Error()) } @@ -37,7 +37,7 @@ func TestProcessRootIncludesAbs(t *testing.T) { }, }, } - diags := bundle.Apply(context.Background(), b, mutator.ProcessRootIncludes()) + diags := bundle.Apply(context.Background(), b, loader.ProcessRootIncludes()) require.True(t, diags.HasError()) assert.ErrorContains(t, diags.Error(), "must be relative paths") } @@ -56,7 +56,7 @@ func TestProcessRootIncludesSingleGlob(t *testing.T) { testutil.Touch(t, b.RootPath, "a.yml") testutil.Touch(t, b.RootPath, "b.yml") - diags := bundle.Apply(context.Background(), b, mutator.ProcessRootIncludes()) + diags := bundle.Apply(context.Background(), b, loader.ProcessRootIncludes()) require.NoError(t, diags.Error()) assert.Equal(t, []string{"a.yml", "b.yml"}, b.Config.Include) } @@ -75,7 +75,7 @@ func TestProcessRootIncludesMultiGlob(t *testing.T) { testutil.Touch(t, b.RootPath, "a1.yml") testutil.Touch(t, b.RootPath, "b1.yml") - diags := bundle.Apply(context.Background(), b, mutator.ProcessRootIncludes()) + diags := bundle.Apply(context.Background(), b, loader.ProcessRootIncludes()) require.NoError(t, diags.Error()) assert.Equal(t, []string{"a1.yml", "b1.yml"}, b.Config.Include) } @@ -93,7 +93,7 @@ func TestProcessRootIncludesRemoveDups(t *testing.T) { testutil.Touch(t, b.RootPath, "a.yml") - diags := bundle.Apply(context.Background(), b, mutator.ProcessRootIncludes()) + diags := bundle.Apply(context.Background(), b, loader.ProcessRootIncludes()) require.NoError(t, diags.Error()) assert.Equal(t, []string{"a.yml"}, b.Config.Include) } @@ -107,7 +107,7 @@ func TestProcessRootIncludesNotExists(t *testing.T) { }, }, } - diags := bundle.Apply(context.Background(), b, mutator.ProcessRootIncludes()) + diags := bundle.Apply(context.Background(), b, loader.ProcessRootIncludes()) require.True(t, diags.HasError()) assert.ErrorContains(t, diags.Error(), "notexist.yml defined in 'include' section does not match any files") } diff --git a/bundle/config/loader/testdata/databricks.yml b/bundle/config/loader/testdata/databricks.yml new file mode 100644 index 0000000000..1a0635b898 --- /dev/null +++ b/bundle/config/loader/testdata/databricks.yml @@ -0,0 +1,2 @@ +bundle: + name: loader_test diff --git a/bundle/config/loader/testdata/host.yml b/bundle/config/loader/testdata/host.yml new file mode 100644 index 0000000000..f83830d1d1 --- /dev/null +++ b/bundle/config/loader/testdata/host.yml @@ -0,0 +1,2 @@ +workspace: + host: bar diff --git a/bundle/config/mutator/mutator.go b/bundle/config/mutator/mutator.go index c45a6c15e1..10842a93b9 100644 --- a/bundle/config/mutator/mutator.go +++ b/bundle/config/mutator/mutator.go @@ -3,13 +3,14 @@ package mutator import ( "github.com/databricks/cli/bundle" "github.com/databricks/cli/bundle/config" + "github.com/databricks/cli/bundle/config/loader" "github.com/databricks/cli/bundle/scripts" ) func DefaultMutators() []bundle.Mutator { return []bundle.Mutator{ scripts.Execute(config.ScriptPreInit), - ProcessRootIncludes(), + loader.ProcessRootIncludes(), EnvironmentsToTargets(), InitializeVariables(), DefineDefaultTarget(), From 64a5bed8b19b92cbdbbf4ede154abab65c84a22d Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Tue, 26 Mar 2024 20:40:09 +0100 Subject: [PATCH 06/15] Load configuration entry point from mutator instead of bundle.Load --- bundle/bundle.go | 7 +---- bundle/bundle_test.go | 4 +-- bundle/config/mutator/mutator.go | 5 +++- bundle/phases/load.go | 29 +++++++++++++++++++ bundle/tests/conflicting_resource_ids_test.go | 12 ++++---- bundle/tests/include_test.go | 4 +-- bundle/tests/loader.go | 3 +- bundle/tests/python_wheel_test.go | 12 +++----- cmd/root/bundle_test.go | 4 +++ libs/template/renderer_test.go | 15 +++++----- 10 files changed, 63 insertions(+), 32 deletions(-) create mode 100644 bundle/phases/load.go diff --git a/bundle/bundle.go b/bundle/bundle.go index 2e193bbf39..977ca2247c 100644 --- a/bundle/bundle.go +++ b/bundle/bundle.go @@ -74,12 +74,7 @@ func Load(ctx context.Context, path string) (*Bundle, error) { if err != nil { return nil, err } - log.Debugf(ctx, "Loading bundle configuration from: %s", configFile) - root, err := config.Load(configFile) - if err != nil { - return nil, err - } - b.Config = *root + log.Debugf(ctx, "Found bundle root at %s (file %s)", b.RootPath, configFile) return b, nil } diff --git a/bundle/bundle_test.go b/bundle/bundle_test.go index be716a40ac..908b446e24 100644 --- a/bundle/bundle_test.go +++ b/bundle/bundle_test.go @@ -20,8 +20,8 @@ func TestLoadNotExists(t *testing.T) { func TestLoadExists(t *testing.T) { b, err := Load(context.Background(), "./tests/basic") - require.Nil(t, err) - assert.Equal(t, "basic", b.Config.Bundle.Name) + assert.NoError(t, err) + assert.NotNil(t, b) } func TestBundleCacheDir(t *testing.T) { diff --git a/bundle/config/mutator/mutator.go b/bundle/config/mutator/mutator.go index 10842a93b9..99b7e9ac99 100644 --- a/bundle/config/mutator/mutator.go +++ b/bundle/config/mutator/mutator.go @@ -9,8 +9,11 @@ import ( func DefaultMutators() []bundle.Mutator { return []bundle.Mutator{ - scripts.Execute(config.ScriptPreInit), + loader.EntryPoint(), loader.ProcessRootIncludes(), + + // Execute preinit script after loading all configuration files. + scripts.Execute(config.ScriptPreInit), EnvironmentsToTargets(), InitializeVariables(), DefineDefaultTarget(), diff --git a/bundle/phases/load.go b/bundle/phases/load.go new file mode 100644 index 0000000000..fa06687754 --- /dev/null +++ b/bundle/phases/load.go @@ -0,0 +1,29 @@ +package phases + +import ( + "github.com/databricks/cli/bundle" + "github.com/databricks/cli/bundle/config/mutator" +) + +// The load phase loads configuration from disk and performs +// lightweight preprocessing (anything that can be done without network I/O). +func Load() bundle.Mutator { + return newPhase( + "load", + mutator.DefaultMutators(), + ) +} + +func LoadDefaultTarget() bundle.Mutator { + return newPhase( + "load", + append(mutator.DefaultMutators(), mutator.SelectDefaultTarget()), + ) +} + +func LoadNamedTarget(target string) bundle.Mutator { + return newPhase( + "load", + append(mutator.DefaultMutators(), mutator.SelectTarget(target)), + ) +} diff --git a/bundle/tests/conflicting_resource_ids_test.go b/bundle/tests/conflicting_resource_ids_test.go index 16dd1c33ab..e7f0aa28f2 100644 --- a/bundle/tests/conflicting_resource_ids_test.go +++ b/bundle/tests/conflicting_resource_ids_test.go @@ -7,23 +7,25 @@ import ( "testing" "github.com/databricks/cli/bundle" - "github.com/databricks/cli/bundle/config/mutator" + "github.com/databricks/cli/bundle/phases" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) func TestConflictingResourceIdsNoSubconfig(t *testing.T) { ctx := context.Background() - _, err := bundle.Load(ctx, "./conflicting_resource_ids/no_subconfigurations") + b, err := bundle.Load(ctx, "./conflicting_resource_ids/no_subconfigurations") + require.NoError(t, err) + diags := bundle.Apply(ctx, b, phases.Load()) bundleConfigPath := filepath.FromSlash("conflicting_resource_ids/no_subconfigurations/databricks.yml") - assert.ErrorContains(t, err, fmt.Sprintf("multiple resources named foo (job at %s, pipeline at %s)", bundleConfigPath, bundleConfigPath)) + assert.ErrorContains(t, diags.Error(), fmt.Sprintf("multiple resources named foo (job at %s, pipeline at %s)", bundleConfigPath, bundleConfigPath)) } func TestConflictingResourceIdsOneSubconfig(t *testing.T) { ctx := context.Background() b, err := bundle.Load(ctx, "./conflicting_resource_ids/one_subconfiguration") require.NoError(t, err) - diags := bundle.Apply(ctx, b, bundle.Seq(mutator.DefaultMutators()...)) + diags := bundle.Apply(ctx, b, phases.Load()) bundleConfigPath := filepath.FromSlash("conflicting_resource_ids/one_subconfiguration/databricks.yml") resourcesConfigPath := filepath.FromSlash("conflicting_resource_ids/one_subconfiguration/resources.yml") assert.ErrorContains(t, diags.Error(), fmt.Sprintf("multiple resources named foo (job at %s, pipeline at %s)", bundleConfigPath, resourcesConfigPath)) @@ -33,7 +35,7 @@ func TestConflictingResourceIdsTwoSubconfigs(t *testing.T) { ctx := context.Background() b, err := bundle.Load(ctx, "./conflicting_resource_ids/two_subconfigurations") require.NoError(t, err) - diags := bundle.Apply(ctx, b, bundle.Seq(mutator.DefaultMutators()...)) + diags := bundle.Apply(ctx, b, phases.Load()) resources1ConfigPath := filepath.FromSlash("conflicting_resource_ids/two_subconfigurations/resources1.yml") resources2ConfigPath := filepath.FromSlash("conflicting_resource_ids/two_subconfigurations/resources2.yml") assert.ErrorContains(t, diags.Error(), fmt.Sprintf("multiple resources named foo (job at %s, pipeline at %s)", resources1ConfigPath, resources2ConfigPath)) diff --git a/bundle/tests/include_test.go b/bundle/tests/include_test.go index fd8ae7198d..5b0235f605 100644 --- a/bundle/tests/include_test.go +++ b/bundle/tests/include_test.go @@ -7,7 +7,7 @@ import ( "testing" "github.com/databricks/cli/bundle" - "github.com/databricks/cli/bundle/config/mutator" + "github.com/databricks/cli/bundle/phases" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "golang.org/x/exp/maps" @@ -17,7 +17,7 @@ func TestIncludeInvalid(t *testing.T) { ctx := context.Background() b, err := bundle.Load(ctx, "./include_invalid") require.NoError(t, err) - diags := bundle.Apply(ctx, b, bundle.Seq(mutator.DefaultMutators()...)) + diags := bundle.Apply(ctx, b, phases.Load()) require.Error(t, diags.Error()) assert.ErrorContains(t, diags.Error(), "notexists.yml defined in 'include' section does not match any files") } diff --git a/bundle/tests/loader.go b/bundle/tests/loader.go index 228763ce92..ef9212cfbe 100644 --- a/bundle/tests/loader.go +++ b/bundle/tests/loader.go @@ -6,6 +6,7 @@ import ( "github.com/databricks/cli/bundle" "github.com/databricks/cli/bundle/config/mutator" + "github.com/databricks/cli/bundle/phases" "github.com/stretchr/testify/require" ) @@ -13,7 +14,7 @@ func load(t *testing.T, path string) *bundle.Bundle { ctx := context.Background() b, err := bundle.Load(ctx, path) require.NoError(t, err) - diags := bundle.Apply(ctx, b, bundle.Seq(mutator.DefaultMutators()...)) + diags := bundle.Apply(ctx, b, phases.Load()) require.NoError(t, diags.Error()) return b } diff --git a/bundle/tests/python_wheel_test.go b/bundle/tests/python_wheel_test.go index 412b507fe9..e2266516ae 100644 --- a/bundle/tests/python_wheel_test.go +++ b/bundle/tests/python_wheel_test.go @@ -16,8 +16,7 @@ func TestPythonWheelBuild(t *testing.T) { b, err := bundle.Load(ctx, "./python_wheel/python_wheel") require.NoError(t, err) - m := phases.Build() - diags := bundle.Apply(ctx, b, m) + diags := bundle.Apply(ctx, b, bundle.Seq(phases.Load(), phases.Build())) require.NoError(t, diags.Error()) matches, err := filepath.Glob("./python_wheel/python_wheel/my_test_code/dist/my_test_code-*.whl") @@ -34,8 +33,7 @@ func TestPythonWheelBuildAutoDetect(t *testing.T) { b, err := bundle.Load(ctx, "./python_wheel/python_wheel_no_artifact") require.NoError(t, err) - m := phases.Build() - diags := bundle.Apply(ctx, b, m) + diags := bundle.Apply(ctx, b, bundle.Seq(phases.Load(), phases.Build())) require.NoError(t, diags.Error()) matches, err := filepath.Glob("./python_wheel/python_wheel_no_artifact/dist/my_test_code-*.whl") @@ -52,8 +50,7 @@ func TestPythonWheelWithDBFSLib(t *testing.T) { b, err := bundle.Load(ctx, "./python_wheel/python_wheel_dbfs_lib") require.NoError(t, err) - m := phases.Build() - diags := bundle.Apply(ctx, b, m) + diags := bundle.Apply(ctx, b, bundle.Seq(phases.Load(), phases.Build())) require.NoError(t, diags.Error()) match := libraries.MatchWithArtifacts() @@ -66,8 +63,7 @@ func TestPythonWheelBuildNoBuildJustUpload(t *testing.T) { b, err := bundle.Load(ctx, "./python_wheel/python_wheel_no_artifact_no_setup") require.NoError(t, err) - m := phases.Build() - diags := bundle.Apply(ctx, b, m) + diags := bundle.Apply(ctx, b, bundle.Seq(phases.Load(), phases.Build())) require.NoError(t, diags.Error()) match := libraries.MatchWithArtifacts() diff --git a/cmd/root/bundle_test.go b/cmd/root/bundle_test.go index a3dec491d2..97412ff69f 100644 --- a/cmd/root/bundle_test.go +++ b/cmd/root/bundle_test.go @@ -40,8 +40,12 @@ func emptyCommand(t *testing.T) *cobra.Command { func setup(t *testing.T, cmd *cobra.Command, host string) *bundle.Bundle { setupDatabricksCfg(t) + rootPath := t.TempDir() + testutil.Touch(t, rootPath, "databricks.yml") + err := configureBundle(cmd, []string{"validate"}, func(_ context.Context) (*bundle.Bundle, error) { return &bundle.Bundle{ + RootPath: rootPath, Config: config.Root{ Bundle: config.Bundle{ Name: "test", diff --git a/libs/template/renderer_test.go b/libs/template/renderer_test.go index cad58a5326..a8678a5251 100644 --- a/libs/template/renderer_test.go +++ b/libs/template/renderer_test.go @@ -14,7 +14,6 @@ import ( "github.com/databricks/cli/bundle" bundleConfig "github.com/databricks/cli/bundle/config" - "github.com/databricks/cli/bundle/config/mutator" "github.com/databricks/cli/bundle/phases" "github.com/databricks/cli/cmd/root" "github.com/databricks/cli/libs/diag" @@ -66,23 +65,25 @@ func assertBuiltinTemplateValid(t *testing.T, template string, settings map[stri require.NoError(t, err) err = renderer.persistToDisk() require.NoError(t, err) + b, err := bundle.Load(ctx, filepath.Join(tempDir, "template", "my_project")) require.NoError(t, err) + diags := bundle.Apply(ctx, b, phases.LoadNamedTarget(target)) + require.NoError(t, diags.Error()) // Apply initialize / validation mutators bundle.ApplyFunc(ctx, b, func(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { b.Config.Workspace.CurrentUser = &bundleConfig.User{User: cachedUser} + b.Config.Bundle.Terraform = &bundleConfig.Terraform{ + ExecPath: "sh", + } return nil }) b.Tagging = tags.ForCloud(w.Config) b.WorkspaceClient() - b.Config.Bundle.Terraform = &bundleConfig.Terraform{ - ExecPath: "sh", - } - diags := bundle.Apply(ctx, b, bundle.Seq( - bundle.Seq(mutator.DefaultMutators()...), - mutator.SelectTarget(target), + + diags = bundle.Apply(ctx, b, bundle.Seq( phases.Initialize(), )) require.NoError(t, diags.Error()) From b79ae8fd7b3131befa0195deaf93928f643dcee9 Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Tue, 26 Mar 2024 20:53:39 +0100 Subject: [PATCH 07/15] Fold target selection into seq --- bundle/tests/loader.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/bundle/tests/loader.go b/bundle/tests/loader.go index ef9212cfbe..e7cf18f732 100644 --- a/bundle/tests/loader.go +++ b/bundle/tests/loader.go @@ -23,9 +23,8 @@ func loadTarget(t *testing.T, path, env string) *bundle.Bundle { ctx := context.Background() b, err := bundle.Load(ctx, path) require.NoError(t, err) - diags := bundle.Apply(ctx, b, bundle.Seq(mutator.DefaultMutatorsForTarget(env)...)) - require.NoError(t, diags.Error()) - diags = bundle.Apply(ctx, b, bundle.Seq( + diags := bundle.Apply(ctx, b, bundle.Seq( + phases.LoadNamedTarget(env), mutator.RewriteSyncPaths(), mutator.MergeJobClusters(), mutator.MergeJobTasks(), From 64111e50f19ea6178c8b9aa36cad6993a581ccfe Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Wed, 27 Mar 2024 11:05:26 +0100 Subject: [PATCH 08/15] Make bundle loaders return diagnostics This is incompatible with the function signature of Cobra's `PreRunE` functions, so this change updates any use of PreRunE to be inlined in the main run function. --- cmd/bundle/deploy.go | 14 +- cmd/bundle/deployment/bind.go | 15 +- cmd/bundle/deployment/unbind.go | 15 +- cmd/bundle/destroy.go | 14 +- cmd/bundle/generate.go | 8 +- cmd/bundle/generate/job.go | 13 +- cmd/bundle/generate/pipeline.go | 13 +- cmd/bundle/launch.go | 2 - cmd/bundle/run.go | 19 ++- cmd/bundle/summary.go | 21 ++- cmd/bundle/sync.go | 11 +- cmd/bundle/test.go | 3 - cmd/bundle/utils/utils.go | 29 ++-- cmd/bundle/validate.go | 15 +- cmd/labs/project/entrypoint.go | 7 +- cmd/root/auth.go | 26 +-- cmd/root/bundle.go | 157 +++++------------- cmd/root/bundle_test.go | 49 +++--- cmd/root/bundleflag/completion.go | 27 +++ cmd/root/bundleflag/completion_test.go | 39 +++++ cmd/root/bundleflag/flags.go | 49 ++++++ cmd/root/bundleflag/flags_test.go | 49 ++++++ cmd/root/bundleflag/testdata/empty/.gitkeep | 0 .../testdata/invalid/databricks.yml | 5 + .../bundleflag/testdata/valid/databricks.yml | 7 + cmd/root/profileflag/flag.go | 20 +++ cmd/root/root.go | 7 +- 27 files changed, 383 insertions(+), 251 deletions(-) create mode 100644 cmd/root/bundleflag/completion.go create mode 100644 cmd/root/bundleflag/completion_test.go create mode 100644 cmd/root/bundleflag/flags.go create mode 100644 cmd/root/bundleflag/flags_test.go create mode 100644 cmd/root/bundleflag/testdata/empty/.gitkeep create mode 100644 cmd/root/bundleflag/testdata/invalid/databricks.yml create mode 100644 cmd/root/bundleflag/testdata/valid/databricks.yml create mode 100644 cmd/root/profileflag/flag.go diff --git a/cmd/bundle/deploy.go b/cmd/bundle/deploy.go index 8b8cb9f2ec..919b15a723 100644 --- a/cmd/bundle/deploy.go +++ b/cmd/bundle/deploy.go @@ -13,10 +13,9 @@ import ( func newDeployCommand() *cobra.Command { cmd := &cobra.Command{ - Use: "deploy", - Short: "Deploy bundle", - Args: root.NoArgs, - PreRunE: utils.ConfigureBundleWithVariables, + Use: "deploy", + Short: "Deploy bundle", + Args: root.NoArgs, } var force bool @@ -30,7 +29,10 @@ func newDeployCommand() *cobra.Command { cmd.RunE = func(cmd *cobra.Command, args []string) error { ctx := cmd.Context() - b := bundle.Get(ctx) + b, diags := utils.ConfigureBundleWithVariables(cmd) + if err := diags.Error(); err != nil { + return diags.Error() + } bundle.ApplyFunc(ctx, b, func(context.Context, *bundle.Bundle) diag.Diagnostics { b.Config.Bundle.Force = force @@ -46,7 +48,7 @@ func newDeployCommand() *cobra.Command { return nil }) - diags := bundle.Apply(ctx, b, bundle.Seq( + diags = bundle.Apply(ctx, b, bundle.Seq( phases.Initialize(), phases.Build(), phases.Deploy(), diff --git a/cmd/bundle/deployment/bind.go b/cmd/bundle/deployment/bind.go index 11c560b12a..71f441d3db 100644 --- a/cmd/bundle/deployment/bind.go +++ b/cmd/bundle/deployment/bind.go @@ -16,10 +16,9 @@ import ( func newBindCommand() *cobra.Command { cmd := &cobra.Command{ - Use: "bind KEY RESOURCE_ID", - Short: "Bind bundle-defined resources to existing resources", - Args: root.ExactArgs(2), - PreRunE: utils.ConfigureBundleWithVariables, + Use: "bind KEY RESOURCE_ID", + Short: "Bind bundle-defined resources to existing resources", + Args: root.ExactArgs(2), } var autoApprove bool @@ -29,7 +28,11 @@ func newBindCommand() *cobra.Command { cmd.RunE = func(cmd *cobra.Command, args []string) error { ctx := cmd.Context() - b := bundle.Get(ctx) + b, diags := utils.ConfigureBundleWithVariables(cmd) + if err := diags.Error(); err != nil { + return diags.Error() + } + resource, err := b.Config.Resources.FindResourceByConfigKey(args[0]) if err != nil { return err @@ -50,7 +53,7 @@ func newBindCommand() *cobra.Command { return nil }) - diags := bundle.Apply(ctx, b, bundle.Seq( + diags = bundle.Apply(ctx, b, bundle.Seq( phases.Initialize(), phases.Bind(&terraform.BindOptions{ AutoApprove: autoApprove, diff --git a/cmd/bundle/deployment/unbind.go b/cmd/bundle/deployment/unbind.go index 76727877f8..9de5285a52 100644 --- a/cmd/bundle/deployment/unbind.go +++ b/cmd/bundle/deployment/unbind.go @@ -13,10 +13,9 @@ import ( func newUnbindCommand() *cobra.Command { cmd := &cobra.Command{ - Use: "unbind KEY", - Short: "Unbind bundle-defined resources from its managed remote resource", - Args: root.ExactArgs(1), - PreRunE: utils.ConfigureBundleWithVariables, + Use: "unbind KEY", + Short: "Unbind bundle-defined resources from its managed remote resource", + Args: root.ExactArgs(1), } var forceLock bool @@ -24,7 +23,11 @@ func newUnbindCommand() *cobra.Command { cmd.RunE = func(cmd *cobra.Command, args []string) error { ctx := cmd.Context() - b := bundle.Get(ctx) + b, diags := utils.ConfigureBundleWithVariables(cmd) + if err := diags.Error(); err != nil { + return diags.Error() + } + resource, err := b.Config.Resources.FindResourceByConfigKey(args[0]) if err != nil { return err @@ -35,7 +38,7 @@ func newUnbindCommand() *cobra.Command { return nil }) - diags := bundle.Apply(cmd.Context(), b, bundle.Seq( + diags = bundle.Apply(cmd.Context(), b, bundle.Seq( phases.Initialize(), phases.Unbind(resource.TerraformResourceName(), args[0]), )) diff --git a/cmd/bundle/destroy.go b/cmd/bundle/destroy.go index 38b717713d..cd7e630626 100644 --- a/cmd/bundle/destroy.go +++ b/cmd/bundle/destroy.go @@ -18,10 +18,9 @@ import ( func newDestroyCommand() *cobra.Command { cmd := &cobra.Command{ - Use: "destroy", - Short: "Destroy deployed bundle resources", - Args: root.NoArgs, - PreRunE: utils.ConfigureBundleWithVariables, + Use: "destroy", + Short: "Destroy deployed bundle resources", + Args: root.NoArgs, } var autoApprove bool @@ -31,7 +30,10 @@ func newDestroyCommand() *cobra.Command { cmd.RunE = func(cmd *cobra.Command, args []string) error { ctx := cmd.Context() - b := bundle.Get(ctx) + b, diags := utils.ConfigureBundleWithVariables(cmd) + if err := diags.Error(); err != nil { + return diags.Error() + } bundle.ApplyFunc(ctx, b, func(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { // If `--force-lock` is specified, force acquisition of the deployment lock. @@ -58,7 +60,7 @@ func newDestroyCommand() *cobra.Command { return fmt.Errorf("please specify --auto-approve since selected logging format is json") } - diags := bundle.Apply(ctx, b, bundle.Seq( + diags = bundle.Apply(ctx, b, bundle.Seq( phases.Initialize(), phases.Build(), phases.Destroy(), diff --git a/cmd/bundle/generate.go b/cmd/bundle/generate.go index 6c48b15866..1e3d56e430 100644 --- a/cmd/bundle/generate.go +++ b/cmd/bundle/generate.go @@ -2,7 +2,6 @@ package bundle import ( "github.com/databricks/cli/cmd/bundle/generate" - "github.com/databricks/cli/cmd/bundle/utils" "github.com/spf13/cobra" ) @@ -10,10 +9,9 @@ func newGenerateCommand() *cobra.Command { var key string cmd := &cobra.Command{ - Use: "generate", - Short: "Generate bundle configuration", - Long: "Generate bundle configuration", - PreRunE: utils.ConfigureBundleWithVariables, + Use: "generate", + Short: "Generate bundle configuration", + Long: "Generate bundle configuration", } cmd.AddCommand(generate.NewGenerateJobCommand()) diff --git a/cmd/bundle/generate/job.go b/cmd/bundle/generate/job.go index c5a94a8f6f..99bc616604 100644 --- a/cmd/bundle/generate/job.go +++ b/cmd/bundle/generate/job.go @@ -5,7 +5,6 @@ import ( "os" "path/filepath" - "github.com/databricks/cli/bundle" "github.com/databricks/cli/bundle/config/generate" "github.com/databricks/cli/cmd/root" "github.com/databricks/cli/libs/cmdio" @@ -24,9 +23,8 @@ func NewGenerateJobCommand() *cobra.Command { var force bool cmd := &cobra.Command{ - Use: "job", - Short: "Generate bundle configuration for a job", - PreRunE: root.MustConfigureBundle, + Use: "job", + Short: "Generate bundle configuration for a job", } cmd.Flags().Int64Var(&jobId, "existing-job-id", 0, `Job ID of the job to generate config for`) @@ -43,9 +41,12 @@ func NewGenerateJobCommand() *cobra.Command { cmd.RunE = func(cmd *cobra.Command, args []string) error { ctx := cmd.Context() - b := bundle.Get(ctx) - w := b.WorkspaceClient() + b, diags := root.MustConfigureBundle(cmd) + if err := diags.Error(); err != nil { + return diags.Error() + } + w := b.WorkspaceClient() job, err := w.Jobs.Get(ctx, jobs.GetJobRequest{JobId: jobId}) if err != nil { return err diff --git a/cmd/bundle/generate/pipeline.go b/cmd/bundle/generate/pipeline.go index 4c5fcf4255..bd973fe0b3 100644 --- a/cmd/bundle/generate/pipeline.go +++ b/cmd/bundle/generate/pipeline.go @@ -5,7 +5,6 @@ import ( "os" "path/filepath" - "github.com/databricks/cli/bundle" "github.com/databricks/cli/bundle/config/generate" "github.com/databricks/cli/cmd/root" "github.com/databricks/cli/libs/cmdio" @@ -24,9 +23,8 @@ func NewGeneratePipelineCommand() *cobra.Command { var force bool cmd := &cobra.Command{ - Use: "pipeline", - Short: "Generate bundle configuration for a pipeline", - PreRunE: root.MustConfigureBundle, + Use: "pipeline", + Short: "Generate bundle configuration for a pipeline", } cmd.Flags().StringVar(&pipelineId, "existing-pipeline-id", "", `ID of the pipeline to generate config for`) @@ -43,9 +41,12 @@ func NewGeneratePipelineCommand() *cobra.Command { cmd.RunE = func(cmd *cobra.Command, args []string) error { ctx := cmd.Context() - b := bundle.Get(ctx) - w := b.WorkspaceClient() + b, diags := root.MustConfigureBundle(cmd) + if err := diags.Error(); err != nil { + return diags.Error() + } + w := b.WorkspaceClient() pipeline, err := w.Pipelines.Get(ctx, pipelines.GetPipelineRequest{PipelineId: pipelineId}) if err != nil { return err diff --git a/cmd/bundle/launch.go b/cmd/bundle/launch.go index f376ebdae6..0d2b4233b3 100644 --- a/cmd/bundle/launch.go +++ b/cmd/bundle/launch.go @@ -16,8 +16,6 @@ func newLaunchCommand() *cobra.Command { // We're not ready to expose this command until we specify its semantics. Hidden: true, - - PreRunE: root.MustConfigureBundle, } cmd.RunE = func(cmd *cobra.Command, args []string) error { diff --git a/cmd/bundle/run.go b/cmd/bundle/run.go index 87ea8610cc..e6a8e1ba4a 100644 --- a/cmd/bundle/run.go +++ b/cmd/bundle/run.go @@ -17,10 +17,9 @@ import ( func newRunCommand() *cobra.Command { cmd := &cobra.Command{ - Use: "run [flags] KEY", - Short: "Run a resource (e.g. a job or a pipeline)", - Args: root.MaximumNArgs(1), - PreRunE: utils.ConfigureBundleWithVariables, + Use: "run [flags] KEY", + Short: "Run a resource (e.g. a job or a pipeline)", + Args: root.MaximumNArgs(1), } var runOptions run.Options @@ -33,9 +32,12 @@ func newRunCommand() *cobra.Command { cmd.RunE = func(cmd *cobra.Command, args []string) error { ctx := cmd.Context() - b := bundle.Get(ctx) + b, diags := utils.ConfigureBundleWithVariables(cmd) + if err := diags.Error(); err != nil { + return diags.Error() + } - diags := bundle.Apply(ctx, b, bundle.Seq( + diags = bundle.Apply(ctx, b, bundle.Seq( phases.Initialize(), terraform.Interpolate(), terraform.Write(), @@ -109,15 +111,14 @@ func newRunCommand() *cobra.Command { return nil, cobra.ShellCompDirectiveNoFileComp } - err := root.MustConfigureBundle(cmd, args) - if err != nil { + b, diags := root.MustConfigureBundle(cmd) + if err := diags.Error(); err != nil { cobra.CompErrorln(err.Error()) return nil, cobra.ShellCompDirectiveError } // No completion in the context of a bundle. // Source and destination paths are taken from bundle configuration. - b := bundle.GetOrNil(cmd.Context()) if b == nil { return nil, cobra.ShellCompDirectiveNoFileComp } diff --git a/cmd/bundle/summary.go b/cmd/bundle/summary.go index a28ceede97..5a64b46c0a 100644 --- a/cmd/bundle/summary.go +++ b/cmd/bundle/summary.go @@ -18,10 +18,9 @@ import ( func newSummaryCommand() *cobra.Command { cmd := &cobra.Command{ - Use: "summary", - Short: "Describe the bundle resources and their deployment states", - Args: root.NoArgs, - PreRunE: utils.ConfigureBundleWithVariables, + Use: "summary", + Short: "Describe the bundle resources and their deployment states", + Args: root.NoArgs, // This command is currently intended for the Databricks VSCode extension only Hidden: true, @@ -31,14 +30,18 @@ func newSummaryCommand() *cobra.Command { cmd.Flags().BoolVar(&forcePull, "force-pull", false, "Skip local cache and load the state from the remote workspace") cmd.RunE = func(cmd *cobra.Command, args []string) error { - b := bundle.Get(cmd.Context()) + ctx := cmd.Context() + b, diags := utils.ConfigureBundleWithVariables(cmd) + if err := diags.Error(); err != nil { + return diags.Error() + } - diags := bundle.Apply(cmd.Context(), b, phases.Initialize()) + diags = bundle.Apply(ctx, b, phases.Initialize()) if err := diags.Error(); err != nil { return err } - cacheDir, err := terraform.Dir(cmd.Context(), b) + cacheDir, err := terraform.Dir(ctx, b) if err != nil { return err } @@ -47,7 +50,7 @@ func newSummaryCommand() *cobra.Command { noCache := errors.Is(stateFileErr, os.ErrNotExist) || errors.Is(configFileErr, os.ErrNotExist) if forcePull || noCache { - diags = bundle.Apply(cmd.Context(), b, bundle.Seq( + diags = bundle.Apply(ctx, b, bundle.Seq( terraform.StatePull(), terraform.Interpolate(), terraform.Write(), @@ -57,7 +60,7 @@ func newSummaryCommand() *cobra.Command { } } - diags = bundle.Apply(cmd.Context(), b, terraform.Load()) + diags = bundle.Apply(ctx, b, terraform.Load()) if err := diags.Error(); err != nil { return err } diff --git a/cmd/bundle/sync.go b/cmd/bundle/sync.go index 0b7f9b3a90..0818aecf73 100644 --- a/cmd/bundle/sync.go +++ b/cmd/bundle/sync.go @@ -36,8 +36,6 @@ func newSyncCommand() *cobra.Command { Use: "sync [flags]", Short: "Synchronize bundle tree to the workspace", Args: root.NoArgs, - - PreRunE: utils.ConfigureBundleWithVariables, } var f syncFlags @@ -46,10 +44,14 @@ func newSyncCommand() *cobra.Command { cmd.Flags().BoolVar(&f.watch, "watch", false, "watch local file system for changes") cmd.RunE = func(cmd *cobra.Command, args []string) error { - b := bundle.Get(cmd.Context()) + ctx := cmd.Context() + b, diags := utils.ConfigureBundleWithVariables(cmd) + if err := diags.Error(); err != nil { + return diags.Error() + } // Run initialize phase to make sure paths are set. - diags := bundle.Apply(cmd.Context(), b, phases.Initialize()) + diags = bundle.Apply(ctx, b, phases.Initialize()) if err := diags.Error(); err != nil { return err } @@ -59,7 +61,6 @@ func newSyncCommand() *cobra.Command { return err } - ctx := cmd.Context() s, err := sync.New(ctx, *opts) if err != nil { return err diff --git a/cmd/bundle/test.go b/cmd/bundle/test.go index ea1a4b716e..4d30e727d5 100644 --- a/cmd/bundle/test.go +++ b/cmd/bundle/test.go @@ -3,7 +3,6 @@ package bundle import ( "fmt" - "github.com/databricks/cli/cmd/root" "github.com/spf13/cobra" ) @@ -15,8 +14,6 @@ func newTestCommand() *cobra.Command { // We're not ready to expose this command until we specify its semantics. Hidden: true, - - PreRunE: root.MustConfigureBundle, } cmd.RunE = func(cmd *cobra.Command, args []string) error { diff --git a/cmd/bundle/utils/utils.go b/cmd/bundle/utils/utils.go index e53a40b9d6..d585c62204 100644 --- a/cmd/bundle/utils/utils.go +++ b/cmd/bundle/utils/utils.go @@ -9,23 +9,30 @@ import ( "github.com/spf13/cobra" ) -func ConfigureBundleWithVariables(cmd *cobra.Command, args []string) error { +func configureVariables(cmd *cobra.Command, b *bundle.Bundle, variables []string) diag.Diagnostics { + return bundle.ApplyFunc(cmd.Context(), b, func(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { + err := b.Config.InitializeVariables(variables) + return diag.FromErr(err) + }) +} + +func ConfigureBundleWithVariables(cmd *cobra.Command) (*bundle.Bundle, diag.Diagnostics) { // Load bundle config and apply target - err := root.MustConfigureBundle(cmd, args) - if err != nil { - return err + b, diags := root.MustConfigureBundle(cmd) + if diags.HasError() { + return nil, diags } variables, err := cmd.Flags().GetStringSlice("var") if err != nil { - return err + return nil, diag.FromErr(err) } // Initialize variables by assigning them values passed as command line flags - b := bundle.Get(cmd.Context()) - diags := bundle.ApplyFunc(cmd.Context(), b, func(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { - err := b.Config.InitializeVariables(variables) - return diag.FromErr(err) - }) - return diags.Error() + diags = diags.Extend(configureVariables(cmd, b, variables)) + if diags.HasError() { + return nil, diags + } + + return b, diags } diff --git a/cmd/bundle/validate.go b/cmd/bundle/validate.go index 42686b3284..57bf6f7b9b 100644 --- a/cmd/bundle/validate.go +++ b/cmd/bundle/validate.go @@ -13,16 +13,19 @@ import ( func newValidateCommand() *cobra.Command { cmd := &cobra.Command{ - Use: "validate", - Short: "Validate configuration", - Args: root.NoArgs, - PreRunE: utils.ConfigureBundleWithVariables, + Use: "validate", + Short: "Validate configuration", + Args: root.NoArgs, } cmd.RunE = func(cmd *cobra.Command, args []string) error { - b := bundle.Get(cmd.Context()) + ctx := cmd.Context() + b, diags := utils.ConfigureBundleWithVariables(cmd) + if err := diags.Error(); err != nil { + return diags.Error() + } - diags := bundle.Apply(cmd.Context(), b, phases.Initialize()) + diags = bundle.Apply(ctx, b, phases.Initialize()) if err := diags.Error(); err != nil { return err } diff --git a/cmd/labs/project/entrypoint.go b/cmd/labs/project/entrypoint.go index 96f46d4b5c..99edf83c8c 100644 --- a/cmd/labs/project/entrypoint.go +++ b/cmd/labs/project/entrypoint.go @@ -10,7 +10,6 @@ import ( "path/filepath" "strings" - "github.com/databricks/cli/bundle" "github.com/databricks/cli/cmd/root" "github.com/databricks/cli/internal/build" "github.com/databricks/cli/libs/cmdio" @@ -203,11 +202,11 @@ func (e *Entrypoint) getLoginConfig(cmd *cobra.Command) (*loginConfig, *config.C return lc, cfg, nil } if e.IsBundleAware { - err = root.TryConfigureBundle(cmd, []string{}) - if err != nil { + b, diags := root.TryConfigureBundle(cmd) + if err := diags.Error(); err != nil { return nil, nil, fmt.Errorf("bundle: %w", err) } - if b := bundle.GetOrNil(cmd.Context()); b != nil { + if b != nil { log.Infof(ctx, "Using login configuration from Databricks Asset Bundle") return &loginConfig{}, b.WorkspaceClient().Config, nil } diff --git a/cmd/root/auth.go b/cmd/root/auth.go index 89c7641c54..9188bce657 100644 --- a/cmd/root/auth.go +++ b/cmd/root/auth.go @@ -6,7 +6,7 @@ import ( "fmt" "net/http" - "github.com/databricks/cli/bundle" + "github.com/databricks/cli/cmd/root/profileflag" "github.com/databricks/cli/libs/cmdio" "github.com/databricks/cli/libs/databrickscfg" "github.com/databricks/databricks-sdk-go" @@ -19,20 +19,6 @@ import ( var workspaceClient int var accountClient int -func initProfileFlag(cmd *cobra.Command) { - cmd.PersistentFlags().StringP("profile", "p", "", "~/.databrickscfg profile") - cmd.RegisterFlagCompletionFunc("profile", databrickscfg.ProfileCompletion) -} - -func profileFlagValue(cmd *cobra.Command) (string, bool) { - profileFlag := cmd.Flag("profile") - if profileFlag == nil { - return "", false - } - value := profileFlag.Value.String() - return value, value != "" -} - // Helper function to create an account client or prompt once if the given configuration is not valid. func accountClientOrPrompt(ctx context.Context, cfg *config.Config, allowPrompt bool) (*databricks.AccountClient, error) { a, err := databricks.NewAccountClient((*databricks.Config)(cfg)) @@ -72,7 +58,7 @@ func MustAccountClient(cmd *cobra.Command, args []string) error { cfg := &config.Config{} // The command-line profile flag takes precedence over DATABRICKS_CONFIG_PROFILE. - profile, hasProfileFlag := profileFlagValue(cmd) + profile, hasProfileFlag := profileflag.Value(cmd) if hasProfileFlag { cfg.Profile = profile } @@ -142,18 +128,18 @@ func MustWorkspaceClient(cmd *cobra.Command, args []string) error { cfg := &config.Config{} // The command-line profile flag takes precedence over DATABRICKS_CONFIG_PROFILE. - profile, hasProfileFlag := profileFlagValue(cmd) + profile, hasProfileFlag := profileflag.Value(cmd) if hasProfileFlag { cfg.Profile = profile } // Try to load a bundle configuration if we're allowed to by the caller (see `./auth_options.go`). if !shouldSkipLoadBundle(cmd.Context()) { - err := TryConfigureBundle(cmd, args) - if err != nil { + b, diags := TryConfigureBundle(cmd) + if err := diags.Error(); err != nil { return err } - if b := bundle.GetOrNil(cmd.Context()); b != nil { + if b != nil { client, err := b.InitializeWorkspaceClient() if err != nil { return err diff --git a/cmd/root/bundle.go b/cmd/root/bundle.go index 6a6aeb4d2f..24b123ac5b 100644 --- a/cmd/root/bundle.go +++ b/cmd/root/bundle.go @@ -4,146 +4,79 @@ import ( "context" "github.com/databricks/cli/bundle" - "github.com/databricks/cli/bundle/config/mutator" - "github.com/databricks/cli/bundle/env" + "github.com/databricks/cli/bundle/phases" + "github.com/databricks/cli/cmd/root/bundleflag" + "github.com/databricks/cli/cmd/root/profileflag" "github.com/databricks/cli/libs/diag" - envlib "github.com/databricks/cli/libs/env" + "github.com/databricks/cli/libs/env" "github.com/spf13/cobra" - "golang.org/x/exp/maps" ) -// getTarget returns the name of the target to operate in. -func getTarget(cmd *cobra.Command) (value string) { - // The command line flag takes precedence. - flag := cmd.Flag("target") - if flag != nil { - value = flag.Value.String() - if value != "" { - return - } +// configureProfile applies the profile flag to the bundle. +func configureProfile(cmd *cobra.Command, b *bundle.Bundle) diag.Diagnostics { + profile, ok := profileflag.Value(cmd) + if !ok || profile == "" { + // If it's not set, use the environment variable. + profile, ok = env.Lookup(cmd.Context(), "DATABRICKS_CONFIG_PROFILE") } - oldFlag := cmd.Flag("environment") - if oldFlag != nil { - value = oldFlag.Value.String() - if value != "" { - return - } + if !ok || profile == "" { + return nil } - // If it's not set, use the environment variable. - target, _ := env.Target(cmd.Context()) - return target + return bundle.ApplyFunc(cmd.Context(), b, func(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { + b.Config.Workspace.Profile = profile + return nil + }) } -func getProfile(cmd *cobra.Command) (value string) { - // The command line flag takes precedence. - flag := cmd.Flag("profile") - if flag != nil { - value = flag.Value.String() - if value != "" { - return - } +// configureBundle applies basic mutators to the bundle configures it on the command's context. +func configureBundle(cmd *cobra.Command, b *bundle.Bundle) (*bundle.Bundle, diag.Diagnostics) { + var m bundle.Mutator + if target := bundleflag.Target(cmd); target == "" { + m = phases.LoadDefaultTarget() + } else { + m = phases.LoadNamedTarget(target) } - // If it's not set, use the environment variable. - return envlib.Get(cmd.Context(), "DATABRICKS_CONFIG_PROFILE") -} - -// loadBundle loads the bundle configuration and applies default mutators. -func loadBundle(cmd *cobra.Command, args []string, load func(ctx context.Context) (*bundle.Bundle, error)) (*bundle.Bundle, error) { + // Load bundle and select target. ctx := cmd.Context() - b, err := load(ctx) - if err != nil { - return nil, err - } - - // No bundle is fine in case of `TryConfigureBundle`. - if b == nil { - return nil, nil - } - - profile := getProfile(cmd) - if profile != "" { - diags := bundle.ApplyFunc(ctx, b, func(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { - b.Config.Workspace.Profile = profile - return nil - }) - if err := diags.Error(); err != nil { - return nil, err - } + diags := bundle.Apply(ctx, b, m) + if diags.HasError() { + return nil, diags } - diags := bundle.Apply(ctx, b, bundle.Seq(mutator.DefaultMutators()...)) - if err := diags.Error(); err != nil { - return nil, err + // Configure the workspace profile if the flag has been set. + diags = diags.Extend(configureProfile(cmd, b)) + if diags.HasError() { + return nil, diags } - return b, nil + return b, diags } -// configureBundle loads the bundle configuration and configures it on the command's context. -func configureBundle(cmd *cobra.Command, args []string, load func(ctx context.Context) (*bundle.Bundle, error)) error { - b, err := loadBundle(cmd, args, load) +// MustConfigureBundle configures a bundle on the command context. +func MustConfigureBundle(cmd *cobra.Command) (*bundle.Bundle, diag.Diagnostics) { + b, err := bundle.MustLoad(cmd.Context()) if err != nil { - return err - } - - // No bundle is fine in case of `TryConfigureBundle`. - if b == nil { - return nil + return nil, diag.FromErr(err) } - var m bundle.Mutator - env := getTarget(cmd) - if env == "" { - m = mutator.SelectDefaultTarget() - } else { - m = mutator.SelectTarget(env) - } - - ctx := cmd.Context() - diags := bundle.Apply(ctx, b, m) - if err := diags.Error(); err != nil { - return err - } - - cmd.SetContext(bundle.Context(ctx, b)) - return nil -} - -// MustConfigureBundle configures a bundle on the command context. -func MustConfigureBundle(cmd *cobra.Command, args []string) error { - return configureBundle(cmd, args, bundle.MustLoad) + return configureBundle(cmd, b) } // TryConfigureBundle configures a bundle on the command context // if there is one, but doesn't fail if there isn't one. -func TryConfigureBundle(cmd *cobra.Command, args []string) error { - return configureBundle(cmd, args, bundle.TryLoad) -} - -// targetCompletion executes to autocomplete the argument to the target flag. -func targetCompletion(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) { - b, err := loadBundle(cmd, args, bundle.MustLoad) +func TryConfigureBundle(cmd *cobra.Command) (*bundle.Bundle, diag.Diagnostics) { + b, err := bundle.TryLoad(cmd.Context()) if err != nil { - cobra.CompErrorln(err.Error()) - return nil, cobra.ShellCompDirectiveError + return nil, diag.FromErr(err) } - return maps.Keys(b.Config.Targets), cobra.ShellCompDirectiveDefault -} - -func initTargetFlag(cmd *cobra.Command) { - // To operate in the context of a bundle, all commands must take an "target" parameter. - cmd.PersistentFlags().StringP("target", "t", "", "bundle target to use (if applicable)") - cmd.RegisterFlagCompletionFunc("target", targetCompletion) -} + // No bundle is fine in this case. + if b == nil { + return nil, nil + } -// DEPRECATED flag -func initEnvironmentFlag(cmd *cobra.Command) { - // To operate in the context of a bundle, all commands must take an "environment" parameter. - cmd.PersistentFlags().StringP("environment", "e", "", "bundle target to use (if applicable)") - cmd.PersistentFlags().MarkDeprecated("environment", "use --target flag instead") - cmd.RegisterFlagCompletionFunc("environment", targetCompletion) + return configureBundle(cmd, b) } diff --git a/cmd/root/bundle_test.go b/cmd/root/bundle_test.go index 97412ff69f..452b5ec42c 100644 --- a/cmd/root/bundle_test.go +++ b/cmd/root/bundle_test.go @@ -2,16 +2,19 @@ package root import ( "context" + "fmt" "os" "path/filepath" "runtime" "testing" "github.com/databricks/cli/bundle" - "github.com/databricks/cli/bundle/config" + "github.com/databricks/cli/cmd/root/bundleflag" + "github.com/databricks/cli/cmd/root/profileflag" "github.com/databricks/cli/internal/testutil" "github.com/spf13/cobra" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func setupDatabricksCfg(t *testing.T) { @@ -33,7 +36,7 @@ func emptyCommand(t *testing.T) *cobra.Command { ctx := context.Background() cmd := &cobra.Command{} cmd.SetContext(ctx) - initProfileFlag(cmd) + profileflag.Init(cmd) return cmd } @@ -41,23 +44,18 @@ func setup(t *testing.T, cmd *cobra.Command, host string) *bundle.Bundle { setupDatabricksCfg(t) rootPath := t.TempDir() - testutil.Touch(t, rootPath, "databricks.yml") - - err := configureBundle(cmd, []string{"validate"}, func(_ context.Context) (*bundle.Bundle, error) { - return &bundle.Bundle{ - RootPath: rootPath, - Config: config.Root{ - Bundle: config.Bundle{ - Name: "test", - }, - Workspace: config.Workspace{ - Host: host, - }, - }, - }, nil - }) - assert.NoError(t, err) - return bundle.Get(cmd.Context()) + testutil.Chdir(t, rootPath) + + contents := fmt.Sprintf(` +workspace: + host: %q +`, host) + err := os.WriteFile(filepath.Join(rootPath, "databricks.yml"), []byte(contents), 0644) + require.NoError(t, err) + + b, diags := MustConfigureBundle(cmd) + require.NoError(t, diags.Error()) + return b } func TestBundleConfigureDefault(t *testing.T) { @@ -142,38 +140,37 @@ func TestBundleConfigureWithProfileFlagAndEnvVariable(t *testing.T) { func TestTargetFlagFull(t *testing.T) { cmd := emptyCommand(t) - initTargetFlag(cmd) + bundleflag.Init(cmd) cmd.SetArgs([]string{"version", "--target", "development"}) ctx := context.Background() err := cmd.ExecuteContext(ctx) assert.NoError(t, err) - assert.Equal(t, getTarget(cmd), "development") + assert.Equal(t, "development", bundleflag.Target(cmd)) } func TestTargetFlagShort(t *testing.T) { cmd := emptyCommand(t) - initTargetFlag(cmd) + bundleflag.Init(cmd) cmd.SetArgs([]string{"version", "-t", "production"}) ctx := context.Background() err := cmd.ExecuteContext(ctx) assert.NoError(t, err) - assert.Equal(t, getTarget(cmd), "production") + assert.Equal(t, "production", bundleflag.Target(cmd)) } // TODO: remove when environment flag is fully deprecated func TestTargetEnvironmentFlag(t *testing.T) { cmd := emptyCommand(t) - initTargetFlag(cmd) - initEnvironmentFlag(cmd) + bundleflag.Init(cmd) cmd.SetArgs([]string{"version", "--environment", "development"}) ctx := context.Background() err := cmd.ExecuteContext(ctx) assert.NoError(t, err) - assert.Equal(t, getTarget(cmd), "development") + assert.Equal(t, "development", bundleflag.Target(cmd)) } diff --git a/cmd/root/bundleflag/completion.go b/cmd/root/bundleflag/completion.go new file mode 100644 index 0000000000..5fa264d364 --- /dev/null +++ b/cmd/root/bundleflag/completion.go @@ -0,0 +1,27 @@ +package bundleflag + +import ( + "github.com/databricks/cli/bundle" + "github.com/databricks/cli/bundle/phases" + "github.com/spf13/cobra" + "golang.org/x/exp/maps" +) + +// targetCompletion executes to autocomplete the argument to the target flag. +func targetCompletion(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) { + ctx := cmd.Context() + b, err := bundle.MustLoad(ctx) + if err != nil { + cobra.CompErrorln(err.Error()) + return nil, cobra.ShellCompDirectiveError + } + + // Load bundle but don't select a target (we're completing those). + diags := bundle.Apply(ctx, b, phases.Load()) + if err := diags.Error(); err != nil { + cobra.CompErrorln(err.Error()) + return nil, cobra.ShellCompDirectiveError + } + + return maps.Keys(b.Config.Targets), cobra.ShellCompDirectiveDefault +} diff --git a/cmd/root/bundleflag/completion_test.go b/cmd/root/bundleflag/completion_test.go new file mode 100644 index 0000000000..e5ab393cb0 --- /dev/null +++ b/cmd/root/bundleflag/completion_test.go @@ -0,0 +1,39 @@ +package bundleflag + +import ( + "context" + "testing" + + "github.com/databricks/cli/internal/testutil" + "github.com/spf13/cobra" + "github.com/stretchr/testify/assert" +) + +func prepareCompletionTest(t *testing.T, dir string) *cobra.Command { + testutil.CleanupEnvironment(t) + testutil.Chdir(t, dir) + cmd := cobra.Command{} + cmd.SetContext(context.Background()) + return &cmd +} + +func TestCompletionEmpty(t *testing.T) { + cmd := prepareCompletionTest(t, "./testdata/empty") + out, comp := targetCompletion(cmd, []string{}, "") + assert.Equal(t, cobra.ShellCompDirectiveError, comp) + assert.Empty(t, out) +} + +func TestCompletionInvalid(t *testing.T) { + cmd := prepareCompletionTest(t, "./testdata/invalid") + out, comp := targetCompletion(cmd, []string{}, "") + assert.Equal(t, cobra.ShellCompDirectiveError, comp) + assert.Empty(t, out) +} + +func TestCompletionValid(t *testing.T) { + cmd := prepareCompletionTest(t, "./testdata/valid") + out, comp := targetCompletion(cmd, []string{}, "") + assert.Equal(t, cobra.ShellCompDirectiveDefault, comp) + assert.ElementsMatch(t, []string{"foo", "bar", "qux"}, out) +} diff --git a/cmd/root/bundleflag/flags.go b/cmd/root/bundleflag/flags.go new file mode 100644 index 0000000000..b867fb726a --- /dev/null +++ b/cmd/root/bundleflag/flags.go @@ -0,0 +1,49 @@ +package bundleflag + +import ( + "github.com/databricks/cli/bundle/env" + "github.com/spf13/cobra" +) + +// Target returns the name of the target to operate in. +func Target(cmd *cobra.Command) (value string) { + // The command line flag takes precedence. + flag := cmd.Flag("target") + if flag != nil { + value = flag.Value.String() + if value != "" { + return + } + } + + oldFlag := cmd.Flag("environment") + if oldFlag != nil { + value = oldFlag.Value.String() + if value != "" { + return + } + } + + // If it's not set, use the environment variable. + target, _ := env.Target(cmd.Context()) + return target +} + +func Init(cmd *cobra.Command) { + initTargetFlag(cmd) + initEnvironmentFlag(cmd) +} + +func initTargetFlag(cmd *cobra.Command) { + // To operate in the context of a bundle, all commands must take an "target" parameter. + cmd.PersistentFlags().StringP("target", "t", "", "bundle target to use (if applicable)") + cmd.RegisterFlagCompletionFunc("target", targetCompletion) +} + +// DEPRECATED flag +func initEnvironmentFlag(cmd *cobra.Command) { + // To operate in the context of a bundle, all commands must take an "environment" parameter. + cmd.PersistentFlags().StringP("environment", "e", "", "bundle target to use (if applicable)") + cmd.PersistentFlags().MarkDeprecated("environment", "use --target flag instead") + cmd.RegisterFlagCompletionFunc("environment", targetCompletion) +} diff --git a/cmd/root/bundleflag/flags_test.go b/cmd/root/bundleflag/flags_test.go new file mode 100644 index 0000000000..cc34749538 --- /dev/null +++ b/cmd/root/bundleflag/flags_test.go @@ -0,0 +1,49 @@ +package bundleflag + +import ( + "context" + "testing" + + benv "github.com/databricks/cli/bundle/env" + "github.com/databricks/cli/libs/env" + "github.com/spf13/cobra" + "github.com/stretchr/testify/require" +) + +func prepareFlagTest() *cobra.Command { + cmd := &cobra.Command{} + cmd.SetContext(context.Background()) + Init(cmd) + return cmd +} + +func TestTargetValue(t *testing.T) { + cmd := prepareFlagTest() + cmd.RunE = func(cmd *cobra.Command, args []string) error { + target := Target(cmd) + if target != "test" { + t.Errorf("expected 'test', got %q", target) + } + return nil + } + + t.Run("target flag", func(t *testing.T) { + cmd.SetArgs([]string{"--target", "test"}) + err := cmd.Execute() + require.NoError(t, err) + }) + + t.Run("environment flag", func(t *testing.T) { + cmd.SetArgs([]string{"--environment", "test"}) + err := cmd.Execute() + require.NoError(t, err) + }) + + t.Run("environment variable", func(t *testing.T) { + ctx := env.Set(cmd.Context(), benv.TargetVariable, "test") + cmd.SetContext(ctx) + cmd.SetArgs([]string{}) + err := cmd.Execute() + require.NoError(t, err) + }) +} diff --git a/cmd/root/bundleflag/testdata/empty/.gitkeep b/cmd/root/bundleflag/testdata/empty/.gitkeep new file mode 100644 index 0000000000..e69de29bb2 diff --git a/cmd/root/bundleflag/testdata/invalid/databricks.yml b/cmd/root/bundleflag/testdata/invalid/databricks.yml new file mode 100644 index 0000000000..964e2dc1a7 --- /dev/null +++ b/cmd/root/bundleflag/testdata/invalid/databricks.yml @@ -0,0 +1,5 @@ +bundle: + name: invalid + +include: + - ./i_dont_exist.yml diff --git a/cmd/root/bundleflag/testdata/valid/databricks.yml b/cmd/root/bundleflag/testdata/valid/databricks.yml new file mode 100644 index 0000000000..d893895746 --- /dev/null +++ b/cmd/root/bundleflag/testdata/valid/databricks.yml @@ -0,0 +1,7 @@ +bundle: + name: valid + +targets: + foo: ~ + bar: ~ + qux: ~ diff --git a/cmd/root/profileflag/flag.go b/cmd/root/profileflag/flag.go new file mode 100644 index 0000000000..5110458d64 --- /dev/null +++ b/cmd/root/profileflag/flag.go @@ -0,0 +1,20 @@ +package profileflag + +import ( + "github.com/databricks/cli/libs/databrickscfg" + "github.com/spf13/cobra" +) + +func Init(cmd *cobra.Command) { + cmd.PersistentFlags().StringP("profile", "p", "", "~/.databrickscfg profile") + cmd.RegisterFlagCompletionFunc("profile", databrickscfg.ProfileCompletion) +} + +func Value(cmd *cobra.Command) (string, bool) { + profileFlag := cmd.Flag("profile") + if profileFlag == nil { + return "", false + } + value := profileFlag.Value.String() + return value, value != "" +} diff --git a/cmd/root/root.go b/cmd/root/root.go index 38eb42ccb0..ffb10bc586 100644 --- a/cmd/root/root.go +++ b/cmd/root/root.go @@ -8,6 +8,8 @@ import ( "log/slog" + "github.com/databricks/cli/cmd/root/bundleflag" + "github.com/databricks/cli/cmd/root/profileflag" "github.com/databricks/cli/internal/build" "github.com/databricks/cli/libs/cmdio" "github.com/databricks/cli/libs/log" @@ -38,9 +40,8 @@ func New(ctx context.Context) *cobra.Command { logFlags := initLogFlags(cmd) progressLoggerFlag := initProgressLoggerFlag(cmd, logFlags) outputFlag := initOutputFlag(cmd) - initProfileFlag(cmd) - initEnvironmentFlag(cmd) - initTargetFlag(cmd) + profileflag.Init(cmd) + bundleflag.Init(cmd) cmd.PersistentPreRunE = func(cmd *cobra.Command, args []string) error { ctx := cmd.Context() From d1dd71bfaa547562897547bab4ac04b4bfd29ea3 Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Wed, 27 Mar 2024 11:47:38 +0100 Subject: [PATCH 09/15] Revert profile move --- cmd/root/auth.go | 19 +++- cmd/root/bundle.go | 88 ++++++++++++++++--- cmd/root/bundle_test.go | 17 ++-- cmd/root/bundleflag/completion.go | 27 ------ cmd/root/bundleflag/completion_test.go | 39 -------- cmd/root/bundleflag/flags.go | 49 ----------- cmd/root/bundleflag/flags_test.go | 49 ----------- cmd/root/bundleflag/testdata/empty/.gitkeep | 0 .../testdata/invalid/databricks.yml | 5 -- .../bundleflag/testdata/valid/databricks.yml | 7 -- cmd/root/profileflag/flag.go | 20 ----- cmd/root/root.go | 7 +- 12 files changed, 104 insertions(+), 223 deletions(-) delete mode 100644 cmd/root/bundleflag/completion.go delete mode 100644 cmd/root/bundleflag/completion_test.go delete mode 100644 cmd/root/bundleflag/flags.go delete mode 100644 cmd/root/bundleflag/flags_test.go delete mode 100644 cmd/root/bundleflag/testdata/empty/.gitkeep delete mode 100644 cmd/root/bundleflag/testdata/invalid/databricks.yml delete mode 100644 cmd/root/bundleflag/testdata/valid/databricks.yml delete mode 100644 cmd/root/profileflag/flag.go diff --git a/cmd/root/auth.go b/cmd/root/auth.go index 9188bce657..0edfaaa838 100644 --- a/cmd/root/auth.go +++ b/cmd/root/auth.go @@ -6,7 +6,6 @@ import ( "fmt" "net/http" - "github.com/databricks/cli/cmd/root/profileflag" "github.com/databricks/cli/libs/cmdio" "github.com/databricks/cli/libs/databrickscfg" "github.com/databricks/databricks-sdk-go" @@ -19,6 +18,20 @@ import ( var workspaceClient int var accountClient int +func initProfileFlag(cmd *cobra.Command) { + cmd.PersistentFlags().StringP("profile", "p", "", "~/.databrickscfg profile") + cmd.RegisterFlagCompletionFunc("profile", databrickscfg.ProfileCompletion) +} + +func profileFlagValue(cmd *cobra.Command) (string, bool) { + profileFlag := cmd.Flag("profile") + if profileFlag == nil { + return "", false + } + value := profileFlag.Value.String() + return value, value != "" +} + // Helper function to create an account client or prompt once if the given configuration is not valid. func accountClientOrPrompt(ctx context.Context, cfg *config.Config, allowPrompt bool) (*databricks.AccountClient, error) { a, err := databricks.NewAccountClient((*databricks.Config)(cfg)) @@ -58,7 +71,7 @@ func MustAccountClient(cmd *cobra.Command, args []string) error { cfg := &config.Config{} // The command-line profile flag takes precedence over DATABRICKS_CONFIG_PROFILE. - profile, hasProfileFlag := profileflag.Value(cmd) + profile, hasProfileFlag := profileFlagValue(cmd) if hasProfileFlag { cfg.Profile = profile } @@ -128,7 +141,7 @@ func MustWorkspaceClient(cmd *cobra.Command, args []string) error { cfg := &config.Config{} // The command-line profile flag takes precedence over DATABRICKS_CONFIG_PROFILE. - profile, hasProfileFlag := profileflag.Value(cmd) + profile, hasProfileFlag := profileFlagValue(cmd) if hasProfileFlag { cfg.Profile = profile } diff --git a/cmd/root/bundle.go b/cmd/root/bundle.go index 24b123ac5b..636bb8584c 100644 --- a/cmd/root/bundle.go +++ b/cmd/root/bundle.go @@ -4,23 +4,56 @@ import ( "context" "github.com/databricks/cli/bundle" + "github.com/databricks/cli/bundle/env" "github.com/databricks/cli/bundle/phases" - "github.com/databricks/cli/cmd/root/bundleflag" - "github.com/databricks/cli/cmd/root/profileflag" "github.com/databricks/cli/libs/diag" - "github.com/databricks/cli/libs/env" + envlib "github.com/databricks/cli/libs/env" "github.com/spf13/cobra" + "golang.org/x/exp/maps" ) -// configureProfile applies the profile flag to the bundle. -func configureProfile(cmd *cobra.Command, b *bundle.Bundle) diag.Diagnostics { - profile, ok := profileflag.Value(cmd) - if !ok || profile == "" { - // If it's not set, use the environment variable. - profile, ok = env.Lookup(cmd.Context(), "DATABRICKS_CONFIG_PROFILE") +// getTarget returns the name of the target to operate in. +func getTarget(cmd *cobra.Command) (value string) { + // The command line flag takes precedence. + flag := cmd.Flag("target") + if flag != nil { + value = flag.Value.String() + if value != "" { + return + } + } + + oldFlag := cmd.Flag("environment") + if oldFlag != nil { + value = oldFlag.Value.String() + if value != "" { + return + } + } + + // If it's not set, use the environment variable. + target, _ := env.Target(cmd.Context()) + return target +} + +func getProfile(cmd *cobra.Command) (value string) { + // The command line flag takes precedence. + flag := cmd.Flag("profile") + if flag != nil { + value = flag.Value.String() + if value != "" { + return + } } - if !ok || profile == "" { + // If it's not set, use the environment variable. + return envlib.Get(cmd.Context(), "DATABRICKS_CONFIG_PROFILE") +} + +// configureProfile applies the profile flag to the bundle. +func configureProfile(cmd *cobra.Command, b *bundle.Bundle) diag.Diagnostics { + profile := getProfile(cmd) + if profile == "" { return nil } @@ -33,7 +66,7 @@ func configureProfile(cmd *cobra.Command, b *bundle.Bundle) diag.Diagnostics { // configureBundle applies basic mutators to the bundle configures it on the command's context. func configureBundle(cmd *cobra.Command, b *bundle.Bundle) (*bundle.Bundle, diag.Diagnostics) { var m bundle.Mutator - if target := bundleflag.Target(cmd); target == "" { + if target := getTarget(cmd); target == "" { m = phases.LoadDefaultTarget() } else { m = phases.LoadNamedTarget(target) @@ -80,3 +113,36 @@ func TryConfigureBundle(cmd *cobra.Command) (*bundle.Bundle, diag.Diagnostics) { return configureBundle(cmd, b) } + +// targetCompletion executes to autocomplete the argument to the target flag. +func targetCompletion(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) { + ctx := cmd.Context() + b, err := bundle.MustLoad(ctx) + if err != nil { + cobra.CompErrorln(err.Error()) + return nil, cobra.ShellCompDirectiveError + } + + // Load bundle but don't select a target (we're completing those). + diags := bundle.Apply(ctx, b, phases.Load()) + if err := diags.Error(); err != nil { + cobra.CompErrorln(err.Error()) + return nil, cobra.ShellCompDirectiveError + } + + return maps.Keys(b.Config.Targets), cobra.ShellCompDirectiveDefault +} + +func initTargetFlag(cmd *cobra.Command) { + // To operate in the context of a bundle, all commands must take an "target" parameter. + cmd.PersistentFlags().StringP("target", "t", "", "bundle target to use (if applicable)") + cmd.RegisterFlagCompletionFunc("target", targetCompletion) +} + +// DEPRECATED flag +func initEnvironmentFlag(cmd *cobra.Command) { + // To operate in the context of a bundle, all commands must take an "environment" parameter. + cmd.PersistentFlags().StringP("environment", "e", "", "bundle target to use (if applicable)") + cmd.PersistentFlags().MarkDeprecated("environment", "use --target flag instead") + cmd.RegisterFlagCompletionFunc("environment", targetCompletion) +} diff --git a/cmd/root/bundle_test.go b/cmd/root/bundle_test.go index 452b5ec42c..80c5044834 100644 --- a/cmd/root/bundle_test.go +++ b/cmd/root/bundle_test.go @@ -9,8 +9,6 @@ import ( "testing" "github.com/databricks/cli/bundle" - "github.com/databricks/cli/cmd/root/bundleflag" - "github.com/databricks/cli/cmd/root/profileflag" "github.com/databricks/cli/internal/testutil" "github.com/spf13/cobra" "github.com/stretchr/testify/assert" @@ -36,7 +34,7 @@ func emptyCommand(t *testing.T) *cobra.Command { ctx := context.Background() cmd := &cobra.Command{} cmd.SetContext(ctx) - profileflag.Init(cmd) + initProfileFlag(cmd) return cmd } @@ -140,37 +138,38 @@ func TestBundleConfigureWithProfileFlagAndEnvVariable(t *testing.T) { func TestTargetFlagFull(t *testing.T) { cmd := emptyCommand(t) - bundleflag.Init(cmd) + initTargetFlag(cmd) cmd.SetArgs([]string{"version", "--target", "development"}) ctx := context.Background() err := cmd.ExecuteContext(ctx) assert.NoError(t, err) - assert.Equal(t, "development", bundleflag.Target(cmd)) + assert.Equal(t, "development", getTarget(cmd)) } func TestTargetFlagShort(t *testing.T) { cmd := emptyCommand(t) - bundleflag.Init(cmd) + initTargetFlag(cmd) cmd.SetArgs([]string{"version", "-t", "production"}) ctx := context.Background() err := cmd.ExecuteContext(ctx) assert.NoError(t, err) - assert.Equal(t, "production", bundleflag.Target(cmd)) + assert.Equal(t, "production", getTarget(cmd)) } // TODO: remove when environment flag is fully deprecated func TestTargetEnvironmentFlag(t *testing.T) { cmd := emptyCommand(t) - bundleflag.Init(cmd) + initTargetFlag(cmd) + initEnvironmentFlag(cmd) cmd.SetArgs([]string{"version", "--environment", "development"}) ctx := context.Background() err := cmd.ExecuteContext(ctx) assert.NoError(t, err) - assert.Equal(t, "development", bundleflag.Target(cmd)) + assert.Equal(t, "development", getTarget(cmd)) } diff --git a/cmd/root/bundleflag/completion.go b/cmd/root/bundleflag/completion.go deleted file mode 100644 index 5fa264d364..0000000000 --- a/cmd/root/bundleflag/completion.go +++ /dev/null @@ -1,27 +0,0 @@ -package bundleflag - -import ( - "github.com/databricks/cli/bundle" - "github.com/databricks/cli/bundle/phases" - "github.com/spf13/cobra" - "golang.org/x/exp/maps" -) - -// targetCompletion executes to autocomplete the argument to the target flag. -func targetCompletion(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) { - ctx := cmd.Context() - b, err := bundle.MustLoad(ctx) - if err != nil { - cobra.CompErrorln(err.Error()) - return nil, cobra.ShellCompDirectiveError - } - - // Load bundle but don't select a target (we're completing those). - diags := bundle.Apply(ctx, b, phases.Load()) - if err := diags.Error(); err != nil { - cobra.CompErrorln(err.Error()) - return nil, cobra.ShellCompDirectiveError - } - - return maps.Keys(b.Config.Targets), cobra.ShellCompDirectiveDefault -} diff --git a/cmd/root/bundleflag/completion_test.go b/cmd/root/bundleflag/completion_test.go deleted file mode 100644 index e5ab393cb0..0000000000 --- a/cmd/root/bundleflag/completion_test.go +++ /dev/null @@ -1,39 +0,0 @@ -package bundleflag - -import ( - "context" - "testing" - - "github.com/databricks/cli/internal/testutil" - "github.com/spf13/cobra" - "github.com/stretchr/testify/assert" -) - -func prepareCompletionTest(t *testing.T, dir string) *cobra.Command { - testutil.CleanupEnvironment(t) - testutil.Chdir(t, dir) - cmd := cobra.Command{} - cmd.SetContext(context.Background()) - return &cmd -} - -func TestCompletionEmpty(t *testing.T) { - cmd := prepareCompletionTest(t, "./testdata/empty") - out, comp := targetCompletion(cmd, []string{}, "") - assert.Equal(t, cobra.ShellCompDirectiveError, comp) - assert.Empty(t, out) -} - -func TestCompletionInvalid(t *testing.T) { - cmd := prepareCompletionTest(t, "./testdata/invalid") - out, comp := targetCompletion(cmd, []string{}, "") - assert.Equal(t, cobra.ShellCompDirectiveError, comp) - assert.Empty(t, out) -} - -func TestCompletionValid(t *testing.T) { - cmd := prepareCompletionTest(t, "./testdata/valid") - out, comp := targetCompletion(cmd, []string{}, "") - assert.Equal(t, cobra.ShellCompDirectiveDefault, comp) - assert.ElementsMatch(t, []string{"foo", "bar", "qux"}, out) -} diff --git a/cmd/root/bundleflag/flags.go b/cmd/root/bundleflag/flags.go deleted file mode 100644 index b867fb726a..0000000000 --- a/cmd/root/bundleflag/flags.go +++ /dev/null @@ -1,49 +0,0 @@ -package bundleflag - -import ( - "github.com/databricks/cli/bundle/env" - "github.com/spf13/cobra" -) - -// Target returns the name of the target to operate in. -func Target(cmd *cobra.Command) (value string) { - // The command line flag takes precedence. - flag := cmd.Flag("target") - if flag != nil { - value = flag.Value.String() - if value != "" { - return - } - } - - oldFlag := cmd.Flag("environment") - if oldFlag != nil { - value = oldFlag.Value.String() - if value != "" { - return - } - } - - // If it's not set, use the environment variable. - target, _ := env.Target(cmd.Context()) - return target -} - -func Init(cmd *cobra.Command) { - initTargetFlag(cmd) - initEnvironmentFlag(cmd) -} - -func initTargetFlag(cmd *cobra.Command) { - // To operate in the context of a bundle, all commands must take an "target" parameter. - cmd.PersistentFlags().StringP("target", "t", "", "bundle target to use (if applicable)") - cmd.RegisterFlagCompletionFunc("target", targetCompletion) -} - -// DEPRECATED flag -func initEnvironmentFlag(cmd *cobra.Command) { - // To operate in the context of a bundle, all commands must take an "environment" parameter. - cmd.PersistentFlags().StringP("environment", "e", "", "bundle target to use (if applicable)") - cmd.PersistentFlags().MarkDeprecated("environment", "use --target flag instead") - cmd.RegisterFlagCompletionFunc("environment", targetCompletion) -} diff --git a/cmd/root/bundleflag/flags_test.go b/cmd/root/bundleflag/flags_test.go deleted file mode 100644 index cc34749538..0000000000 --- a/cmd/root/bundleflag/flags_test.go +++ /dev/null @@ -1,49 +0,0 @@ -package bundleflag - -import ( - "context" - "testing" - - benv "github.com/databricks/cli/bundle/env" - "github.com/databricks/cli/libs/env" - "github.com/spf13/cobra" - "github.com/stretchr/testify/require" -) - -func prepareFlagTest() *cobra.Command { - cmd := &cobra.Command{} - cmd.SetContext(context.Background()) - Init(cmd) - return cmd -} - -func TestTargetValue(t *testing.T) { - cmd := prepareFlagTest() - cmd.RunE = func(cmd *cobra.Command, args []string) error { - target := Target(cmd) - if target != "test" { - t.Errorf("expected 'test', got %q", target) - } - return nil - } - - t.Run("target flag", func(t *testing.T) { - cmd.SetArgs([]string{"--target", "test"}) - err := cmd.Execute() - require.NoError(t, err) - }) - - t.Run("environment flag", func(t *testing.T) { - cmd.SetArgs([]string{"--environment", "test"}) - err := cmd.Execute() - require.NoError(t, err) - }) - - t.Run("environment variable", func(t *testing.T) { - ctx := env.Set(cmd.Context(), benv.TargetVariable, "test") - cmd.SetContext(ctx) - cmd.SetArgs([]string{}) - err := cmd.Execute() - require.NoError(t, err) - }) -} diff --git a/cmd/root/bundleflag/testdata/empty/.gitkeep b/cmd/root/bundleflag/testdata/empty/.gitkeep deleted file mode 100644 index e69de29bb2..0000000000 diff --git a/cmd/root/bundleflag/testdata/invalid/databricks.yml b/cmd/root/bundleflag/testdata/invalid/databricks.yml deleted file mode 100644 index 964e2dc1a7..0000000000 --- a/cmd/root/bundleflag/testdata/invalid/databricks.yml +++ /dev/null @@ -1,5 +0,0 @@ -bundle: - name: invalid - -include: - - ./i_dont_exist.yml diff --git a/cmd/root/bundleflag/testdata/valid/databricks.yml b/cmd/root/bundleflag/testdata/valid/databricks.yml deleted file mode 100644 index d893895746..0000000000 --- a/cmd/root/bundleflag/testdata/valid/databricks.yml +++ /dev/null @@ -1,7 +0,0 @@ -bundle: - name: valid - -targets: - foo: ~ - bar: ~ - qux: ~ diff --git a/cmd/root/profileflag/flag.go b/cmd/root/profileflag/flag.go deleted file mode 100644 index 5110458d64..0000000000 --- a/cmd/root/profileflag/flag.go +++ /dev/null @@ -1,20 +0,0 @@ -package profileflag - -import ( - "github.com/databricks/cli/libs/databrickscfg" - "github.com/spf13/cobra" -) - -func Init(cmd *cobra.Command) { - cmd.PersistentFlags().StringP("profile", "p", "", "~/.databrickscfg profile") - cmd.RegisterFlagCompletionFunc("profile", databrickscfg.ProfileCompletion) -} - -func Value(cmd *cobra.Command) (string, bool) { - profileFlag := cmd.Flag("profile") - if profileFlag == nil { - return "", false - } - value := profileFlag.Value.String() - return value, value != "" -} diff --git a/cmd/root/root.go b/cmd/root/root.go index ffb10bc586..38eb42ccb0 100644 --- a/cmd/root/root.go +++ b/cmd/root/root.go @@ -8,8 +8,6 @@ import ( "log/slog" - "github.com/databricks/cli/cmd/root/bundleflag" - "github.com/databricks/cli/cmd/root/profileflag" "github.com/databricks/cli/internal/build" "github.com/databricks/cli/libs/cmdio" "github.com/databricks/cli/libs/log" @@ -40,8 +38,9 @@ func New(ctx context.Context) *cobra.Command { logFlags := initLogFlags(cmd) progressLoggerFlag := initProgressLoggerFlag(cmd, logFlags) outputFlag := initOutputFlag(cmd) - profileflag.Init(cmd) - bundleflag.Init(cmd) + initProfileFlag(cmd) + initEnvironmentFlag(cmd) + initTargetFlag(cmd) cmd.PersistentPreRunE = func(cmd *cobra.Command, args []string) error { ctx := cmd.Context() From 4d494b629cf71868e4aa2b5f4038e62d08a06305 Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Wed, 27 Mar 2024 11:53:55 +0100 Subject: [PATCH 10/15] Write entry point configuration from test --- cmd/bundle/generate/generate_test.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/cmd/bundle/generate/generate_test.go b/cmd/bundle/generate/generate_test.go index 69ef639ae0..dbe17b9fb3 100644 --- a/cmd/bundle/generate/generate_test.go +++ b/cmd/bundle/generate/generate_test.go @@ -10,6 +10,7 @@ import ( "testing" "github.com/databricks/cli/bundle" + "github.com/databricks/cli/internal/testutil" "github.com/databricks/databricks-sdk-go/experimental/mocks" "github.com/databricks/databricks-sdk-go/service/compute" "github.com/databricks/databricks-sdk-go/service/jobs" @@ -23,6 +24,7 @@ func TestGeneratePipelineCommand(t *testing.T) { cmd := NewGeneratePipelineCommand() root := t.TempDir() + testutil.Touch(t, root, "databricks.yml") b := &bundle.Bundle{ RootPath: root, } @@ -121,6 +123,7 @@ func TestGenerateJobCommand(t *testing.T) { cmd := NewGenerateJobCommand() root := t.TempDir() + testutil.Touch(t, root, "databricks.yml") b := &bundle.Bundle{ RootPath: root, } From 93de5c79466975074f9c68c5ca40f7c87c34f575 Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Wed, 27 Mar 2024 13:00:52 +0100 Subject: [PATCH 11/15] Return bundle if configured on context --- cmd/root/bundle.go | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/cmd/root/bundle.go b/cmd/root/bundle.go index 636bb8584c..fa1039f50b 100644 --- a/cmd/root/bundle.go +++ b/cmd/root/bundle.go @@ -90,6 +90,13 @@ func configureBundle(cmd *cobra.Command, b *bundle.Bundle) (*bundle.Bundle, diag // MustConfigureBundle configures a bundle on the command context. func MustConfigureBundle(cmd *cobra.Command) (*bundle.Bundle, diag.Diagnostics) { + // A bundle may be configured on the context when testing. + // If it is, return it immediately. + b := bundle.GetOrNil(cmd.Context()) + if b != nil { + return b, nil + } + b, err := bundle.MustLoad(cmd.Context()) if err != nil { return nil, diag.FromErr(err) @@ -101,6 +108,13 @@ func MustConfigureBundle(cmd *cobra.Command) (*bundle.Bundle, diag.Diagnostics) // TryConfigureBundle configures a bundle on the command context // if there is one, but doesn't fail if there isn't one. func TryConfigureBundle(cmd *cobra.Command) (*bundle.Bundle, diag.Diagnostics) { + // A bundle may be configured on the context when testing. + // If it is, return it immediately. + b := bundle.GetOrNil(cmd.Context()) + if b != nil { + return b, nil + } + b, err := bundle.TryLoad(cmd.Context()) if err != nil { return nil, diag.FromErr(err) From 790103ef53d9f55f008579b5049147c8366892ba Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Wed, 27 Mar 2024 13:03:55 +0100 Subject: [PATCH 12/15] No need for databricks.yml now that bundle is stubbed --- cmd/bundle/generate/generate_test.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/cmd/bundle/generate/generate_test.go b/cmd/bundle/generate/generate_test.go index dbe17b9fb3..69ef639ae0 100644 --- a/cmd/bundle/generate/generate_test.go +++ b/cmd/bundle/generate/generate_test.go @@ -10,7 +10,6 @@ import ( "testing" "github.com/databricks/cli/bundle" - "github.com/databricks/cli/internal/testutil" "github.com/databricks/databricks-sdk-go/experimental/mocks" "github.com/databricks/databricks-sdk-go/service/compute" "github.com/databricks/databricks-sdk-go/service/jobs" @@ -24,7 +23,6 @@ func TestGeneratePipelineCommand(t *testing.T) { cmd := NewGeneratePipelineCommand() root := t.TempDir() - testutil.Touch(t, root, "databricks.yml") b := &bundle.Bundle{ RootPath: root, } @@ -123,7 +121,6 @@ func TestGenerateJobCommand(t *testing.T) { cmd := NewGenerateJobCommand() root := t.TempDir() - testutil.Touch(t, root, "databricks.yml") b := &bundle.Bundle{ RootPath: root, } From f701924041094bd9a8ec08afb1bbcca95589df57 Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Wed, 27 Mar 2024 13:07:48 +0100 Subject: [PATCH 13/15] Update comment --- 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 fa1039f50b..4ed89c57b8 100644 --- a/cmd/root/bundle.go +++ b/cmd/root/bundle.go @@ -63,7 +63,7 @@ func configureProfile(cmd *cobra.Command, b *bundle.Bundle) diag.Diagnostics { }) } -// configureBundle applies basic mutators to the bundle configures it on the command's context. +// configureBundle loads the bundle configuration and configures flag values, if any. func configureBundle(cmd *cobra.Command, b *bundle.Bundle) (*bundle.Bundle, diag.Diagnostics) { var m bundle.Mutator if target := getTarget(cmd); target == "" { From 846148085011b083183ce9ddc327b40bbf6af9ee Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Thu, 28 Mar 2024 10:07:53 +0100 Subject: [PATCH 14/15] Add assertions to bundle profile selection tests --- cmd/root/bundle_test.go | 55 +++++++++++++++++++++-------------------- 1 file changed, 28 insertions(+), 27 deletions(-) diff --git a/cmd/root/bundle_test.go b/cmd/root/bundle_test.go index 80c5044834..81e2e5a4ac 100644 --- a/cmd/root/bundle_test.go +++ b/cmd/root/bundle_test.go @@ -61,9 +61,10 @@ func TestBundleConfigureDefault(t *testing.T) { cmd := emptyCommand(t) b := setup(t, cmd, "https://x.com") - assert.NotPanics(t, func() { - b.WorkspaceClient() - }) + + client, err := b.InitializeWorkspaceClient() + require.NoError(t, err) + assert.Equal(t, "https://x.com", client.Config.Host) } func TestBundleConfigureWithMultipleMatches(t *testing.T) { @@ -71,9 +72,9 @@ func TestBundleConfigureWithMultipleMatches(t *testing.T) { cmd := emptyCommand(t) b := setup(t, cmd, "https://a.com") - assert.Panics(t, func() { - b.WorkspaceClient() - }) + + _, err := b.InitializeWorkspaceClient() + assert.ErrorContains(t, err, "multiple profiles matched: PROFILE-1, PROFILE-2") } func TestBundleConfigureWithNonExistentProfileFlag(t *testing.T) { @@ -81,11 +82,10 @@ func TestBundleConfigureWithNonExistentProfileFlag(t *testing.T) { cmd := emptyCommand(t) cmd.Flag("profile").Value.Set("NOEXIST") - b := setup(t, cmd, "https://x.com") - assert.Panics(t, func() { - b.WorkspaceClient() - }) + + _, err := b.InitializeWorkspaceClient() + assert.ErrorContains(t, err, "has no NOEXIST profile configured") } func TestBundleConfigureWithMismatchedProfile(t *testing.T) { @@ -93,11 +93,10 @@ func TestBundleConfigureWithMismatchedProfile(t *testing.T) { cmd := emptyCommand(t) cmd.Flag("profile").Value.Set("PROFILE-1") - b := setup(t, cmd, "https://x.com") - assert.PanicsWithError(t, "cannot resolve bundle auth configuration: config host mismatch: profile uses host https://a.com, but CLI configured to use https://x.com", func() { - b.WorkspaceClient() - }) + + _, 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) { @@ -105,35 +104,37 @@ func TestBundleConfigureWithCorrectProfile(t *testing.T) { cmd := emptyCommand(t) cmd.Flag("profile").Value.Set("PROFILE-1") - b := setup(t, cmd, "https://a.com") - assert.NotPanics(t, func() { - b.WorkspaceClient() - }) + + 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") + t.Setenv("DATABRICKS_CONFIG_PROFILE", "PROFILE-1") cmd := emptyCommand(t) b := setup(t, cmd, "https://x.com") - assert.PanicsWithError(t, "cannot resolve bundle auth configuration: config host mismatch: profile uses host https://a.com, but CLI configured to use https://x.com", func() { - b.WorkspaceClient() - }) + + _, 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") + t.Setenv("DATABRICKS_CONFIG_PROFILE", "NOEXIST") cmd := emptyCommand(t) cmd.Flag("profile").Value.Set("PROFILE-1") - b := setup(t, cmd, "https://a.com") - assert.NotPanics(t, func() { - b.WorkspaceClient() - }) + + 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 TestTargetFlagFull(t *testing.T) { From 96343434b05f9872651f005643e0aaec2e4c54ba Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Thu, 28 Mar 2024 10:22:10 +0100 Subject: [PATCH 15/15] More coverage --- cmd/root/bundle_test.go | 94 +++++++++++++++++++++++++++++++++++++---- 1 file changed, 86 insertions(+), 8 deletions(-) diff --git a/cmd/root/bundle_test.go b/cmd/root/bundle_test.go index 81e2e5a4ac..3018842870 100644 --- a/cmd/root/bundle_test.go +++ b/cmd/root/bundle_test.go @@ -38,7 +38,7 @@ func emptyCommand(t *testing.T) *cobra.Command { return cmd } -func setup(t *testing.T, cmd *cobra.Command, host string) *bundle.Bundle { +func setupWithHost(t *testing.T, cmd *cobra.Command, host string) *bundle.Bundle { setupDatabricksCfg(t) rootPath := t.TempDir() @@ -56,11 +56,29 @@ workspace: return b } +func setupWithProfile(t *testing.T, cmd *cobra.Command, profile string) *bundle.Bundle { + setupDatabricksCfg(t) + + rootPath := t.TempDir() + testutil.Chdir(t, rootPath) + + contents := fmt.Sprintf(` +workspace: + profile: %q +`, profile) + err := os.WriteFile(filepath.Join(rootPath, "databricks.yml"), []byte(contents), 0644) + 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 := setup(t, cmd, "https://x.com") + b := setupWithHost(t, cmd, "https://x.com") client, err := b.InitializeWorkspaceClient() require.NoError(t, err) @@ -71,7 +89,7 @@ func TestBundleConfigureWithMultipleMatches(t *testing.T) { testutil.CleanupEnvironment(t) cmd := emptyCommand(t) - b := setup(t, cmd, "https://a.com") + b := setupWithHost(t, cmd, "https://a.com") _, err := b.InitializeWorkspaceClient() assert.ErrorContains(t, err, "multiple profiles matched: PROFILE-1, PROFILE-2") @@ -82,7 +100,7 @@ func TestBundleConfigureWithNonExistentProfileFlag(t *testing.T) { cmd := emptyCommand(t) cmd.Flag("profile").Value.Set("NOEXIST") - b := setup(t, cmd, "https://x.com") + b := setupWithHost(t, cmd, "https://x.com") _, err := b.InitializeWorkspaceClient() assert.ErrorContains(t, err, "has no NOEXIST profile configured") @@ -93,7 +111,7 @@ func TestBundleConfigureWithMismatchedProfile(t *testing.T) { cmd := emptyCommand(t) cmd.Flag("profile").Value.Set("PROFILE-1") - b := setup(t, cmd, "https://x.com") + 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") @@ -104,7 +122,7 @@ func TestBundleConfigureWithCorrectProfile(t *testing.T) { cmd := emptyCommand(t) cmd.Flag("profile").Value.Set("PROFILE-1") - b := setup(t, cmd, "https://a.com") + b := setupWithHost(t, cmd, "https://a.com") client, err := b.InitializeWorkspaceClient() require.NoError(t, err) @@ -117,7 +135,7 @@ func TestBundleConfigureWithMismatchedProfileEnvVariable(t *testing.T) { t.Setenv("DATABRICKS_CONFIG_PROFILE", "PROFILE-1") cmd := emptyCommand(t) - b := setup(t, cmd, "https://x.com") + 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") @@ -129,7 +147,7 @@ func TestBundleConfigureWithProfileFlagAndEnvVariable(t *testing.T) { t.Setenv("DATABRICKS_CONFIG_PROFILE", "NOEXIST") cmd := emptyCommand(t) cmd.Flag("profile").Value.Set("PROFILE-1") - b := setup(t, cmd, "https://a.com") + b := setupWithHost(t, cmd, "https://a.com") client, err := b.InitializeWorkspaceClient() require.NoError(t, err) @@ -137,6 +155,66 @@ func TestBundleConfigureWithProfileFlagAndEnvVariable(t *testing.T) { 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) + cmd.Flag("profile").Value.Set("PROFILE-2") + 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) + cmd.Flag("profile").Value.Set("PROFILE-2") + 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) { cmd := emptyCommand(t) initTargetFlag(cmd)