Skip to content

Commit f948985

Browse files
authored
Refactor deletes in direct deployment (#3163)
## Changes Remove DoDelete method from IResource, add a static function per resource. ## Why Delete operation is special - it does not require config. In fact, it is often run when config is gone. On the other hand all other operations need config. To support both delete and non-delete operation in one class we would need to have constant checks/asserts config != nil. To avoid this complication, I'm taking delete out of IResource. ## Tests Existing tests.
1 parent 0933d2b commit f948985

10 files changed

Lines changed: 73 additions & 22 deletions

File tree

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
resources:
2+
pipelines:
3+
my_pipeline:
4+
name: test-pipeline
5+
libraries:
6+
- file:
7+
path: "./foo.py"
8+
9+
schemas:
10+
my_schema:
11+
name: test-schema
12+
catalog_name: main
13+
comment: COMMENT1

acceptance/bundle/destroy/all-resources/foo.py

Whitespace-only changes.
Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
2+
>>> [CLI] bundle deploy
3+
Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/test-bundle/default/files...
4+
Deploying resources...
5+
Updating deployment state...
6+
Deployment complete!
7+
8+
>>> [CLI] bundle destroy --auto-approve
9+
The following resources will be deleted:
10+
delete pipeline my_pipeline
11+
delete schema my_schema
12+
13+
All files and directories at the following location will be deleted: /Workspace/Users/[USERNAME]/.bundle/test-bundle/default
14+
15+
Deleting files...
16+
Destroy complete!
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
trace $CLI bundle deploy
2+
trace $CLI bundle destroy --auto-approve

bundle/terranova/apply.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -207,7 +207,7 @@ func (d *Deployer) Create(ctx context.Context, resource tnresources.IResource, c
207207
}
208208

209209
func (d *Deployer) Recreate(ctx context.Context, oldResource tnresources.IResource, oldID string, config any) error {
210-
err := oldResource.DoDelete(ctx, oldID)
210+
err := tnresources.DeleteResource(ctx, d.client, d.group, oldID)
211211
if err != nil {
212212
return fmt.Errorf("deleting old id=%s: %w", oldID, err)
213213
}
@@ -267,7 +267,7 @@ func (d *Deployer) Update(ctx context.Context, resource tnresources.IResource, o
267267

268268
func (d *Deployer) Delete(ctx context.Context, resource tnresources.IResource, oldID string) error {
269269
// TODO: recognize 404 and 403 as "deleted" and proceed to removing state
270-
err := resource.DoDelete(ctx, oldID)
270+
err := tnresources.DeleteResource(ctx, d.client, d.group, oldID)
271271
if err != nil {
272272
return fmt.Errorf("deleting id=%s: %w", oldID, err)
273273
}

bundle/terranova/tnresources/app.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ func (r *ResourceApp) DoUpdate(ctx context.Context, id string) (string, error) {
5454
return response.Name, nil
5555
}
5656

57-
func (r *ResourceApp) DoDelete(ctx context.Context, id string) error {
57+
func DeleteApp(ctx context.Context, client *databricks.WorkspaceClient, id string) error {
5858
// TODO: implement app deletion
5959
return nil
6060
}

bundle/terranova/tnresources/job.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -52,12 +52,12 @@ func (r *ResourceJob) DoUpdate(ctx context.Context, id string) (string, error) {
5252
return id, nil
5353
}
5454

55-
func (r *ResourceJob) DoDelete(ctx context.Context, id string) error {
55+
func DeleteJob(ctx context.Context, client *databricks.WorkspaceClient, id string) error {
5656
idInt, err := strconv.ParseInt(id, 10, 64)
5757
if err != nil {
5858
return err
5959
}
60-
err = r.client.Jobs.DeleteByJobId(ctx, idInt)
60+
err = client.Jobs.DeleteByJobId(ctx, idInt)
6161
if err != nil {
6262
return SDKError{Method: "Jobs.DeleteByJobId", Err: err}
6363
}

bundle/terranova/tnresources/pipeline.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,8 +49,8 @@ func (r *ResourcePipeline) DoUpdate(ctx context.Context, id string) (string, err
4949
return id, nil
5050
}
5151

52-
func (r *ResourcePipeline) DoDelete(ctx context.Context, id string) error {
53-
err := r.client.Pipelines.DeleteByPipelineId(ctx, id)
52+
func DeletePipeline(ctx context.Context, client *databricks.WorkspaceClient, id string) error {
53+
err := client.Pipelines.DeleteByPipelineId(ctx, id)
5454
if err != nil {
5555
return SDKError{Method: "Pipelines.DeleteByPipelineId", Err: err}
5656
}

bundle/terranova/tnresources/resource.go

Lines changed: 34 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -11,19 +11,35 @@ import (
1111
"github.com/databricks/databricks-sdk-go"
1212
)
1313

14+
const (
15+
_jobs = "jobs"
16+
_pipelines = "pipelines"
17+
_schemas = "schemas"
18+
_apps = "apps"
19+
)
20+
1421
var supportedResources = map[string]reflect.Value{
15-
"jobs": reflect.ValueOf(NewResourceJob),
16-
"pipelines": reflect.ValueOf(NewResourcePipeline),
17-
"schemas": reflect.ValueOf(NewResourceSchema),
18-
"apps": reflect.ValueOf(NewResourceApp),
22+
_jobs: reflect.ValueOf(NewResourceJob),
23+
_pipelines: reflect.ValueOf(NewResourcePipeline),
24+
_schemas: reflect.ValueOf(NewResourceSchema),
25+
_apps: reflect.ValueOf(NewResourceApp),
1926
}
2027

2128
// This types matches what Config() returns and should match 'config' field in the resource struct
2229
var supportedResourcesTypes = map[string]reflect.Type{
23-
"jobs": reflect.TypeOf(ResourceJob{}.config),
24-
"pipelines": reflect.TypeOf(ResourcePipeline{}.config),
25-
"schemas": reflect.TypeOf(ResourceSchema{}.config),
26-
"apps": reflect.TypeOf(ResourceApp{}.config),
30+
_jobs: reflect.TypeOf(ResourceJob{}.config),
31+
_pipelines: reflect.TypeOf(ResourcePipeline{}.config),
32+
_schemas: reflect.TypeOf(ResourceSchema{}.config),
33+
_apps: reflect.TypeOf(ResourceApp{}.config),
34+
}
35+
36+
type DeleteResourceFN = func(ctx context.Context, client *databricks.WorkspaceClient, oldID string) error
37+
38+
var deletableResources = map[string]DeleteResourceFN{
39+
_jobs: DeleteJob,
40+
_pipelines: DeletePipeline,
41+
_schemas: DeleteSchema,
42+
_apps: DeleteApp,
2743
}
2844

2945
type IResource interface {
@@ -34,9 +50,7 @@ type IResource interface {
3450

3551
// Update the resource. Returns id of the resource.
3652
// Usually returns the same id as oldId but can also return a different one (e.g. schemas and volumes when certain fields are changed)
37-
DoUpdate(ctx context.Context, oldId string) (string, error)
38-
39-
DoDelete(ctx context.Context, oldId string) error
53+
DoUpdate(ctx context.Context, oldID string) (string, error)
4054

4155
WaitAfterCreate(ctx context.Context) error
4256
WaitAfterUpdate(ctx context.Context) error
@@ -59,9 +73,7 @@ func invokeConstructor(ctor reflect.Value, client *databricks.WorkspaceClient, c
5973
// Prepare the config value matching the expected type.
6074
var cfgVal reflect.Value
6175
if cfg == nil {
62-
// Treat nil as a request for the zero value of the expected config type. This
63-
// is useful for actions (like deletion) where the config is irrelevant.
64-
cfgVal = reflect.Zero(expectedCfgType)
76+
return nil, errors.New("internal error, config must not be nil")
6577
} else {
6678
suppliedVal := reflect.ValueOf(cfg)
6779
if suppliedVal.Type() != expectedCfgType {
@@ -116,3 +128,11 @@ func New(client *databricks.WorkspaceClient, group, name string, config any) (IR
116128

117129
return result, cfgType, nil
118130
}
131+
132+
func DeleteResource(ctx context.Context, client *databricks.WorkspaceClient, group, id string) error {
133+
fn, ok := deletableResources[group]
134+
if !ok {
135+
return fmt.Errorf("cannot delete %s", group)
136+
}
137+
return fn(ctx, client, id)
138+
}

bundle/terranova/tnresources/schema.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ func (r *ResourceSchema) DoUpdate(ctx context.Context, id string) (string, error
5151
return response.FullName, nil
5252
}
5353

54-
func (r *ResourceSchema) DoDelete(ctx context.Context, id string) error {
54+
func DeleteSchema(ctx context.Context, client *databricks.WorkspaceClient, id string) error {
5555
// TODO: implement schema deletion
5656
return nil
5757
}

0 commit comments

Comments
 (0)