Skip to content

Commit

Permalink
Report all empty resources present in error diagnostic (#1685)
Browse files Browse the repository at this point in the history
## Changes
This PR addressed post-merge feedback from
#1673.

## Tests
Unit tests, and manually.
```
Error: experiment undefined-experiment is not defined
  at resources.experiments.undefined-experiment
  in databricks.yml:11:26

Error: job undefined-job is not defined
  at resources.jobs.undefined-job
  in databricks.yml:6:19

Error: pipeline undefined-pipeline is not defined
  at resources.pipelines.undefined-pipeline
  in databricks.yml:14:24

Name: undefined-job
Target: default

Found 3 errors
```
  • Loading branch information
shreyas-goenka authored Aug 20, 2024
1 parent 78d0ac5 commit 242d4b5
Show file tree
Hide file tree
Showing 7 changed files with 90 additions and 63 deletions.
42 changes: 26 additions & 16 deletions bundle/config/validate/all_resources_have_values.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package validate
import (
"context"
"fmt"
"slices"
"strings"

"github.com/databricks/cli/bundle"
Expand All @@ -21,27 +22,36 @@ func (m *allResourcesHaveValues) Name() string {
}

func (m *allResourcesHaveValues) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics {
rv := b.Config.Value().Get("resources")

// Skip if there are no resources block defined, or the resources block is empty.
if rv.Kind() == dyn.KindInvalid || rv.Kind() == dyn.KindNil {
return nil
}
diags := diag.Diagnostics{}

_, err := dyn.MapByPattern(
rv,
dyn.NewPattern(dyn.AnyKey(), dyn.AnyKey()),
b.Config.Value(),
dyn.NewPattern(dyn.Key("resources"), dyn.AnyKey(), dyn.AnyKey()),
func(p dyn.Path, v dyn.Value) (dyn.Value, error) {
if v.Kind() == dyn.KindInvalid || v.Kind() == dyn.KindNil {
// Type of the resource, stripped of the trailing 's' to make it
// singular.
rType := strings.TrimSuffix(p[0].Key(), "s")

rName := p[1].Key()
return v, fmt.Errorf("%s %s is not defined", rType, rName)
if v.Kind() != dyn.KindNil {
return v, nil
}

// Type of the resource, stripped of the trailing 's' to make it
// singular.
rType := strings.TrimSuffix(p[1].Key(), "s")

// Name of the resource. Eg: "foo" in "jobs.foo".
rName := p[2].Key()

diags = append(diags, diag.Diagnostic{
Severity: diag.Error,
Summary: fmt.Sprintf("%s %s is not defined", rType, rName),
Locations: v.Locations(),
Paths: []dyn.Path{slices.Clone(p)},
})

return v, nil
},
)
return diag.FromErr(err)
if err != nil {
diags = append(diags, diag.FromErr(err)...)
}

return diags
}
9 changes: 0 additions & 9 deletions bundle/tests/environments_job_and_pipeline_test.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package config_tests

import (
"path/filepath"
"testing"

"github.com/databricks/cli/bundle/config"
Expand All @@ -15,8 +14,6 @@ func TestJobAndPipelineDevelopmentWithEnvironment(t *testing.T) {
assert.Len(t, b.Config.Resources.Pipelines, 1)

p := b.Config.Resources.Pipelines["nyc_taxi_pipeline"]
l := b.Config.GetLocation("resources.pipelines.nyc_taxi_pipeline")
assert.Equal(t, "environments_job_and_pipeline/databricks.yml", filepath.ToSlash(l.File))
assert.Equal(t, b.Config.Bundle.Mode, config.Development)
assert.True(t, p.Development)
require.Len(t, p.Libraries, 1)
Expand All @@ -30,8 +27,6 @@ func TestJobAndPipelineStagingWithEnvironment(t *testing.T) {
assert.Len(t, b.Config.Resources.Pipelines, 1)

p := b.Config.Resources.Pipelines["nyc_taxi_pipeline"]
l := b.Config.GetLocation("resources.pipelines.nyc_taxi_pipeline")
assert.Equal(t, "environments_job_and_pipeline/databricks.yml", filepath.ToSlash(l.File))
assert.False(t, p.Development)
require.Len(t, p.Libraries, 1)
assert.Equal(t, "./dlt/nyc_taxi_loader", p.Libraries[0].Notebook.Path)
Expand All @@ -44,16 +39,12 @@ func TestJobAndPipelineProductionWithEnvironment(t *testing.T) {
assert.Len(t, b.Config.Resources.Pipelines, 1)

p := b.Config.Resources.Pipelines["nyc_taxi_pipeline"]
pl := b.Config.GetLocation("resources.pipelines.nyc_taxi_pipeline")
assert.Equal(t, "environments_job_and_pipeline/databricks.yml", filepath.ToSlash(pl.File))
assert.False(t, p.Development)
require.Len(t, p.Libraries, 1)
assert.Equal(t, "./dlt/nyc_taxi_loader", p.Libraries[0].Notebook.Path)
assert.Equal(t, "nyc_taxi_production", p.Target)

j := b.Config.Resources.Jobs["pipeline_schedule"]
jl := b.Config.GetLocation("resources.jobs.pipeline_schedule")
assert.Equal(t, "environments_job_and_pipeline/databricks.yml", filepath.ToSlash(jl.File))
assert.Equal(t, "Daily refresh of production pipeline", j.Name)
require.Len(t, j.Tasks, 1)
assert.NotEmpty(t, j.Tasks[0].PipelineTask.PipelineId)
Expand Down
8 changes: 0 additions & 8 deletions bundle/tests/undefined_job/databricks.yml

This file was deleted.

22 changes: 0 additions & 22 deletions bundle/tests/undefined_job_test.go

This file was deleted.

8 changes: 0 additions & 8 deletions bundle/tests/undefined_pipeline/databricks.yml

This file was deleted.

14 changes: 14 additions & 0 deletions bundle/tests/undefined_resources/databricks.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
bundle:
name: undefined-job

resources:
jobs:
undefined-job:
test:
name: "Test Job"

experiments:
undefined-experiment:

pipelines:
undefined-pipeline:
50 changes: 50 additions & 0 deletions bundle/tests/undefined_resources_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
package config_tests

import (
"context"
"path/filepath"
"testing"

"github.com/databricks/cli/bundle"
"github.com/databricks/cli/bundle/config/validate"
"github.com/databricks/cli/libs/diag"
"github.com/databricks/cli/libs/dyn"
"github.com/stretchr/testify/assert"
)

func TestUndefinedResourcesLoadWithError(t *testing.T) {
b := load(t, "./undefined_resources")
diags := bundle.Apply(context.Background(), b, validate.AllResourcesHaveValues())

assert.Len(t, diags, 3)
assert.Contains(t, diags, diag.Diagnostic{
Severity: diag.Error,
Summary: "job undefined-job is not defined",
Locations: []dyn.Location{{
File: filepath.FromSlash("undefined_resources/databricks.yml"),
Line: 6,
Column: 19,
}},
Paths: []dyn.Path{dyn.MustPathFromString("resources.jobs.undefined-job")},
})
assert.Contains(t, diags, diag.Diagnostic{
Severity: diag.Error,
Summary: "experiment undefined-experiment is not defined",
Locations: []dyn.Location{{
File: filepath.FromSlash("undefined_resources/databricks.yml"),
Line: 11,
Column: 26,
}},
Paths: []dyn.Path{dyn.MustPathFromString("resources.experiments.undefined-experiment")},
})
assert.Contains(t, diags, diag.Diagnostic{
Severity: diag.Error,
Summary: "pipeline undefined-pipeline is not defined",
Locations: []dyn.Location{{
File: filepath.FromSlash("undefined_resources/databricks.yml"),
Line: 14,
Column: 24,
}},
Paths: []dyn.Path{dyn.MustPathFromString("resources.pipelines.undefined-pipeline")},
})
}

0 comments on commit 242d4b5

Please sign in to comment.