Skip to content
Merged
Show file tree
Hide file tree
Changes from 6 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
73 changes: 73 additions & 0 deletions bundle/bundle_read_only.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
package bundle

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 {
b *Bundle
}

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) 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()
}

func (r ReadOnlyBundle) CacheDir(ctx context.Context, paths ...string) (string, error) {
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)
}
Comment thread
andrewnester marked this conversation as resolved.
68 changes: 68 additions & 0 deletions bundle/config/root_read_only.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
package config

import (
"github.com/databricks/cli/bundle/config/resources"
"github.com/databricks/cli/bundle/config/variable"
"github.com/databricks/cli/libs/dyn"
"github.com/databricks/databricks-sdk-go/service/jobs"
)

type ReadOnlyConfig struct {
r Root
}

func ReadOnly(r Root) ReadOnlyConfig {
return ReadOnlyConfig{r: r}
Comment thread
andrewnester marked this conversation as resolved.
Outdated
}

func (r ReadOnlyConfig) Artifacts() Artifacts {
return r.r.Artifacts
}

func (r ReadOnlyConfig) Bundle() Bundle {
return r.r.Bundle
}

func (r ReadOnlyConfig) Environments() map[string]*Target {
return r.r.Environments
}

func (r ReadOnlyConfig) Experimental() *Experimental {
return r.r.Experimental
}

func (r ReadOnlyConfig) Include() []string {
return r.r.Include
}

func (r ReadOnlyConfig) Permissions() []resources.Permission {
return r.r.Permissions
}

func (r ReadOnlyConfig) Resources() Resources {
return r.r.Resources
}

func (r ReadOnlyConfig) Targets() map[string]*Target {
return r.r.Targets
}

func (r ReadOnlyConfig) RunAs() *jobs.JobRunAs {
return r.r.RunAs
}

func (r ReadOnlyConfig) Sync() Sync {
return r.r.Sync
}

func (r ReadOnlyConfig) Variables() map[string]*variable.Variable {
return r.r.Variables
}

func (r ReadOnlyConfig) Workspace() Workspace {
return r.r.Workspace
}

func (r ReadOnlyConfig) GetLocation(path string) dyn.Location {
return r.r.GetLocation(path)
}
54 changes: 54 additions & 0 deletions bundle/config/validate/files_to_sync.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
package validate

import (
"context"

"github.com/databricks/cli/bundle"
"github.com/databricks/cli/bundle/deploy/files"
"github.com/databricks/cli/libs/diag"
)

func FilesToSync() bundle.ReadOnlyMutator {
return &filesToSync{}
}

type filesToSync struct {
}

func (v *filesToSync) Name() string {
return "validate:files_to_sync"
}

func (v *filesToSync) Apply(ctx context.Context, rb bundle.ReadOnlyBundle) diag.Diagnostics {
sync, err := files.GetSync(ctx, rb)
if err != nil {
return diag.FromErr(err)
}

fl, err := sync.GetFileList(ctx)
if err != nil {
return diag.FromErr(err)
}

if len(fl) != 0 {
return nil
}

diags := diag.Diagnostics{}
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",
})
} else {
loc := location{path: "sync.exclude", rb: rb}
diags = diags.Append(diag.Diagnostic{
Severity: diag.Warning,
Summary: "There are no files to sync, please check your .gitignore and sync.exclude configuration",
Location: loc.Location(),
Path: loc.Path(),
})
}

return diags
}
61 changes: 61 additions & 0 deletions bundle/config/validate/job_cluster_key_defined.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
package validate

import (
"context"
"fmt"

"github.com/databricks/cli/bundle"
"github.com/databricks/cli/libs/diag"
)

func JobClusterKeyDefined() bundle.ReadOnlyMutator {
return &jobClusterKeyDefined{}
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I like this one! Question, no immediate action needed.

So we have a few more cases of references throughout our APIs. I suspect the ones that customers would hit most often are job_cluster_key, then task_key, and then some small long tails including the new environment_key. Makes me wonder how far we should go with these checks? And maybe whether it's worthwhile making this pattern more generic?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

If we are able to define to which fields attributes like task_key, job_cluster_key and etc are referencing to in some general way (like key value map of config path and etc.) we can make these generic. But I like the very explicit nature of it and would rather prefer add separate explicit mutator for each type of these checks


type jobClusterKeyDefined struct {
}

func (v *jobClusterKeyDefined) Name() string {
return "validate:job_cluster_key_defined"
}

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 != "" {
Comment thread
andrewnester marked this conversation as resolved.
Outdated
jobClusterKeys[task.JobClusterKey] = location{
path: fmt.Sprintf("resources.jobs.%s.tasks[%d].job_cluster_key", k, index),
rb: rb,
}
}
}

// 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(),
})
}
}
Comment thread
andrewnester marked this conversation as resolved.
}

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 {
Comment thread
andrewnester marked this conversation as resolved.
Outdated
return true
}
}
}
return false
}
43 changes: 43 additions & 0 deletions bundle/config/validate/validate.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
package validate

import (
"context"

"github.com/databricks/cli/bundle"
"github.com/databricks/cli/libs/diag"
"github.com/databricks/cli/libs/dyn"
)

type validate struct {
}

type location struct {
path string
rb bundle.ReadOnlyBundle
}

func (l location) Location() dyn.Location {
return l.rb.Config().GetLocation(l.path)
}

func (l location) Path() dyn.Path {
return dyn.MustPathFromString(l.path)
}

// Apply implements bundle.Mutator.
func (v *validate) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics {
return bundle.ApplyReadOnly(ctx, bundle.ReadOnly(b), bundle.Parallel(
JobClusterKeyDefined(),
FilesToSync(),
ValidateSyncPatterns(),
))
}

// Name implements bundle.Mutator.
func (v *validate) Name() string {
return "validate"
}

func Validate() bundle.Mutator {
return &validate{}
}
79 changes: 79 additions & 0 deletions bundle/config/validate/validate_sync_patterns.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
package validate

import (
"context"
"fmt"
"sync"

"github.com/databricks/cli/bundle"
"github.com/databricks/cli/libs/diag"
"github.com/databricks/cli/libs/fileset"
"golang.org/x/sync/errgroup"
)

func ValidateSyncPatterns() bundle.ReadOnlyMutator {
return &validateSyncPatterns{}
}

type validateSyncPatterns struct {
}

func (v *validateSyncPatterns) Name() string {
return "validate:validate_sync_patterns"
}

func (v *validateSyncPatterns) Apply(ctx context.Context, rb bundle.ReadOnlyBundle) diag.Diagnostics {
s := rb.Config().Sync()
if len(s.Exclude) == 0 && len(s.Include) == 0 {
return nil
}

diags, err := checkPatterns(s.Exclude, "sync.exclude", rb)
if err != nil {
return diag.FromErr(err)
}

includeDiags, err := checkPatterns(s.Include, "sync.include", rb)
if err != nil {
return diag.FromErr(err)
}

return diags.Extend(includeDiags)
}

func checkPatterns(patterns []string, path string, rb bundle.ReadOnlyBundle) (diag.Diagnostics, error) {
var mu sync.Mutex
var errs errgroup.Group
var diags diag.Diagnostics

for i, pattern := range patterns {
index := i
p := pattern
errs.Go(func() error {
fs, err := fileset.NewGlobSet(rb.RootPath(), []string{p})
if err != nil {
return err
}

all, err := fs.All()
if err != nil {
return err
}

if len(all) == 0 {
loc := location{path: fmt.Sprintf("%s[%d]", path, index), rb: rb}
mu.Lock()
diags = diags.Append(diag.Diagnostic{
Severity: diag.Warning,
Summary: fmt.Sprintf("Pattern %s does not match any files", p),
Location: loc.Location(),
Path: loc.Path(),
})
mu.Unlock()
}
return nil
})
}

return diags, errs.Wait()
}
2 changes: 1 addition & 1 deletion bundle/deploy/files/delete.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ func (m *delete) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics {
}

// Clean up sync snapshot file
sync, err := GetSync(ctx, b)
sync, err := GetSync(ctx, bundle.ReadOnly(b))
if err != nil {
return diag.FromErr(err)
}
Expand Down
Loading