Skip to content

Commit

Permalink
Log the bundle root configuration file if applicable (#657)
Browse files Browse the repository at this point in the history
## Changes

Pass through the `context.Context` to the bundle loader functions.

## Tests

Unit tests pass.
  • Loading branch information
pietern authored Aug 11, 2023
1 parent 6b615cc commit 8656c4a
Show file tree
Hide file tree
Showing 9 changed files with 57 additions and 42 deletions.
14 changes: 9 additions & 5 deletions bundle/bundle.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
package bundle

import (
"context"
"fmt"
"os"
"path/filepath"
Expand All @@ -16,6 +17,7 @@ import (
"github.com/databricks/cli/folders"
"github.com/databricks/cli/libs/git"
"github.com/databricks/cli/libs/locker"
"github.com/databricks/cli/libs/log"
"github.com/databricks/cli/libs/terraform"
"github.com/databricks/databricks-sdk-go"
sdkconfig "github.com/databricks/databricks-sdk-go/config"
Expand Down Expand Up @@ -45,7 +47,7 @@ type Bundle struct {

const ExtraIncludePathsKey string = "DATABRICKS_BUNDLE_INCLUDES"

func Load(path string) (*Bundle, error) {
func Load(ctx context.Context, path string) (*Bundle, error) {
bundle := &Bundle{}
stat, err := os.Stat(path)
if err != nil {
Expand All @@ -56,6 +58,7 @@ func Load(path string) (*Bundle, error) {
_, hasIncludePathEnv := os.LookupEnv(ExtraIncludePathsKey)
_, hasBundleRootEnv := os.LookupEnv(envBundleRoot)
if hasIncludePathEnv && hasBundleRootEnv && stat.IsDir() {
log.Debugf(ctx, "No bundle configuration; using bundle root: %s", path)
bundle.Config = config.Root{
Path: path,
Bundle: config.Bundle{
Expand All @@ -66,6 +69,7 @@ func Load(path string) (*Bundle, error) {
}
return nil, err
}
log.Debugf(ctx, "Loading bundle configuration from: %s", configFile)
err = bundle.Config.Load(configFile)
if err != nil {
return nil, err
Expand All @@ -75,19 +79,19 @@ func Load(path string) (*Bundle, error) {

// MustLoad returns a bundle configuration.
// It returns an error if a bundle was not found or could not be loaded.
func MustLoad() (*Bundle, error) {
func MustLoad(ctx context.Context) (*Bundle, error) {
root, err := mustGetRoot()
if err != nil {
return nil, err
}

return Load(root)
return Load(ctx, root)
}

// TryLoad returns a bundle configuration if there is one, but doesn't fail if there isn't one.
// It returns an error if a bundle was found but could not be loaded.
// It returns a `nil` bundle if a bundle was not found.
func TryLoad() (*Bundle, error) {
func TryLoad(ctx context.Context) (*Bundle, error) {
root, err := tryGetRoot()
if err != nil {
return nil, err
Expand All @@ -98,7 +102,7 @@ func TryLoad() (*Bundle, error) {
return nil, nil
}

return Load(root)
return Load(ctx, root)
}

func (b *Bundle) WorkspaceClient() *databricks.WorkspaceClient {
Expand Down
21 changes: 11 additions & 10 deletions bundle/bundle_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package bundle

import (
"context"
"os"
"path/filepath"
"testing"
Expand All @@ -10,13 +11,13 @@ import (
)

func TestLoadNotExists(t *testing.T) {
b, err := Load("/doesntexist")
b, err := Load(context.Background(), "/doesntexist")
assert.True(t, os.IsNotExist(err))
assert.Nil(t, b)
}

func TestLoadExists(t *testing.T) {
b, err := Load("./tests/basic")
b, err := Load(context.Background(), "./tests/basic")
require.Nil(t, err)
assert.Equal(t, "basic", b.Config.Bundle.Name)
}
Expand All @@ -27,7 +28,7 @@ func TestBundleCacheDir(t *testing.T) {
require.NoError(t, err)
f1.Close()

bundle, err := Load(projectDir)
bundle, err := Load(context.Background(), projectDir)
require.NoError(t, err)

// Artificially set environment.
Expand All @@ -51,7 +52,7 @@ func TestBundleCacheDirOverride(t *testing.T) {
require.NoError(t, err)
f1.Close()

bundle, err := Load(projectDir)
bundle, err := Load(context.Background(), projectDir)
require.NoError(t, err)

// Artificially set environment.
Expand All @@ -70,39 +71,39 @@ func TestBundleCacheDirOverride(t *testing.T) {

func TestBundleMustLoadSuccess(t *testing.T) {
t.Setenv(envBundleRoot, "./tests/basic")
b, err := MustLoad()
b, err := MustLoad(context.Background())
require.NoError(t, err)
assert.Equal(t, "tests/basic", filepath.ToSlash(b.Config.Path))
}

func TestBundleMustLoadFailureWithEnv(t *testing.T) {
t.Setenv(envBundleRoot, "./tests/doesntexist")
_, err := MustLoad()
_, err := MustLoad(context.Background())
require.Error(t, err, "not a directory")
}

func TestBundleMustLoadFailureIfNotFound(t *testing.T) {
chdir(t, t.TempDir())
_, err := MustLoad()
_, err := MustLoad(context.Background())
require.Error(t, err, "unable to find bundle root")
}

func TestBundleTryLoadSuccess(t *testing.T) {
t.Setenv(envBundleRoot, "./tests/basic")
b, err := TryLoad()
b, err := TryLoad(context.Background())
require.NoError(t, err)
assert.Equal(t, "tests/basic", filepath.ToSlash(b.Config.Path))
}

func TestBundleTryLoadFailureWithEnv(t *testing.T) {
t.Setenv(envBundleRoot, "./tests/doesntexist")
_, err := TryLoad()
_, err := TryLoad(context.Background())
require.Error(t, err, "not a directory")
}

func TestBundleTryLoadOkIfNotFound(t *testing.T) {
chdir(t, t.TempDir())
b, err := TryLoad()
b, err := TryLoad(context.Background())
assert.NoError(t, err)
assert.Nil(t, b)
}
9 changes: 5 additions & 4 deletions bundle/root_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package bundle

import (
"context"
"os"
"path/filepath"
"testing"
Expand Down Expand Up @@ -108,7 +109,7 @@ func TestLoadYamlWhenIncludesEnvPresent(t *testing.T) {
chdir(t, filepath.Join(".", "tests", "basic"))
t.Setenv(ExtraIncludePathsKey, "test")

bundle, err := MustLoad()
bundle, err := MustLoad(context.Background())
assert.NoError(t, err)
assert.Equal(t, "basic", bundle.Config.Bundle.Name)

Expand All @@ -123,7 +124,7 @@ func TestLoadDefautlBundleWhenNoYamlAndRootAndIncludesEnvPresent(t *testing.T) {
t.Setenv(envBundleRoot, dir)
t.Setenv(ExtraIncludePathsKey, "test")

bundle, err := MustLoad()
bundle, err := MustLoad(context.Background())
assert.NoError(t, err)
assert.Equal(t, dir, bundle.Config.Path)
}
Expand All @@ -133,7 +134,7 @@ func TestErrorIfNoYamlNoRootEnvAndIncludesEnvPresent(t *testing.T) {
chdir(t, dir)
t.Setenv(ExtraIncludePathsKey, "test")

_, err := MustLoad()
_, err := MustLoad(context.Background())
assert.Error(t, err)
}

Expand All @@ -142,6 +143,6 @@ func TestErrorIfNoYamlNoIncludesEnvAndRootEnvPresent(t *testing.T) {
chdir(t, dir)
t.Setenv(envBundleRoot, dir)

_, err := MustLoad()
_, err := MustLoad(context.Background())
assert.Error(t, err)
}
21 changes: 12 additions & 9 deletions bundle/tests/bundle/wheel_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,48 +12,51 @@ import (
)

func TestBundlePythonWheelBuild(t *testing.T) {
b, err := bundle.Load("./python_wheel")
ctx := context.Background()
b, err := bundle.Load(ctx, "./python_wheel")
require.NoError(t, err)

m := phases.Build()
err = m.Apply(context.Background(), b)
err = m.Apply(ctx, b)
require.NoError(t, err)

matches, err := filepath.Glob("python_wheel/my_test_code/dist/my_test_code-*.whl")
require.NoError(t, err)
require.Equal(t, 1, len(matches))

match := libraries.MatchWithArtifacts()
err = match.Apply(context.Background(), b)
err = match.Apply(ctx, b)
require.NoError(t, err)
}

func TestBundlePythonWheelBuildAutoDetect(t *testing.T) {
b, err := bundle.Load("./python_wheel_no_artifact")
ctx := context.Background()
b, err := bundle.Load(ctx, "./python_wheel_no_artifact")
require.NoError(t, err)

m := phases.Build()
err = m.Apply(context.Background(), b)
err = m.Apply(ctx, b)
require.NoError(t, err)

matches, err := filepath.Glob("python_wheel/my_test_code/dist/my_test_code-*.whl")
require.NoError(t, err)
require.Equal(t, 1, len(matches))

match := libraries.MatchWithArtifacts()
err = match.Apply(context.Background(), b)
err = match.Apply(ctx, b)
require.NoError(t, err)
}

func TestBundlePythonWheelWithDBFSLib(t *testing.T) {
b, err := bundle.Load("./python_wheel_dbfs_lib")
ctx := context.Background()
b, err := bundle.Load(ctx, "./python_wheel_dbfs_lib")
require.NoError(t, err)

m := phases.Build()
err = m.Apply(context.Background(), b)
err = m.Apply(ctx, b)
require.NoError(t, err)

match := libraries.MatchWithArtifacts()
err = match.Apply(context.Background(), b)
err = match.Apply(ctx, b)
require.NoError(t, err)
}
13 changes: 8 additions & 5 deletions bundle/tests/conflicting_resource_ids_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,24 +13,27 @@ import (
)

func TestConflictingResourceIdsNoSubconfig(t *testing.T) {
_, err := bundle.Load("./conflicting_resource_ids/no_subconfigurations")
ctx := context.Background()
_, err := bundle.Load(ctx, "./conflicting_resource_ids/no_subconfigurations")
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))
}

func TestConflictingResourceIdsOneSubconfig(t *testing.T) {
b, err := bundle.Load("./conflicting_resource_ids/one_subconfiguration")
ctx := context.Background()
b, err := bundle.Load(ctx, "./conflicting_resource_ids/one_subconfiguration")
require.NoError(t, err)
err = bundle.Apply(context.Background(), b, bundle.Seq(mutator.DefaultMutators()...))
err = bundle.Apply(ctx, b, bundle.Seq(mutator.DefaultMutators()...))
bundleConfigPath := filepath.FromSlash("conflicting_resource_ids/one_subconfiguration/databricks.yml")
resourcesConfigPath := filepath.FromSlash("conflicting_resource_ids/one_subconfiguration/resources.yml")
assert.ErrorContains(t, err, fmt.Sprintf("multiple resources named foo (job at %s, pipeline at %s)", bundleConfigPath, resourcesConfigPath))
}

func TestConflictingResourceIdsTwoSubconfigs(t *testing.T) {
b, err := bundle.Load("./conflicting_resource_ids/two_subconfigurations")
ctx := context.Background()
b, err := bundle.Load(ctx, "./conflicting_resource_ids/two_subconfigurations")
require.NoError(t, err)
err = bundle.Apply(context.Background(), b, bundle.Seq(mutator.DefaultMutators()...))
err = bundle.Apply(ctx, b, bundle.Seq(mutator.DefaultMutators()...))
resources1ConfigPath := filepath.FromSlash("conflicting_resource_ids/two_subconfigurations/resources1.yml")
resources2ConfigPath := filepath.FromSlash("conflicting_resource_ids/two_subconfigurations/resources2.yml")
assert.ErrorContains(t, err, fmt.Sprintf("multiple resources named foo (job at %s, pipeline at %s)", resources1ConfigPath, resources2ConfigPath))
Expand Down
5 changes: 3 additions & 2 deletions bundle/tests/include_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,10 @@ import (
)

func TestIncludeInvalid(t *testing.T) {
b, err := bundle.Load("./include_invalid")
ctx := context.Background()
b, err := bundle.Load(ctx, "./include_invalid")
require.NoError(t, err)
err = bundle.Apply(context.Background(), b, bundle.Seq(mutator.DefaultMutators()...))
err = bundle.Apply(ctx, b, bundle.Seq(mutator.DefaultMutators()...))
require.Error(t, err)
assert.Contains(t, err.Error(), "notexists.yml defined in 'include' section does not match any files")
}
Expand Down
5 changes: 3 additions & 2 deletions bundle/tests/loader.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,10 @@ import (
)

func load(t *testing.T, path string) *bundle.Bundle {
b, err := bundle.Load(path)
ctx := context.Background()
b, err := bundle.Load(ctx, path)
require.NoError(t, err)
err = bundle.Apply(context.Background(), b, bundle.Seq(mutator.DefaultMutators()...))
err = bundle.Apply(ctx, b, bundle.Seq(mutator.DefaultMutators()...))
require.NoError(t, err)
return b
}
Expand Down
9 changes: 5 additions & 4 deletions cmd/root/bundle.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package root

import (
"context"
"os"

"github.com/databricks/cli/bundle"
Expand Down Expand Up @@ -41,8 +42,9 @@ func getProfile(cmd *cobra.Command) (value string) {
}

// loadBundle loads the bundle configuration and applies default mutators.
func loadBundle(cmd *cobra.Command, args []string, load func() (*bundle.Bundle, error)) (*bundle.Bundle, error) {
b, err := load()
func loadBundle(cmd *cobra.Command, args []string, load func(ctx context.Context) (*bundle.Bundle, error)) (*bundle.Bundle, error) {
ctx := cmd.Context()
b, err := load(ctx)
if err != nil {
return nil, err
}
Expand All @@ -57,7 +59,6 @@ func loadBundle(cmd *cobra.Command, args []string, load func() (*bundle.Bundle,
b.Config.Workspace.Profile = profile
}

ctx := cmd.Context()
err = bundle.Apply(ctx, b, bundle.Seq(mutator.DefaultMutators()...))
if err != nil {
return nil, err
Expand All @@ -67,7 +68,7 @@ func loadBundle(cmd *cobra.Command, args []string, load func() (*bundle.Bundle,
}

// configureBundle loads the bundle configuration and configures it on the command's context.
func configureBundle(cmd *cobra.Command, args []string, load func() (*bundle.Bundle, error)) error {
func configureBundle(cmd *cobra.Command, args []string, load func(ctx context.Context) (*bundle.Bundle, error)) error {
b, err := loadBundle(cmd, args, load)
if err != nil {
return err
Expand Down
2 changes: 1 addition & 1 deletion cmd/root/bundle_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ func emptyCommand(t *testing.T) *cobra.Command {
func setup(t *testing.T, cmd *cobra.Command, host string) *bundle.Bundle {
setupDatabricksCfg(t)

err := configureBundle(cmd, []string{"validate"}, func() (*bundle.Bundle, error) {
err := configureBundle(cmd, []string{"validate"}, func(_ context.Context) (*bundle.Bundle, error) {
return &bundle.Bundle{
Config: config.Root{
Bundle: config.Bundle{
Expand Down

0 comments on commit 8656c4a

Please sign in to comment.