Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Load bundle configuration from mutator #1318

Merged
merged 8 commits into from
Mar 27, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 1 addition & 6 deletions bundle/bundle.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
4 changes: 2 additions & 2 deletions bundle/bundle_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
34 changes: 34 additions & 0 deletions bundle/config/loader/entry_point.go
Original file line number Diff line number Diff line change
@@ -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)
}
26 changes: 26 additions & 0 deletions bundle/config/loader/entry_point_test.go
Original file line number Diff line number Diff line change
@@ -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)
}
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package mutator
package loader

import (
"context"
Expand Down
Original file line number Diff line number Diff line change
@@ -1,38 +1,35 @@
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",
},
},
}

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)
}
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package mutator
package loader

import (
"context"
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package mutator_test
package loader_test

import (
"context"
Expand All @@ -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"
Expand All @@ -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())
}

Expand All @@ -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")
}
Expand All @@ -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)
}
Expand All @@ -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)
}
Expand All @@ -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)
}
Expand All @@ -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")
}
2 changes: 2 additions & 0 deletions bundle/config/loader/testdata/databricks.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
bundle:
name: loader_test
2 changes: 2 additions & 0 deletions bundle/config/loader/testdata/host.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
workspace:
host: bar
6 changes: 5 additions & 1 deletion bundle/config/mutator/mutator.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,17 @@ 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{
loader.EntryPoint(),
loader.ProcessRootIncludes(),

// Execute preinit script after loading all configuration files.
scripts.Execute(config.ScriptPreInit),
ProcessRootIncludes(),
EnvironmentsToTargets(),
InitializeVariables(),
DefineDefaultTarget(),
Expand Down
29 changes: 29 additions & 0 deletions bundle/phases/load.go
Original file line number Diff line number Diff line change
@@ -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 {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great catch. I'm working on a change on top of this one that modifies MustConfigureBundle and TryConfigureBundle to return diagnostics as well and this is addressed there.

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)),
)
}
12 changes: 7 additions & 5 deletions bundle/tests/conflicting_resource_ids_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand All @@ -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))
Expand Down
4 changes: 2 additions & 2 deletions bundle/tests/include_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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")
}
Expand Down
8 changes: 4 additions & 4 deletions bundle/tests/loader.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,15 @@ import (

"github.com/databricks/cli/bundle"
"github.com/databricks/cli/bundle/config/mutator"
"github.com/databricks/cli/bundle/phases"
"github.com/stretchr/testify/require"
)

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
}
Expand All @@ -22,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(),
Expand Down
12 changes: 4 additions & 8 deletions bundle/tests/python_wheel_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand All @@ -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")
Expand All @@ -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()
Expand All @@ -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()
Expand Down
Loading
Loading