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

Renamed environments to targets in bundle configuration #670

Merged
merged 8 commits into from
Aug 17, 2023
Merged
Show file tree
Hide file tree
Changes from 4 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
10 changes: 5 additions & 5 deletions bundle/bundle.go
Original file line number Diff line number Diff line change
Expand Up @@ -117,10 +117,10 @@ func (b *Bundle) WorkspaceClient() *databricks.WorkspaceClient {
}

// CacheDir returns directory to use for temporary files for this bundle.
// Scoped to the bundle's environment.
// Scoped to the bundle's target.
func (b *Bundle) CacheDir(paths ...string) (string, error) {
if b.Config.Bundle.Environment == "" {
panic("environment not set")
if b.Config.Bundle.Target == "" {
panic("target not set")
}

cacheDirName, exists := os.LookupEnv("DATABRICKS_BUNDLE_TMP")
Expand All @@ -138,8 +138,8 @@ func (b *Bundle) CacheDir(paths ...string) (string, error) {
// Fixed components of the result path.
parts := []string{
cacheDirName,
// Scope with environment name.
b.Config.Bundle.Environment,
// Scope with target name.
b.Config.Bundle.Target,
}

// Append dynamic components of the result path.
Expand Down
16 changes: 8 additions & 8 deletions bundle/bundle_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,16 +31,16 @@ func TestBundleCacheDir(t *testing.T) {
bundle, err := Load(context.Background(), projectDir)
require.NoError(t, err)

// Artificially set environment.
// This is otherwise done by [mutators.SelectEnvironment].
bundle.Config.Bundle.Environment = "default"
// Artificially set target.
// This is otherwise done by [mutators.SelectTarget].
bundle.Config.Bundle.Target = "default"

// unset env variable in case it's set
t.Setenv("DATABRICKS_BUNDLE_TMP", "")

cacheDir, err := bundle.CacheDir()

// format is <CWD>/.databricks/bundle/<environment>
// format is <CWD>/.databricks/bundle/<target>
assert.NoError(t, err)
assert.Equal(t, filepath.Join(projectDir, ".databricks", "bundle", "default"), cacheDir)
}
Expand All @@ -55,16 +55,16 @@ func TestBundleCacheDirOverride(t *testing.T) {
bundle, err := Load(context.Background(), projectDir)
require.NoError(t, err)

// Artificially set environment.
// This is otherwise done by [mutators.SelectEnvironment].
bundle.Config.Bundle.Environment = "default"
// Artificially set target.
// This is otherwise done by [mutators.SelectTarget].
bundle.Config.Bundle.Target = "default"

// now we expect to use 'bundleTmpDir' instead of CWD/.databricks/bundle
t.Setenv("DATABRICKS_BUNDLE_TMP", bundleTmpDir)

cacheDir, err := bundle.CacheDir()

// format is <DATABRICKS_BUNDLE_TMP>/<environment>
// format is <DATABRICKS_BUNDLE_TMP>/<target>
assert.NoError(t, err)
assert.Equal(t, filepath.Join(bundleTmpDir, "default"), cacheDir)
}
Expand Down
9 changes: 6 additions & 3 deletions bundle/config/bundle.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,10 @@ type Bundle struct {
// Default warehouse to run SQL on.
// DefaultWarehouse string `json:"default_warehouse,omitempty"`

// Environment is set by the mutator that selects the environment.
// Target is set by the mutator that selects the target.
Target string `json:"target,omitempty" bundle:"readonly"`

// DEPRECATED. Left for backward compatibility with Target
Environment string `json:"environment,omitempty" bundle:"readonly"`

// Terraform holds configuration related to Terraform.
Expand All @@ -32,10 +35,10 @@ type Bundle struct {
// origin url. Automatically loaded by reading .git directory if not specified
Git Git `json:"git,omitempty"`

// Determines the mode of the environment.
// Determines the mode of the target.
// For example, 'mode: development' can be used for deployments for
// development purposes.
// Annotated readonly as this should be set at the environment level.
// Annotated readonly as this should be set at the target level.
Mode Mode `json:"mode,omitempty" bundle:"readonly"`

// Overrides the compute used for jobs and other supported assets.
Expand Down
37 changes: 0 additions & 37 deletions bundle/config/mutator/default_environment.go

This file was deleted.

37 changes: 37 additions & 0 deletions bundle/config/mutator/default_target.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
package mutator

import (
"context"
"fmt"

"github.com/databricks/cli/bundle"
"github.com/databricks/cli/bundle/config"
)

type defineDefaultTarget struct {
name string
}

// DefineDefaultTarget adds a target named "default"
// to the configuration if none have been defined.
func DefineDefaultTarget() bundle.Mutator {
return &defineDefaultTarget{
name: "default",
}
}

func (m *defineDefaultTarget) Name() string {
return fmt.Sprintf("DefineDefaultTarget(%s)", m.name)
}

func (m *defineDefaultTarget) Apply(_ context.Context, b *bundle.Bundle) error {
// Nothing to do if the configuration has at least 1 target.
if len(b.Config.Targets) > 0 {
return nil
}

// Define default target.
b.Config.Targets = make(map[string]*config.Target)
b.Config.Targets[m.name] = &config.Target{}
return nil
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,25 +11,25 @@ import (
"github.com/stretchr/testify/require"
)

func TestDefaultEnvironment(t *testing.T) {
func TestDefaultTarget(t *testing.T) {
bundle := &bundle.Bundle{}
err := mutator.DefineDefaultEnvironment().Apply(context.Background(), bundle)
err := mutator.DefineDefaultTarget().Apply(context.Background(), bundle)
require.NoError(t, err)
env, ok := bundle.Config.Environments["default"]
env, ok := bundle.Config.Targets["default"]
assert.True(t, ok)
assert.Equal(t, &config.Environment{}, env)
assert.Equal(t, &config.Target{}, env)
}

func TestDefaultEnvironmentAlreadySpecified(t *testing.T) {
func TestDefaultTargetAlreadySpecified(t *testing.T) {
bundle := &bundle.Bundle{
Config: config.Root{
Environments: map[string]*config.Environment{
Targets: map[string]*config.Target{
"development": {},
},
},
}
err := mutator.DefineDefaultEnvironment().Apply(context.Background(), bundle)
err := mutator.DefineDefaultTarget().Apply(context.Background(), bundle)
require.NoError(t, err)
_, ok := bundle.Config.Environments["default"]
_, ok := bundle.Config.Targets["default"]
assert.False(t, ok)
}
6 changes: 3 additions & 3 deletions bundle/config/mutator/default_workspace_root.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,14 +27,14 @@ func (m *defineDefaultWorkspaceRoot) Apply(ctx context.Context, b *bundle.Bundle
return fmt.Errorf("unable to define default workspace root: bundle name not defined")
}

if b.Config.Bundle.Environment == "" {
return fmt.Errorf("unable to define default workspace root: bundle environment not selected")
if b.Config.Bundle.Target == "" {
return fmt.Errorf("unable to define default workspace root: bundle target not selected")
}

b.Config.Workspace.RootPath = fmt.Sprintf(
"~/.bundle/%s/%s",
b.Config.Bundle.Name,
b.Config.Bundle.Environment,
b.Config.Bundle.Target,
)
return nil
}
4 changes: 2 additions & 2 deletions bundle/config/mutator/default_workspace_root_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@ func TestDefaultWorkspaceRoot(t *testing.T) {
bundle := &bundle.Bundle{
Config: config.Root{
Bundle: config.Bundle{
Name: "name",
Environment: "environment",
Name: "name",
Target: "environment",
},
},
}
Expand Down
6 changes: 3 additions & 3 deletions bundle/config/mutator/mutator.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,11 @@ import (
func DefaultMutators() []bundle.Mutator {
return []bundle.Mutator{
ProcessRootIncludes(),
DefineDefaultEnvironment(),
DefineDefaultTarget(),
LoadGitDetails(),
}
}

func DefaultMutatorsForEnvironment(env string) []bundle.Mutator {
return append(DefaultMutators(), SelectEnvironment(env))
func DefaultMutatorsForTarget(env string) []bundle.Mutator {
return append(DefaultMutators(), SelectTarget(env))
}
2 changes: 1 addition & 1 deletion bundle/config/mutator/override_compute.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ func overrideJobCompute(j *resources.Job, compute string) {
func (m *overrideCompute) Apply(ctx context.Context, b *bundle.Bundle) error {
if b.Config.Bundle.Mode != config.Development {
if b.Config.Bundle.ComputeID != "" {
return fmt.Errorf("cannot override compute for an environment that does not use 'mode: development'")
return fmt.Errorf("cannot override compute for an target that does not use 'mode: development'")
}
return nil
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,16 +13,16 @@ import (
"github.com/databricks/databricks-sdk-go/service/ml"
)

type processEnvironmentMode struct{}
type processTargetMode struct{}

const developmentConcurrentRuns = 4

func ProcessEnvironmentMode() bundle.Mutator {
return &processEnvironmentMode{}
func ProcessTargetMode() bundle.Mutator {
return &processTargetMode{}
}

func (m *processEnvironmentMode) Name() string {
return "ProcessEnvironmentMode"
func (m *processTargetMode) Name() string {
return "ProcessTargetMode"
}

// Mark all resources as being for 'development' purposes, i.e.
Expand Down Expand Up @@ -110,22 +110,22 @@ func findIncorrectPath(b *bundle.Bundle, mode config.Mode) string {

func validateProductionMode(ctx context.Context, b *bundle.Bundle, isPrincipalUsed bool) error {
if b.Config.Bundle.Git.Inferred {
env := b.Config.Bundle.Environment
return fmt.Errorf("environment with 'mode: production' must specify an explicit 'environments.%s.git' configuration", env)
env := b.Config.Bundle.Target
return fmt.Errorf("target with 'mode: production' must specify an explicit 'targets.%s.git' configuration", env)
}

r := b.Config.Resources
for i := range r.Pipelines {
if r.Pipelines[i].Development {
return fmt.Errorf("environment with 'mode: production' cannot specify a pipeline with 'development: true'")
return fmt.Errorf("target with 'mode: production' cannot specify a pipeline with 'development: true'")
}
}

if !isPrincipalUsed {
if path := findIncorrectPath(b, config.Production); path != "" {
message := "%s must not contain the current username when using 'mode: production'"
if path == "root_path" {
return fmt.Errorf(message+"\n tip: set workspace.root_path to a shared path such as /Shared/.bundle/${bundle.name}/${bundle.environment}", path)
return fmt.Errorf(message+"\n tip: set workspace.root_path to a shared path such as /Shared/.bundle/${bundle.name}/${bundle.target}", path)
} else {
return fmt.Errorf(message, path)
}
Expand Down Expand Up @@ -165,7 +165,7 @@ func isRunAsSet(r config.Resources) bool {
return true
}

func (m *processEnvironmentMode) Apply(ctx context.Context, b *bundle.Bundle) error {
func (m *processTargetMode) Apply(ctx context.Context, b *bundle.Bundle) error {
switch b.Config.Bundle.Mode {
case config.Development:
err := validateDevelopmentMode(b)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,10 +58,10 @@ func mockBundle(mode config.Mode) *bundle.Bundle {
}
}

func TestProcessEnvironmentModeDevelopment(t *testing.T) {
func TestProcessTargetModeDevelopment(t *testing.T) {
bundle := mockBundle(config.Development)

m := ProcessEnvironmentMode()
m := ProcessTargetMode()
err := m.Apply(context.Background(), bundle)
require.NoError(t, err)
assert.Equal(t, "[dev lennart] job1", bundle.Config.Resources.Jobs["job1"].Name)
Expand All @@ -73,18 +73,18 @@ func TestProcessEnvironmentModeDevelopment(t *testing.T) {
assert.True(t, bundle.Config.Resources.Pipelines["pipeline1"].PipelineSpec.Development)
}

func TestProcessEnvironmentModeDefault(t *testing.T) {
func TestProcessTargetModeDefault(t *testing.T) {
bundle := mockBundle("")

m := ProcessEnvironmentMode()
m := ProcessTargetMode()
err := m.Apply(context.Background(), bundle)
require.NoError(t, err)
assert.Equal(t, "job1", bundle.Config.Resources.Jobs["job1"].Name)
assert.Equal(t, "pipeline1", bundle.Config.Resources.Pipelines["pipeline1"].Name)
assert.False(t, bundle.Config.Resources.Pipelines["pipeline1"].PipelineSpec.Development)
}

func TestProcessEnvironmentModeProduction(t *testing.T) {
func TestProcessTargetModeProduction(t *testing.T) {
bundle := mockBundle(config.Production)

err := validateProductionMode(context.Background(), bundle, false)
Expand Down Expand Up @@ -118,7 +118,7 @@ func TestProcessEnvironmentModeProduction(t *testing.T) {
assert.False(t, bundle.Config.Resources.Pipelines["pipeline1"].PipelineSpec.Development)
}

func TestProcessEnvironmentModeProductionGit(t *testing.T) {
func TestProcessTargetModeProductionGit(t *testing.T) {
bundle := mockBundle(config.Production)

// Pretend the user didn't set Git configuration explicitly
Expand All @@ -129,10 +129,10 @@ func TestProcessEnvironmentModeProductionGit(t *testing.T) {
bundle.Config.Bundle.Git.Inferred = false
}

func TestProcessEnvironmentModeProductionOkForPrincipal(t *testing.T) {
func TestProcessTargetModeProductionOkForPrincipal(t *testing.T) {
bundle := mockBundle(config.Production)

// Our environment has all kinds of problems when not using service principals ...
// Our target has all kinds of problems when not using service principals ...
err := validateProductionMode(context.Background(), bundle, false)
require.Error(t, err)

Expand All @@ -152,7 +152,7 @@ func TestAllResourcesMocked(t *testing.T) {
assert.True(
t,
!field.IsNil() && field.Len() > 0,
"process_environment_mode should support '%s' (please add it to process_environment_mode.go and extend the test suite)",
"process_target_mode should support '%s' (please add it to process_target_mode.go and extend the test suite)",
resources.Type().Field(i).Name,
)
}
Expand All @@ -164,7 +164,7 @@ func TestAllResourcesRenamed(t *testing.T) {
bundle := mockBundle(config.Development)
resources := reflect.ValueOf(bundle.Config.Resources)

m := ProcessEnvironmentMode()
m := ProcessTargetMode()
err := m.Apply(context.Background(), bundle)
require.NoError(t, err)

Expand All @@ -179,7 +179,7 @@ func TestAllResourcesRenamed(t *testing.T) {
assert.True(
t,
strings.Contains(nameField.String(), "dev"),
"process_environment_mode should rename '%s' in '%s'",
"process_target_mode should rename '%s' in '%s'",
key,
resources.Type().Field(i).Name,
)
Expand Down
Loading