Skip to content

Commit cc76ef2

Browse files
authored
direct: add FieldTriggers(); remove ClassifyChanges(), RecreateFields() (#3595)
## Changes New method FieldTriggers(), which is a more powerful version of RecreateFields() that allows associate any action with a field. Builds on top of #3585 ## Why This covers existing classification use cases without custom code. We might still need ClassifyChange function in the future, but exact types involved will likely change by that time (with regard to serialized plan work). ## Tests Existing tests.
1 parent eeb389a commit cc76ef2

File tree

7 files changed

+77
-99
lines changed

7 files changed

+77
-99
lines changed

bundle/terranova/plan.go

Lines changed: 4 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -207,22 +207,11 @@ func calcDiff(adapter *tnresources.Adapter, savedState, config any) (deployplan.
207207
return deployplan.ActionTypeNoop, nil
208208
}
209209

210-
if adapter.MustRecreate(localDiff) {
211-
return deployplan.ActionTypeRecreate, nil
212-
}
213-
214-
if adapter.HasClassifyChanges() {
215-
result, err := adapter.ClassifyChanges(localDiff)
216-
if err != nil {
217-
return deployplan.ActionTypeUnset, err
218-
}
219-
220-
if result == deployplan.ActionTypeUpdateWithID && !adapter.HasDoUpdateWithID() {
221-
return deployplan.ActionTypeUnset, errors.New("internal error: unexpected plan='update_with_id'")
222-
}
210+
result := adapter.ClassifyByTriggers(localDiff)
223211

224-
return result, nil
212+
if result == deployplan.ActionTypeUpdateWithID && !adapter.HasDoUpdateWithID() {
213+
return deployplan.ActionTypeUnset, errors.New("internal error: unexpected plan='update_with_id'")
225214
}
226215

227-
return deployplan.ActionTypeUpdate, nil
216+
return result, nil
228217
}

bundle/terranova/tnresources/adapter.go

Lines changed: 44 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -34,14 +34,10 @@ type IResource interface {
3434
// Example: func (r *ResourceJob) DoDelete(ctx context.Context, id string) error {
3535
DoDelete(ctx context.Context, id string) error
3636

37-
// [Optional] RecreateFields returns a list of fields that will cause resource recreation if changed
38-
// Example: func (r *ResourceJob) RecreateFields() []string { return []string{"name", "type"} }
39-
RecreateFields() []string
40-
41-
// [Optional] ClassifyChanges provides non-default change classification.
42-
// Default is to consider any change "an update" (RecreateFields handled separately).
43-
// Example: func (r *ResourceJob) ClassifyChanges(changes []structdiff.Change) deployplan.ActionType { return deployplan.ActionUpdate }
44-
ClassifyChanges(changes []structdiff.Change) deployplan.ActionType
37+
// [Optional] FieldTriggers returns actions to trigger when given fields are changed.
38+
// Keys are field paths (e.g., ".name", ".catalog_name"). Values are actions.
39+
// Unspecified changed fields default to ActionTypeUpdate.
40+
FieldTriggers() map[string]deployplan.ActionType
4541
}
4642

4743
// IResourceNoRefresh describes additional methods for resource to implement.
@@ -108,9 +104,8 @@ type Adapter struct {
108104
doUpdateWithID *calladapt.BoundCaller
109105
waitAfterCreate *calladapt.BoundCaller
110106
waitAfterUpdate *calladapt.BoundCaller
111-
classifyChanges *calladapt.BoundCaller
112107

113-
recreateFields map[string]struct{}
108+
fieldTriggers map[string]deployplan.ActionType
114109
}
115110

116111
func NewAdapter(typedNil any, client *databricks.WorkspaceClient) (*Adapter, error) {
@@ -135,29 +130,28 @@ func NewAdapter(typedNil any, client *databricks.WorkspaceClient) (*Adapter, err
135130
doUpdateWithID: nil,
136131
waitAfterCreate: nil,
137132
waitAfterUpdate: nil,
138-
classifyChanges: nil,
139-
recreateFields: map[string]struct{}{},
133+
fieldTriggers: map[string]deployplan.ActionType{},
140134
}
141135

142136
err = adapter.initMethods(impl)
143137
if err != nil {
144138
return nil, err
145139
}
146140

147-
// Load optional RecreateFields method from the unified interface
148-
recreateCall, err := calladapt.PrepareCall(impl, calladapt.TypeOf[IResource](), "RecreateFields")
141+
// Load optional FieldTriggers method from the unified interface
142+
triggerCall, err := calladapt.PrepareCall(impl, calladapt.TypeOf[IResource](), "FieldTriggers")
149143
if err != nil {
150144
return nil, err
151145
}
152-
if recreateCall != nil {
153-
outs, err := recreateCall.Call()
146+
if triggerCall != nil {
147+
outs, err := triggerCall.Call()
154148
if err != nil || len(outs) != 1 {
155-
return nil, fmt.Errorf("failed to call RecreateFields: %w", err)
149+
return nil, fmt.Errorf("failed to call FieldTriggers: %w", err)
156150
}
157-
fields := outs[0].([]string)
158-
adapter.recreateFields = make(map[string]struct{}, len(fields))
159-
for _, field := range fields {
160-
adapter.recreateFields[field] = struct{}{}
151+
fields := outs[0].(map[string]deployplan.ActionType)
152+
adapter.fieldTriggers = make(map[string]deployplan.ActionType, len(fields))
153+
for k, v := range fields {
154+
adapter.fieldTriggers[k] = v
161155
}
162156
}
163157

@@ -217,8 +211,7 @@ func (a *Adapter) initMethods(resource any) error {
217211
return err
218212
}
219213

220-
a.classifyChanges, err = calladapt.PrepareCall(resource, calladapt.TypeOf[IResource](), "ClassifyChanges")
221-
return err
214+
return nil
222215
}
223216

224217
// validateTypes validates type matches for variadic triples of (name, actual, expected).
@@ -293,8 +286,18 @@ func (a *Adapter) validate() error {
293286
return err
294287
}
295288

296-
if a.doUpdateWithID != nil && a.classifyChanges == nil {
297-
return errors.New("if DoUpdateWithID is present, should have implement ClassifyChanges")
289+
// FieldTriggers validation
290+
hasUpdateWithIDTrigger := false
291+
for _, action := range a.fieldTriggers {
292+
if action == deployplan.ActionTypeUpdateWithID {
293+
hasUpdateWithIDTrigger = true
294+
}
295+
}
296+
if hasUpdateWithIDTrigger && a.doUpdateWithID == nil {
297+
return errors.New("FieldTriggers includes update_with_id but DoUpdateWithID is not implemented")
298+
}
299+
if a.doUpdateWithID != nil && !hasUpdateWithIDTrigger {
300+
return errors.New("DoUpdateWithID is implemented but FieldTriggers lacks update_with_id trigger")
298301
}
299302

300303
return nil
@@ -381,11 +384,6 @@ func (a *Adapter) DoUpdate(ctx context.Context, id string, newState any) (any, e
381384
}
382385
}
383386

384-
// HasClassifyChanges returns true if the resource implements ClassifyChanges method.
385-
func (a *Adapter) HasClassifyChanges() bool {
386-
return a.classifyChanges != nil
387-
}
388-
389387
// HasDoUpdateWithID returns true if the resource implements DoUpdateWithID method.
390388
func (a *Adapter) HasDoUpdateWithID() bool {
391389
return a.doUpdateWithID != nil
@@ -412,30 +410,26 @@ func (a *Adapter) DoUpdateWithID(ctx context.Context, oldID string, newState any
412410
}
413411
}
414412

415-
// MustRecreate checks if any of the changes require resource recreation.
416-
func (a *Adapter) MustRecreate(changes []structdiff.Change) bool {
417-
if len(a.recreateFields) == 0 {
418-
return false
413+
// ClassifyByTriggers classifies a set of changes using FieldTriggers.
414+
// Unspecified changed fields default to ActionTypeUpdate. Final action is the
415+
// maximum by precedence. No changes yield ActionTypeNoop.
416+
func (a *Adapter) ClassifyByTriggers(changes []structdiff.Change) deployplan.ActionType {
417+
if len(changes) == 0 {
418+
return deployplan.ActionTypeNoop
419419
}
420+
421+
// Default when there are changes but no explicit trigger is update.
422+
result := deployplan.ActionTypeUpdate
420423
for _, change := range changes {
421-
if _, ok := a.recreateFields[change.Path.String()]; ok {
422-
return true
424+
action, ok := a.fieldTriggers[change.Path.String()]
425+
if !ok {
426+
action = deployplan.ActionTypeUpdate
427+
}
428+
if action > result {
429+
result = action
423430
}
424431
}
425-
return false
426-
}
427-
428-
// ClassifyChanges calls the resource's ClassifyChanges method if implemented.
429-
func (a *Adapter) ClassifyChanges(changes []structdiff.Change) (deployplan.ActionType, error) {
430-
if a.classifyChanges == nil {
431-
return deployplan.ActionTypeUnset, errors.New("internal error: ClassifyChanges not implemented")
432-
}
433-
outs, err := a.classifyChanges.Call(changes)
434-
if err != nil {
435-
return deployplan.ActionTypeUnset, err
436-
}
437-
result := outs[0].(deployplan.ActionType)
438-
return result, nil
432+
return result
439433
}
440434

441435
// WaitAfterCreate waits for the resource to become ready after creation.

bundle/terranova/tnresources/all_test.go

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import (
77
"testing"
88

99
"github.com/databricks/cli/bundle/config/resources"
10+
"github.com/databricks/cli/bundle/deployplan"
1011
"github.com/databricks/cli/libs/structdiff/structpath"
1112
"github.com/databricks/cli/libs/structwalk"
1213
"github.com/databricks/cli/libs/testserver"
@@ -155,7 +156,7 @@ func testCRUD(t *testing.T, group string, adapter *Adapter, client *databricks.W
155156
}
156157

157158
// validateFields uses structwalk to generate all valid field paths and checks membership.
158-
func validateFields(t *testing.T, configType reflect.Type, fields map[string]struct{}) {
159+
func validateFields(t *testing.T, configType reflect.Type, fields map[string]deployplan.ActionType) {
159160
validPaths := make(map[string]struct{})
160161

161162
err := structwalk.WalkType(configType, func(path *structpath.PathNode, typ reflect.Type) bool {
@@ -171,15 +172,15 @@ func validateFields(t *testing.T, configType reflect.Type, fields map[string]str
171172
}
172173
}
173174

174-
// TestRecreateFields validates that all fields in RecreateFields
175+
// TestFieldTriggers validates that all trigger keys
175176
// exist in the corresponding ConfigType for each resource.
176-
func TestRecreateFields(t *testing.T) {
177+
func TestFieldTriggers(t *testing.T) {
177178
for resourceName, resource := range SupportedResources {
178179
adapter, err := NewAdapter(resource, nil)
179180
require.NoError(t, err)
180181

181182
t.Run(resourceName, func(t *testing.T) {
182-
validateFields(t, adapter.InputConfigType(), adapter.recreateFields)
183+
validateFields(t, adapter.InputConfigType(), adapter.fieldTriggers)
183184
})
184185
}
185186
}

bundle/terranova/tnresources/app.go

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"fmt"
66

77
"github.com/databricks/cli/bundle/config/resources"
8+
"github.com/databricks/cli/bundle/deployplan"
89
"github.com/databricks/cli/libs/log"
910
"github.com/databricks/databricks-sdk-go"
1011
"github.com/databricks/databricks-sdk-go/retries"
@@ -63,9 +64,9 @@ func (r *ResourceApp) DoDelete(ctx context.Context, id string) error {
6364
return err
6465
}
6566

66-
func (*ResourceApp) RecreateFields() []string {
67-
return []string{
68-
".name",
67+
func (*ResourceApp) FieldTriggers() map[string]deployplan.ActionType {
68+
return map[string]deployplan.ActionType{
69+
".name": deployplan.ActionTypeRecreate,
6970
}
7071
}
7172

bundle/terranova/tnresources/pipeline.go

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"context"
55

66
"github.com/databricks/cli/bundle/config/resources"
7+
"github.com/databricks/cli/bundle/deployplan"
78
"github.com/databricks/databricks-sdk-go"
89
"github.com/databricks/databricks-sdk-go/service/pipelines"
910
)
@@ -77,12 +78,12 @@ func (r *ResourcePipeline) DoDelete(ctx context.Context, id string) error {
7778
return r.client.Pipelines.DeleteByPipelineId(ctx, id)
7879
}
7980

80-
func (*ResourcePipeline) RecreateFields() []string {
81-
return []string{
82-
".storage",
83-
".catalog",
84-
".ingestion_definition.connection_name",
85-
".ingestion_definition.ingestion_gateway_id",
81+
func (*ResourcePipeline) FieldTriggers() map[string]deployplan.ActionType {
82+
return map[string]deployplan.ActionType{
83+
".storage": deployplan.ActionTypeRecreate,
84+
".catalog": deployplan.ActionTypeRecreate,
85+
".ingestion_definition.connection_name": deployplan.ActionTypeRecreate,
86+
".ingestion_definition.ingestion_gateway_id": deployplan.ActionTypeRecreate,
8687
}
8788
}
8889

bundle/terranova/tnresources/schema.go

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"context"
55

66
"github.com/databricks/cli/bundle/config/resources"
7+
"github.com/databricks/cli/bundle/deployplan"
78
"github.com/databricks/cli/libs/log"
89
"github.com/databricks/databricks-sdk-go"
910
"github.com/databricks/databricks-sdk-go/service/catalog"
@@ -65,10 +66,10 @@ func (r *ResourceSchema) DoDelete(ctx context.Context, id string) error {
6566
})
6667
}
6768

68-
func (*ResourceSchema) RecreateFields() []string {
69-
return []string{
70-
".name",
71-
".catalog_name",
72-
".storage_root",
69+
func (*ResourceSchema) FieldTriggers() map[string]deployplan.ActionType {
70+
return map[string]deployplan.ActionType{
71+
".name": deployplan.ActionTypeRecreate,
72+
".catalog_name": deployplan.ActionTypeRecreate,
73+
".storage_root": deployplan.ActionTypeRecreate,
7374
}
7475
}

bundle/terranova/tnresources/volume.go

Lines changed: 7 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ import (
88
"github.com/databricks/cli/bundle/config/resources"
99
"github.com/databricks/cli/bundle/deployplan"
1010
"github.com/databricks/cli/libs/log"
11-
"github.com/databricks/cli/libs/structdiff"
1211
"github.com/databricks/databricks-sdk-go"
1312
"github.com/databricks/databricks-sdk-go/service/catalog"
1413
)
@@ -101,24 +100,16 @@ func (r *ResourceVolume) DoDelete(ctx context.Context, id string) error {
101100
return r.client.Volumes.DeleteByName(ctx, id)
102101
}
103102

104-
func (*ResourceVolume) RecreateFields() []string {
105-
return []string{
106-
".catalog_name",
107-
".schema_name",
108-
".storage_location",
109-
".volume_type",
103+
func (*ResourceVolume) FieldTriggers() map[string]deployplan.ActionType {
104+
return map[string]deployplan.ActionType{
105+
".catalog_name": deployplan.ActionTypeRecreate,
106+
".schema_name": deployplan.ActionTypeRecreate,
107+
".storage_location": deployplan.ActionTypeRecreate,
108+
".volume_type": deployplan.ActionTypeRecreate,
109+
".name": deployplan.ActionTypeUpdateWithID,
110110
}
111111
}
112112

113-
func (r *ResourceVolume) ClassifyChanges(changes []structdiff.Change) deployplan.ActionType {
114-
for _, change := range changes {
115-
if change.Path.String() == ".name" {
116-
return deployplan.ActionTypeUpdateWithID
117-
}
118-
}
119-
return deployplan.ActionTypeUpdate
120-
}
121-
122113
func getNameFromID(id string) (string, error) {
123114
items := strings.Split(id, ".")
124115
if len(items) == 0 {

0 commit comments

Comments
 (0)