Skip to content

Commit 691538b

Browse files
Combine DoUpdate and DoUpdateWithChanges (#3995)
## Changes This PR: 1. Combines DoUpdate and DoUpdateWithChanges 2. Keeps the combined new function as optional ## Why Some resources don't support DoUpdate operations: 1. **Secret scopes** - The Databricks API doesn't provide an update method for secret scopes 3. **Serving endpoints** - Only `DoUpdateWithChanges` is necessary, not the standard `DoUpdate` method This also simplifies the direct deployment framework. ## Tests All existing tests continue to pass.
1 parent c55065c commit 691538b

23 files changed

+84
-80
lines changed

bundle/direct/apply.go

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -105,15 +105,11 @@ func (d *DeploymentUnit) Recreate(ctx context.Context, db *dstate.DeploymentStat
105105
}
106106

107107
func (d *DeploymentUnit) Update(ctx context.Context, db *dstate.DeploymentState, id string, newState any, changes *deployplan.Changes) error {
108-
var remoteState any
109-
var err error
110-
111-
if d.Adapter.HasDoUpdateWithChanges() && changes != nil {
112-
remoteState, err = d.Adapter.DoUpdateWithChanges(ctx, id, newState, changes)
113-
} else {
114-
remoteState, err = d.Adapter.DoUpdate(ctx, id, newState)
108+
if !d.Adapter.HasDoUpdate() {
109+
return fmt.Errorf("internal error: DoUpdate not implemented for resource %s", d.ResourceKey)
115110
}
116111

112+
remoteState, err := d.Adapter.DoUpdate(ctx, id, newState, changes)
117113
if err != nil {
118114
return fmt.Errorf("updating id=%s: %w", id, err)
119115
}

bundle/direct/bundle_plan.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -202,6 +202,12 @@ func (b *DeploymentBundle) CalculatePlan(ctx context.Context, client *databricks
202202
b.RemoteStateCache.Store(resourceKey, remoteState)
203203
}
204204

205+
// Validate that resources without DoUpdate don't have update actions
206+
if action == deployplan.ActionTypeUpdate && !adapter.HasDoUpdate() {
207+
logdiag.LogError(ctx, fmt.Errorf("%s: resource does not support update action but plan produced update", errorPrefix))
208+
return false
209+
}
210+
205211
entry.Action = action.String()
206212

207213
if len(localChangeMap) > 0 || len(remoteChangeMap) > 0 {

bundle/direct/dresources/adapter.go

Lines changed: 24 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -63,13 +63,10 @@ type IResource interface {
6363
// Example: func (r *ResourceVolume) DoCreate(ctx context.Context, newState *catalog.CreateVolumeRequestContent) (string, *catalog.VolumeInfo, error)
6464
DoCreate(ctx context.Context, newState any) (id string, remoteState any, e error)
6565

66-
// DoUpdate updates the resource. ID must not change as a result of this operation. Returns optionally remote state.
66+
// [Optional] DoUpdate updates the resource. ID must not change as a result of this operation. Returns optionally remote state.
6767
// If remote state is available as part of the operation, return it; otherwise return nil.
68-
// Example: func (r *ResourceSchema) DoUpdate(ctx context.Context, id string, newState *catalog.CreateSchema) (*catalog.SchemaInfo, error)
69-
DoUpdate(ctx context.Context, id string, newState any) (remoteState any, e error)
70-
71-
// [Optional] DoUpdateWithChanges updates the resource with information about changes computed during plan. Returns optionally remote state.
72-
DoUpdateWithChanges(ctx context.Context, id string, newState any, changes *deployplan.Changes) (remoteState any, e error)
68+
// Example: func (r *ResourceSchema) DoUpdate(ctx context.Context, id string, newState *catalog.CreateSchema, changes *deployplan.Changes) (*catalog.SchemaInfo, error)
69+
DoUpdate(ctx context.Context, id string, newState any, changes *deployplan.Changes) (remoteState any, e error)
7370

7471
// [Optional] DoUpdateWithID performs an update that may result in resource having a new ID. Returns new id and optionally remote state.
7572
DoUpdateWithID(ctx context.Context, id string, newState any) (newID string, remoteState any, e error)
@@ -94,15 +91,14 @@ type Adapter struct {
9491
doRefresh *calladapt.BoundCaller
9592
doDelete *calladapt.BoundCaller
9693
doCreate *calladapt.BoundCaller
97-
doUpdate *calladapt.BoundCaller
9894

9995
// Optional:
100-
doUpdateWithChanges *calladapt.BoundCaller
101-
doUpdateWithID *calladapt.BoundCaller
102-
waitAfterCreate *calladapt.BoundCaller
103-
waitAfterUpdate *calladapt.BoundCaller
104-
classifyChange *calladapt.BoundCaller
105-
doResize *calladapt.BoundCaller
96+
doUpdate *calladapt.BoundCaller
97+
doUpdateWithID *calladapt.BoundCaller
98+
waitAfterCreate *calladapt.BoundCaller
99+
waitAfterUpdate *calladapt.BoundCaller
100+
classifyChange *calladapt.BoundCaller
101+
doResize *calladapt.BoundCaller
106102

107103
fieldTriggersLocal map[string]deployplan.ActionType
108104
fieldTriggersRemote map[string]deployplan.ActionType
@@ -128,7 +124,6 @@ func NewAdapter(typedNil any, client *databricks.WorkspaceClient) (*Adapter, err
128124
doDelete: nil,
129125
doCreate: nil,
130126
doUpdate: nil,
131-
doUpdateWithChanges: nil,
132127
doUpdateWithID: nil,
133128
doResize: nil,
134129
waitAfterCreate: nil,
@@ -225,14 +220,9 @@ func (a *Adapter) initMethods(resource any) error {
225220
return err
226221
}
227222

228-
a.doUpdate, err = prepareCallRequired(resource, "DoUpdate")
229-
if err != nil {
230-
return err
231-
}
232-
233223
// Optional methods with varying signatures:
234224

235-
a.doUpdateWithChanges, err = calladapt.PrepareCall(resource, calladapt.TypeOf[IResource](), "DoUpdateWithChanges")
225+
a.doUpdate, err = calladapt.PrepareCall(resource, calladapt.TypeOf[IResource](), "DoUpdate")
236226
if err != nil {
237227
return err
238228
}
@@ -299,7 +289,6 @@ func (a *Adapter) validate() error {
299289
validations := []any{
300290
"PrepareState return", a.prepareState.OutTypes[0], stateType,
301291
"DoCreate newState", a.doCreate.InTypes[1], stateType,
302-
"DoUpdate newState", a.doUpdate.InTypes[2], stateType,
303292
}
304293

305294
// If RemapState is implemented, validate its signature.
@@ -319,19 +308,13 @@ func (a *Adapter) validate() error {
319308
}
320309
validations = append(validations, "DoCreate remoteState return", a.doCreate.OutTypes[1], remoteType)
321310

322-
// Validate DoUpdate: must return (remoteType, error)
323-
if len(a.doUpdate.OutTypes) != 2 {
324-
return fmt.Errorf("DoUpdate must return (remoteType, error), got %d return values", len(a.doUpdate.OutTypes))
325-
}
326-
validations = append(validations, "DoUpdate remoteState return", a.doUpdate.OutTypes[0], remoteType)
327-
328-
if a.doUpdateWithChanges != nil {
329-
validations = append(validations, "DoUpdateWithChanges newState", a.doUpdateWithChanges.InTypes[2], stateType)
330-
// DoUpdateWithChanges must return (remoteType, error)
331-
if len(a.doUpdateWithChanges.OutTypes) != 2 {
332-
return fmt.Errorf("DoUpdateWithChanges must return (remoteType, error), got %d return values", len(a.doUpdateWithChanges.OutTypes))
311+
// Validate DoUpdate: must return (remoteType, error) if implemented
312+
if a.doUpdate != nil {
313+
validations = append(validations, "DoUpdate newState", a.doUpdate.InTypes[2], stateType)
314+
if len(a.doUpdate.OutTypes) != 2 {
315+
return fmt.Errorf("DoUpdate must return (remoteType, error), got %d return values", len(a.doUpdate.OutTypes))
333316
}
334-
validations = append(validations, "DoUpdateWithChanges remoteState return", a.doUpdateWithChanges.OutTypes[0], remoteType)
317+
validations = append(validations, "DoUpdate remoteState return", a.doUpdate.OutTypes[0], remoteType)
335318
}
336319

337320
if a.doResize != nil {
@@ -471,30 +454,19 @@ func (a *Adapter) DoCreate(ctx context.Context, newState any) (string, any, erro
471454
return id, remoteState, nil
472455
}
473456

474-
// DoUpdate updates the resource. Returns remote state if available, otherwise nil.
475-
func (a *Adapter) DoUpdate(ctx context.Context, id string, newState any) (any, error) {
476-
outs, err := a.doUpdate.Call(ctx, id, newState)
477-
if err != nil {
478-
return nil, err
479-
}
480-
481-
remoteState := normalizeNilPointer(outs[0])
482-
return remoteState, nil
483-
}
484-
485-
// HasDoUpdateWithChanges returns true if the resource implements DoUpdateWithChanges method.
486-
func (a *Adapter) HasDoUpdateWithChanges() bool {
487-
return a.doUpdateWithChanges != nil
457+
// HasDoUpdate returns true if the resource implements DoUpdate method.
458+
func (a *Adapter) HasDoUpdate() bool {
459+
return a.doUpdate != nil
488460
}
489461

490-
// DoUpdateWithChanges updates the resource with information about changes computed during plan.
462+
// DoUpdate updates the resource with information about changes computed during plan.
491463
// Returns remote state if available, otherwise nil.
492-
func (a *Adapter) DoUpdateWithChanges(ctx context.Context, id string, newState any, changes *deployplan.Changes) (any, error) {
493-
if a.doUpdateWithChanges == nil {
494-
return nil, errors.New("internal error: DoUpdateWithChanges not found")
464+
func (a *Adapter) DoUpdate(ctx context.Context, id string, newState any, changes *deployplan.Changes) (any, error) {
465+
if a.doUpdate == nil {
466+
return nil, errors.New("internal error: DoUpdate not found")
495467
}
496468

497-
outs, err := a.doUpdateWithChanges.Call(ctx, id, newState, changes)
469+
outs, err := a.doUpdate.Call(ctx, id, newState, changes)
498470
if err != nil {
499471
return nil, err
500472
}

bundle/direct/dresources/alert.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,13 @@ 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/sql"
910
)
1011

12+
type Changes = deployplan.Changes
13+
1114
type ResourceAlert struct {
1215
client *databricks.WorkspaceClient
1316
}
@@ -40,7 +43,7 @@ func (r *ResourceAlert) DoCreate(ctx context.Context, config *sql.AlertV2) (stri
4043
}
4144

4245
// DoUpdate updates the alert in place.
43-
func (r *ResourceAlert) DoUpdate(ctx context.Context, id string, config *sql.AlertV2) (*sql.AlertV2, error) {
46+
func (r *ResourceAlert) DoUpdate(ctx context.Context, id string, config *sql.AlertV2, _ *Changes) (*sql.AlertV2, error) {
4447
request := sql.UpdateAlertV2Request{
4548
Id: id,
4649
Alert: *config,

bundle/direct/dresources/all_test.go

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -465,7 +465,7 @@ func testCRUD(t *testing.T, group string, adapter *Adapter, client *databricks.W
465465
require.Equal(t, remote, remoteStateFromWaitCreate)
466466
}
467467

468-
remoteStateFromUpdate, err := adapter.DoUpdate(ctx, createdID, newState)
468+
remoteStateFromUpdate, err := adapter.DoUpdate(ctx, createdID, newState, nil)
469469
require.NoError(t, err, "DoUpdate failed")
470470
if remoteStateFromUpdate != nil {
471471
remappedStateFromUpdate, err := adapter.RemapState(remoteStateFromUpdate)
@@ -576,6 +576,33 @@ func TestFieldTriggers(t *testing.T) {
576576
}
577577
}
578578

579+
// TestFieldTriggersNoUpdateWhenNotImplemented validates that resources without
580+
// DoUpdate implementation don't produce update actions in their FieldTriggers.
581+
func TestFieldTriggersNoUpdateWhenNotImplemented(t *testing.T) {
582+
for resourceName, resource := range SupportedResources {
583+
adapter, err := NewAdapter(resource, nil)
584+
require.NoError(t, err)
585+
586+
if adapter.HasDoUpdate() {
587+
continue
588+
}
589+
590+
t.Run(resourceName+"_local", func(t *testing.T) {
591+
for field, action := range adapter.fieldTriggersLocal {
592+
assert.NotEqual(t, deployplan.ActionTypeUpdate, action,
593+
"resource %s does not implement DoUpdate but field %s triggers update action", resourceName, field)
594+
}
595+
})
596+
597+
t.Run(resourceName+"_remote", func(t *testing.T) {
598+
for field, action := range adapter.fieldTriggersRemote {
599+
assert.NotEqual(t, deployplan.ActionTypeUpdate, action,
600+
"resource %s does not implement DoUpdate but field %s triggers update action", resourceName, field)
601+
}
602+
})
603+
}
604+
}
605+
579606
func setupTestServerClient(t *testing.T) (*testserver.Server, *databricks.WorkspaceClient) {
580607
server := testserver.New(t)
581608
testserver.AddDefaultHandlers(server)

bundle/direct/dresources/app.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ func (r *ResourceApp) DoCreate(ctx context.Context, config *apps.App) (string, *
4242
return waiter.Response.Name, nil, nil
4343
}
4444

45-
func (r *ResourceApp) DoUpdate(ctx context.Context, id string, config *apps.App) (*apps.App, error) {
45+
func (r *ResourceApp) DoUpdate(ctx context.Context, id string, config *apps.App, _ *Changes) (*apps.App, error) {
4646
request := apps.UpdateAppRequest{
4747
App: *config,
4848
Name: id,

bundle/direct/dresources/cluster.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ func (r *ResourceCluster) DoCreate(ctx context.Context, config *compute.ClusterS
8585
return wait.ClusterId, nil, nil
8686
}
8787

88-
func (r *ResourceCluster) DoUpdate(ctx context.Context, id string, config *compute.ClusterSpec) (*compute.ClusterDetails, error) {
88+
func (r *ResourceCluster) DoUpdate(ctx context.Context, id string, config *compute.ClusterSpec, _ *Changes) (*compute.ClusterDetails, error) {
8989
// Same retry as in TF provider logic
9090
// https://github.com/databricks/terraform-provider-databricks/blob/3eecd0f90cf99d7777e79a3d03c41f9b2aafb004/clusters/resource_cluster.go#L624
9191
timeout := 15 * time.Minute

bundle/direct/dresources/dashboard.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -234,7 +234,7 @@ func (r *ResourceDashboard) DoCreate(ctx context.Context, config *resources.Dash
234234
return createResp.DashboardId, responseToState(createResp, publishResp), nil
235235
}
236236

237-
func (r *ResourceDashboard) DoUpdate(ctx context.Context, id string, config *resources.DashboardConfig) (*resources.DashboardConfig, error) {
237+
func (r *ResourceDashboard) DoUpdate(ctx context.Context, id string, config *resources.DashboardConfig, _ *Changes) (*resources.DashboardConfig, error) {
238238
dashboard, err := prepareDashboardRequest(config)
239239
if err != nil {
240240
return nil, err

bundle/direct/dresources/database_catalog.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ func (r *ResourceDatabaseCatalog) DoCreate(ctx context.Context, config *database
3434
return result.Name, nil, nil
3535
}
3636

37-
func (r *ResourceDatabaseCatalog) DoUpdate(ctx context.Context, id string, config *database.DatabaseCatalog) (*database.DatabaseCatalog, error) {
37+
func (r *ResourceDatabaseCatalog) DoUpdate(ctx context.Context, id string, config *database.DatabaseCatalog, _ *Changes) (*database.DatabaseCatalog, error) {
3838
request := database.UpdateDatabaseCatalogRequest{
3939
DatabaseCatalog: *config,
4040
Name: id,

bundle/direct/dresources/database_instance.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ func (d *ResourceDatabaseInstance) DoCreate(ctx context.Context, config *databas
3535
return waiter.Response.Name, nil, nil
3636
}
3737

38-
func (d *ResourceDatabaseInstance) DoUpdate(ctx context.Context, id string, config *database.DatabaseInstance) (*database.DatabaseInstance, error) {
38+
func (d *ResourceDatabaseInstance) DoUpdate(ctx context.Context, id string, config *database.DatabaseInstance, _ *Changes) (*database.DatabaseInstance, error) {
3939
request := database.UpdateDatabaseInstanceRequest{
4040
DatabaseInstance: *config,
4141
Name: config.Name,

0 commit comments

Comments
 (0)