Skip to content

Commit f35452b

Browse files
denikIsaac
andcommitted
Read lifecycle.depends_on from typed config instead of dyn
Co-authored-by: Isaac <no-reply@databricks.com>
1 parent 496252a commit f35452b

8 files changed

Lines changed: 100 additions & 58 deletions

File tree

‎acceptance/bundle/lifecycle/depends-on-validation/databricks.yml‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,3 +14,12 @@ resources:
1414
lifecycle:
1515
depends_on:
1616
- resource: a.b.c
17+
# lifecycle is a separate type on job_runs.
18+
job_runs:
19+
a_run:
20+
job_id: ${resources.jobs.b_job.id}
21+
lifecycle:
22+
triggers:
23+
- on_bundle_deploy: true
24+
depends_on:
25+
- resource: missing_for_run
Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,23 @@
1+
Error: invalid lifecycle.depends_on: no such resource: missing_for_run
2+
at resources.job_runs.a_run.lifecycle.depends_on[0].resource
3+
in databricks.yml:25:23
4+
15
Error: invalid lifecycle.depends_on: resource "a_job" cannot depend on itself
26
at resources.jobs.a_job.lifecycle.depends_on[0].resource
3-
in databricks.yml:10:13
7+
in databricks.yml:10:23
48

59
Error: invalid lifecycle.depends_on: no such resource: missing
610
at resources.jobs.a_job.lifecycle.depends_on[1].resource
7-
in databricks.yml:11:13
11+
in databricks.yml:11:23
812

913
Error: invalid lifecycle.depends_on: no such resource: a.b.c
1014
at resources.jobs.b_job.lifecycle.depends_on[0].resource
11-
in databricks.yml:16:13
15+
in databricks.yml:16:23
1216

1317
Name: test-bundle
1418
Target: default
1519
Workspace:
1620
User: [USERNAME]
1721
Path: /Workspace/Users/[USERNAME]/.bundle/test-bundle/default
1822

19-
Found 3 errors
23+
Found 4 errors

‎bundle/config/resources/job_run.go‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,15 @@ func (r *JobRun) ResourceDescription() ResourceDescription {
123123
}
124124
}
125125

126+
// GetLifecycle returns the lifecycle settings. Lifecycle shadows BaseResource.Lifecycle,
127+
// which therefore stays empty.
128+
func (r *JobRun) GetLifecycle() LifecycleConfig {
129+
if r.Lifecycle == nil {
130+
return Lifecycle{}
131+
}
132+
return r.Lifecycle.Lifecycle
133+
}
134+
126135
// GetName returns the in-product name, which is empty: a run has no name.
127136
func (r *JobRun) GetName() string {
128137
return ""

‎bundle/config/resources/lifecycle.go‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package resources
33
// LifecycleConfig is implemented by Lifecycle and LifecycleWithStarted.
44
type LifecycleConfig interface {
55
HasPreventDestroy() bool
6+
GetDependsOn() []DependsOn
67
}
78

89
// Lifecycle contains base lifecycle settings supported by all resources.
@@ -31,6 +32,11 @@ func (l Lifecycle) HasPreventDestroy() bool {
3132
return l.PreventDestroy
3233
}
3334

35+
// GetDependsOn returns the lifecycle.depends_on entries.
36+
func (l Lifecycle) GetDependsOn() []DependsOn {
37+
return l.DependsOn
38+
}
39+
3440
// LifecycleWithStarted contains lifecycle settings for resources that support lifecycle.started.
3541
// It is used by apps, clusters, and sql_warehouses.
3642
type LifecycleWithStarted struct {
Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
package resources
2+
3+
import (
4+
"testing"
5+
6+
"github.com/stretchr/testify/assert"
7+
)
8+
9+
// Resources that shadow BaseResource.Lifecycle must return their own settings.
10+
func TestGetLifecycleDependsOn(t *testing.T) {
11+
deps := []DependsOn{{Resource: "foo"}}
12+
13+
tests := []struct {
14+
name string
15+
resource interface{ GetLifecycle() LifecycleConfig }
16+
}{
17+
{"base", &Job{Lifecycle: Lifecycle{DependsOn: deps}}},
18+
{"with_started", &App{Lifecycle: &LifecycleWithStarted{DependsOn: deps}}},
19+
{"job_run", &JobRun{Lifecycle: &JobRunLifecycle{DependsOn: deps}}},
20+
}
21+
22+
for _, tc := range tests {
23+
t.Run(tc.name, func(t *testing.T) {
24+
assert.Equal(t, deps, tc.resource.GetLifecycle().GetDependsOn())
25+
})
26+
}
27+
}
28+
29+
func TestJobRunGetLifecycleNil(t *testing.T) {
30+
assert.Empty(t, (&JobRun{}).GetLifecycle().GetDependsOn())
31+
}

‎bundle/config/validate/lifecycle_depends_on.go‎

Lines changed: 24 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,12 @@ import (
44
"cmp"
55
"context"
66
"fmt"
7+
"reflect"
78
"slices"
89

910
"github.com/databricks/cli/bundle"
1011
"github.com/databricks/cli/libs/diag"
11-
"github.com/databricks/cli/libs/dyn"
12+
"github.com/databricks/cli/libs/structs/structpath"
1213
)
1314

1415
type lifecycleDependsOn struct{}
@@ -26,37 +27,33 @@ func (*lifecycleDependsOn) Name() string {
2627
func (*lifecycleDependsOn) Apply(_ context.Context, b *bundle.Bundle) diag.Diagnostics {
2728
var diags diag.Diagnostics
2829

29-
pattern := dyn.NewPattern(dyn.Key("resources"), dyn.AnyKey(), dyn.AnyKey(), dyn.Key("lifecycle"), dyn.Key("depends_on"))
30-
_, err := dyn.MapByPattern(b.Config.Value(), pattern, func(p dyn.Path, v dyn.Value) (dyn.Value, error) {
31-
seq, ok := v.AsSequence()
32-
if !ok {
33-
return v, nil
34-
}
35-
self := p[:3].String()
36-
for i, item := range seq {
37-
ref, ok := item.Get("resource").AsString()
38-
if !ok {
30+
for _, group := range b.Config.Resources.AllResources() {
31+
for key, resource := range group.Resources {
32+
// Python-generated configs can contain null resource entries.
33+
if reflect.ValueOf(resource).IsNil() {
3934
continue
4035
}
41-
res, key, err := b.Config.Resources.ResolveKey(ref)
42-
if err == nil && "resources."+res.ResourceDescription().PluralName+"."+key == self {
43-
err = fmt.Errorf("resource %q cannot depend on itself", ref)
44-
}
45-
if err != nil {
46-
diags = diags.Append(diag.Diagnostic{
47-
Severity: diag.Error,
48-
Summary: "invalid lifecycle.depends_on: " + err.Error(),
49-
Locations: item.Locations(),
50-
Paths: dyn.ToStructPaths(p.Append(dyn.Index(i), dyn.Key("resource"))),
51-
})
36+
37+
self := group.Description.PluralName + "." + key
38+
for i, dep := range resource.GetLifecycle().GetDependsOn() {
39+
target, targetKey, err := b.Config.Resources.ResolveKey(dep.Resource)
40+
if err == nil && target.ResourceDescription().PluralName+"."+targetKey == self {
41+
err = fmt.Errorf("resource %q cannot depend on itself", dep.Resource)
42+
}
43+
if err != nil {
44+
path := structpath.NewPath(nil, "resources", group.Description.PluralName, key, "lifecycle", "depends_on", i, "resource")
45+
diags = diags.Append(diag.Diagnostic{
46+
Severity: diag.Error,
47+
Summary: "invalid lifecycle.depends_on: " + err.Error(),
48+
Locations: b.Config.GetLocationsOf(path),
49+
Paths: []*structpath.PathNode{path},
50+
})
51+
}
5252
}
5353
}
54-
return v, nil
55-
})
56-
if err != nil {
57-
diags = diags.Extend(diag.FromErr(err))
5854
}
59-
// MapByPattern visits resources in random order.
55+
56+
// Resources are visited in random order.
6057
slices.SortStableFunc(diags, func(a, b diag.Diagnostic) int {
6158
return cmp.Compare(a.Paths[0].String(), b.Paths[0].String())
6259
})

‎bundle/direct/bundle_plan.go‎

Lines changed: 12 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import (
1111
"strings"
1212

1313
"github.com/databricks/cli/bundle/config"
14+
"github.com/databricks/cli/bundle/config/resources"
1415
"github.com/databricks/cli/bundle/deployplan"
1516
"github.com/databricks/cli/bundle/direct/dresources"
1617
"github.com/databricks/cli/bundle/direct/dstate"
@@ -1285,7 +1286,7 @@ func (b *DeploymentBundle) makePlan(ctx context.Context, configRoot *config.Root
12851286
}
12861287
}
12871288

1288-
lifecycleDeps, err := LifecycleDependsOn(configRoot, node)
1289+
lifecycleDeps, err := LifecycleDependsOn(configRoot, inputConfig)
12891290
if err != nil {
12901291
return nil, fmt.Errorf("%s: %w", prefix, err)
12911292
}
@@ -1450,35 +1451,20 @@ func (b *DeploymentBundle) getAdapterForKey(resourceKey string) (*dresources.Ada
14501451

14511452
const lifecycleDependsOnLabel = "depends_on"
14521453

1453-
// LifecycleDependsOn returns the edges declared in lifecycle.depends_on of the given node.
1454-
func LifecycleDependsOn(configRoot *config.Root, node string) ([]deployplan.DependsOnEntry, error) {
1455-
nodePath, err := dyn.NewPathFromString(node)
1456-
if err != nil {
1457-
return nil, err
1458-
}
1459-
// permissions and grants are separate nodes without lifecycle.
1460-
if len(nodePath) != 3 {
1461-
return nil, nil
1462-
}
1463-
entries, err := dyn.GetByPath(configRoot.Value(), nodePath.Append(dyn.Key("lifecycle"), dyn.Key("depends_on")))
1464-
if dyn.IsNoSuchKeyError(err) {
1465-
return nil, nil
1466-
}
1467-
if err != nil {
1468-
return nil, err
1469-
}
1470-
seq, isSeq := entries.AsSequence()
1471-
if !isSeq {
1454+
// LifecycleDependsOn returns the edges declared in lifecycle.depends_on of a resource,
1455+
// given its config as returned by [config.Root.GetResourceConfig].
1456+
func LifecycleDependsOn(configRoot *config.Root, inputConfig any) ([]deployplan.DependsOnEntry, error) {
1457+
// Not every node has a lifecycle, e.g. permissions and grants.
1458+
resource, ok := inputConfig.(interface {
1459+
GetLifecycle() resources.LifecycleConfig
1460+
})
1461+
if !ok {
14721462
return nil, nil
14731463
}
14741464

14751465
var result []deployplan.DependsOnEntry
1476-
for _, item := range seq {
1477-
ref, isStr := item.Get("resource").AsString()
1478-
if !isStr {
1479-
continue
1480-
}
1481-
res, key, err := configRoot.Resources.ResolveKey(ref)
1466+
for _, dep := range resource.GetLifecycle().GetDependsOn() {
1467+
res, key, err := configRoot.Resources.ResolveKey(dep.Resource)
14821468
if err != nil {
14831469
return nil, fmt.Errorf("lifecycle.depends_on: %w", err)
14841470
}

‎bundle/migrate/build_state.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -152,7 +152,7 @@ func BuildStateFromTF(
152152
}
153153
}
154154
}
155-
lifecycleDeps, err := direct.LifecycleDependsOn(configRoot, node)
155+
lifecycleDeps, err := direct.LifecycleDependsOn(configRoot, inputConfig)
156156
if err != nil {
157157
return warningsSeen, fmt.Errorf("%s: %w", node, err)
158158
}

0 commit comments

Comments
 (0)