Skip to content

Commit

Permalink
fixes
Browse files Browse the repository at this point in the history
  • Loading branch information
andrewnester committed Apr 18, 2024
1 parent 85dd7e6 commit 40d451f
Show file tree
Hide file tree
Showing 8 changed files with 126 additions and 142 deletions.
41 changes: 2 additions & 39 deletions bundle/bundle_read_only.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,7 @@ import (
"context"

"github.com/databricks/cli/bundle/config"
"github.com/databricks/cli/bundle/metadata"
"github.com/databricks/cli/libs/locker"
"github.com/databricks/cli/libs/tags"
"github.com/databricks/cli/libs/terraform"
"github.com/databricks/databricks-sdk-go"
"github.com/hashicorp/terraform-exec/tfexec"
)

type ReadOnlyBundle struct {
Expand All @@ -20,38 +15,14 @@ func ReadOnly(b *Bundle) ReadOnlyBundle {
return ReadOnlyBundle{b: b}
}

func (r ReadOnlyBundle) Config() config.ReadOnlyConfig {
return config.ReadOnly(r.b.Config)
}

func (r ReadOnlyBundle) AutoApprove() bool {
return r.b.AutoApprove
}

func (r ReadOnlyBundle) Locker() *locker.Locker {
return r.b.Locker
}

func (r ReadOnlyBundle) Metadata() metadata.Metadata {
return r.b.Metadata
}

func (r ReadOnlyBundle) Plan() *terraform.Plan {
return r.b.Plan
func (r ReadOnlyBundle) Config() config.Root {
return r.b.Config
}

func (r ReadOnlyBundle) RootPath() string {
return r.b.RootPath
}

func (r ReadOnlyBundle) Tagging() tags.Cloud {
return r.b.Tagging
}

func (r ReadOnlyBundle) Terraform() *tfexec.Terraform {
return r.b.Terraform
}

func (r ReadOnlyBundle) WorkspaceClient() *databricks.WorkspaceClient {
return r.b.WorkspaceClient()
}
Expand All @@ -60,14 +31,6 @@ func (r ReadOnlyBundle) CacheDir(ctx context.Context, paths ...string) (string,
return r.b.CacheDir(ctx, paths...)
}

func (r ReadOnlyBundle) InternalDir(ctx context.Context) (string, error) {
return r.b.InternalDir(ctx)
}

func (r ReadOnlyBundle) SetWorkpaceClient(w *databricks.WorkspaceClient) {
r.b.SetWorkpaceClient(w)
}

func (r ReadOnlyBundle) GetSyncIncludePatterns(ctx context.Context) ([]string, error) {
return r.b.GetSyncIncludePatterns(ctx)
}
2 changes: 1 addition & 1 deletion bundle/config/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -440,7 +440,7 @@ func validateVariableOverrides(root, target dyn.Value) (err error) {
// Best effort to get the location of configuration value at the specified path.
// This function is useful to annotate error messages with the location, because
// we don't want to fail with a different error message if we cannot retrieve the location.
func (r *Root) GetLocation(path string) dyn.Location {
func (r Root) GetLocation(path string) dyn.Location {
v, err := dyn.Get(r.value, path)
if err != nil {
return dyn.Location{}
Expand Down
68 changes: 0 additions & 68 deletions bundle/config/root_read_only.go

This file was deleted.

2 changes: 1 addition & 1 deletion bundle/config/validate/files_to_sync.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ func (v *filesToSync) Apply(ctx context.Context, rb bundle.ReadOnlyBundle) diag.
}

diags := diag.Diagnostics{}
if len(rb.Config().Sync().Exclude) == 0 {
if len(rb.Config().Sync.Exclude) == 0 {
diags = diags.Append(diag.Diagnostic{
Severity: diag.Warning,
Summary: "There are no files to sync, please check your .gitignore",
Expand Down
48 changes: 20 additions & 28 deletions bundle/config/validate/job_cluster_key_defined.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,40 +22,32 @@ func (v *jobClusterKeyDefined) Name() string {
func (v *jobClusterKeyDefined) Apply(ctx context.Context, rb bundle.ReadOnlyBundle) diag.Diagnostics {
diags := diag.Diagnostics{}

for k, job := range rb.Config().Resources().Jobs {
jobClusterKeys := make(map[string]location)
for index, task := range job.Tasks {
if task.JobClusterKey != "" {
jobClusterKeys[task.JobClusterKey] = location{
path: fmt.Sprintf("resources.jobs.%s.tasks[%d].job_cluster_key", k, index),
rb: rb,
}
for k, job := range rb.Config().Resources.Jobs {
jobClusterKeys := make(map[string]bool)
for _, cluster := range job.JobClusters {
if cluster.JobClusterKey != "" {
jobClusterKeys[cluster.JobClusterKey] = true
}
}

// Check if all job_cluster_keys are defined
for key, loc := range jobClusterKeys {
if !isJobClusterKeyDefined(key, rb) {
diags = diags.Append(diag.Diagnostic{
Severity: diag.Warning,
Summary: fmt.Sprintf("job_cluster_key %s is not defined", key),
Location: loc.Location(),
Path: loc.Path(),
})
for index, task := range job.Tasks {
if task.JobClusterKey != "" {
if _, ok := jobClusterKeys[task.JobClusterKey]; !ok {

This comment has been minimized.

Copy link
@pietern

pietern Apr 18, 2024

Contributor

Can test for ok and then continue to reduce nesting.

loc := location{
path: fmt.Sprintf("resources.jobs.%s.tasks[%d].job_cluster_key", k, index),
rb: rb,
}

diags = diags.Append(diag.Diagnostic{
Severity: diag.Warning,
Summary: fmt.Sprintf("job_cluster_key %s is not defined", task.JobClusterKey),
Location: loc.Location(),
Path: loc.Path(),
})
}
}
}
}

return diags
}

func isJobClusterKeyDefined(key string, rb bundle.ReadOnlyBundle) bool {
for _, job := range rb.Config().Resources().Jobs {
for _, cluster := range job.JobClusters {
if cluster.JobClusterKey == key {
return true
}
}
}
return false
}
97 changes: 97 additions & 0 deletions bundle/config/validate/job_cluster_key_defined_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
package validate

import (
"context"
"testing"

"github.com/databricks/cli/bundle"
"github.com/databricks/cli/bundle/config"
"github.com/databricks/cli/bundle/config/resources"
"github.com/databricks/cli/libs/diag"
"github.com/databricks/databricks-sdk-go/service/jobs"
"github.com/stretchr/testify/require"
)

func TestJobClusterKeyDefined(t *testing.T) {
b := &bundle.Bundle{
Config: config.Root{
Resources: config.Resources{
Jobs: map[string]*resources.Job{
"job1": {
JobSettings: &jobs.JobSettings{
Name: "job1",
JobClusters: []jobs.JobCluster{
{JobClusterKey: "do-not-exist"},
},
Tasks: []jobs.Task{
{JobClusterKey: "do-not-exist"},
},
},
},
},
},
},
}

diags := bundle.ApplyReadOnly(context.Background(), bundle.ReadOnly(b), JobClusterKeyDefined())
require.Len(t, diags, 0)
require.NoError(t, diags.Error())
}

func TestJobClusterKeyNotDefined(t *testing.T) {
b := &bundle.Bundle{
Config: config.Root{
Resources: config.Resources{
Jobs: map[string]*resources.Job{
"job1": {
JobSettings: &jobs.JobSettings{
Name: "job1",
Tasks: []jobs.Task{
{JobClusterKey: "do-not-exist"},
},
},
},
},
},
},
}

diags := bundle.ApplyReadOnly(context.Background(), bundle.ReadOnly(b), JobClusterKeyDefined())
require.Len(t, diags, 1)
require.NoError(t, diags.Error())
require.Equal(t, diags[0].Severity, diag.Warning)
require.Equal(t, diags[0].Summary, "job_cluster_key do-not-exist is not defined")
}

func TestJobClusterKeyDefinedInDifferentJob(t *testing.T) {
b := &bundle.Bundle{
Config: config.Root{
Resources: config.Resources{
Jobs: map[string]*resources.Job{
"job1": {
JobSettings: &jobs.JobSettings{
Name: "job1",
Tasks: []jobs.Task{
{JobClusterKey: "do-not-exist"},
},
},
},
"job2": {
JobSettings: &jobs.JobSettings{
Name: "job2",
JobClusters: []jobs.JobCluster{
{JobClusterKey: "do-not-exist"},
},
},
},
},
},
},
}

diags := bundle.ApplyReadOnly(context.Background(), bundle.ReadOnly(b), JobClusterKeyDefined())
require.Len(t, diags, 1)
require.NoError(t, diags.Error())
require.Equal(t, diags[0].Severity, diag.Warning)
require.Equal(t, diags[0].Summary, "job_cluster_key do-not-exist is not defined")
}
2 changes: 1 addition & 1 deletion bundle/config/validate/validate_sync_patterns.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ func (v *validateSyncPatterns) Name() string {
}

func (v *validateSyncPatterns) Apply(ctx context.Context, rb bundle.ReadOnlyBundle) diag.Diagnostics {
s := rb.Config().Sync()
s := rb.Config().Sync
if len(s.Exclude) == 0 && len(s.Include) == 0 {
return nil
}
Expand Down
8 changes: 4 additions & 4 deletions bundle/deploy/files/sync.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,9 @@ func GetSyncOptions(ctx context.Context, rb bundle.ReadOnlyBundle) (*sync.SyncOp

opts := &sync.SyncOptions{
LocalPath: rb.RootPath(),
RemotePath: rb.Config().Workspace().FilePath,
RemotePath: rb.Config().Workspace.FilePath,
Include: includes,
Exclude: rb.Config().Sync().Exclude,
Exclude: rb.Config().Sync.Exclude,
Host: rb.WorkspaceClient().Config.Host,

Full: false,
Expand All @@ -40,8 +40,8 @@ func GetSyncOptions(ctx context.Context, rb bundle.ReadOnlyBundle) (*sync.SyncOp
WorkspaceClient: rb.WorkspaceClient(),
}

if rb.Config().Workspace().CurrentUser != nil {
opts.CurrentUser = rb.Config().Workspace().CurrentUser.User
if rb.Config().Workspace.CurrentUser != nil {
opts.CurrentUser = rb.Config().Workspace.CurrentUser.User
}

return opts, nil
Expand Down

0 comments on commit 40d451f

Please sign in to comment.