Skip to content

Commit 700366b

Browse files
Varun Deep Sainidenik
andauthored
Reverse dependency order for delete operations in direct engine (#4105)
Fixes: #4089 ## Changes When deleting resources with dependencies, the direct deployment engine now deletes dependents before their dependencies. Previously, resources were deleted in deployment order, which could cause errors when a dependency was deleted before resources that depend on it. Edge direction in the dependency graph is now determined per-node based on action type: - Delete actions: reverse edge (dependent → dependency) - Other actions: normal edge (dependency → dependent) --------- Co-authored-by: Denis Bilenko <[email protected]>
1 parent f915956 commit 700366b

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

55 files changed

+1456
-63
lines changed

NEXT_CHANGELOG.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,8 @@
77
### CLI
88

99
### Bundles
10-
* Pass SYSTEM_ACCESSTOKEN from env to the Terraform provider ([#4135](https://github.com/databricks/cli/pull/4135))
10+
* engine/direct: Fix dependency-ordered deletion by persisting depends_on in state ([#4105](https://github.com/databricks/cli/pull/4105))
11+
* Pass SYSTEM_ACCESSTOKEN from env to the Terraform provider ([#4135](https://github.com/databricks/cli/pull/4135)
1112

1213
### Dependency updates
1314

acceptance/bundle/migrate/basic/out.new_state.json

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,33 @@
5353
"volume_catalog_name": "mycat",
5454
"volume_storage_location": "s3://deco-uc-prod-isolated-aws-us-east-1/metastore/[UUID]/volumes/[UUID]"
5555
}
56-
}
56+
},
57+
"depends_on": [
58+
{
59+
"node": "resources.jobs.test_job",
60+
"label": "${resources.jobs.test_job.id}"
61+
},
62+
{
63+
"node": "resources.jobs.test_job",
64+
"label": "${resources.jobs.test_job.name}"
65+
},
66+
{
67+
"node": "resources.jobs.test_job",
68+
"label": "${resources.jobs.test_job.timeout_seconds}"
69+
},
70+
{
71+
"node": "resources.volumes.test_volume",
72+
"label": "${resources.volumes.test_volume.catalog_name}"
73+
},
74+
{
75+
"node": "resources.volumes.test_volume",
76+
"label": "${resources.volumes.test_volume.id}"
77+
},
78+
{
79+
"node": "resources.volumes.test_volume",
80+
"label": "${resources.volumes.test_volume.storage_location}"
81+
}
82+
]
5783
},
5884
"resources.volumes.test_volume": {
5985
"__id__": "mycat.myschema.myvol",

acceptance/bundle/migrate/default-python/out.state_after_migration.json

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,13 @@
101101
"unit": "DAYS"
102102
}
103103
}
104-
}
104+
},
105+
"depends_on": [
106+
{
107+
"node": "resources.pipelines.my_default_python_etl",
108+
"label": "${resources.pipelines.my_default_python_etl.id}"
109+
}
110+
]
105111
},
106112
"resources.pipelines.my_default_python_etl": {
107113
"__id__": "[UUID]",

acceptance/bundle/migrate/grants/out.new_state.json

Lines changed: 35 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,13 @@
1111
"comment": "mycomment",
1212
"name": "mymodel",
1313
"schema_name": "schema_grants"
14-
}
14+
},
15+
"depends_on": [
16+
{
17+
"node": "resources.schemas.my_schema",
18+
"label": "${resources.schemas.my_schema.name}"
19+
}
20+
]
1521
},
1622
"resources.registered_models.my_registered_model.grants": {
1723
"__id__": "function/main.schema_grants.mymodel",
@@ -26,7 +32,13 @@
2632
]
2733
}
2834
]
29-
}
35+
},
36+
"depends_on": [
37+
{
38+
"node": "resources.registered_models.my_registered_model",
39+
"label": "${resources.registered_models.my_registered_model.id}"
40+
}
41+
]
3042
},
3143
"resources.schemas.my_schema": {
3244
"__id__": "main.schema_grants",
@@ -49,7 +61,13 @@
4961
]
5062
}
5163
]
52-
}
64+
},
65+
"depends_on": [
66+
{
67+
"node": "resources.schemas.my_schema",
68+
"label": "${resources.schemas.my_schema.id}"
69+
}
70+
]
5371
},
5472
"resources.volumes.my_volume": {
5573
"__id__": "main.schema_grants.volume_name",
@@ -58,7 +76,13 @@
5876
"name": "volume_name",
5977
"schema_name": "schema_grants",
6078
"volume_type": "MANAGED"
61-
}
79+
},
80+
"depends_on": [
81+
{
82+
"node": "resources.schemas.my_schema",
83+
"label": "${resources.schemas.my_schema.name}"
84+
}
85+
]
6286
},
6387
"resources.volumes.my_volume.grants": {
6488
"__id__": "volume/main.schema_grants.volume_name",
@@ -74,7 +98,13 @@
7498
]
7599
}
76100
]
77-
}
101+
},
102+
"depends_on": [
103+
{
104+
"node": "resources.volumes.my_volume",
105+
"label": "${resources.volumes.my_volume.id}"
106+
}
107+
]
78108
}
79109
}
80110
}

acceptance/bundle/migrate/permissions/out.new_state.json

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,13 @@
4242
"user_name": "[USERNAME]"
4343
}
4444
]
45-
}
45+
},
46+
"depends_on": [
47+
{
48+
"node": "resources.jobs.test_job",
49+
"label": "${resources.jobs.test_job.id}"
50+
}
51+
]
4652
},
4753
"resources.pipelines.test_pipeline": {
4854
"__id__": "[UUID]",
@@ -77,7 +83,13 @@
7783
"user_name": "[USERNAME]"
7884
}
7985
]
80-
}
86+
},
87+
"depends_on": [
88+
{
89+
"node": "resources.pipelines.test_pipeline",
90+
"label": "${resources.pipelines.test_pipeline.id}"
91+
}
92+
]
8193
}
8294
}
8395
}

acceptance/bundle/migrate/runas/out.new_state.json

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,13 @@
4343
"user_name": "[USERNAME]"
4444
}
4545
]
46-
}
46+
},
47+
"depends_on": [
48+
{
49+
"node": "resources.pipelines.foo",
50+
"label": "${resources.pipelines.foo.id}"
51+
}
52+
]
4753
}
4854
}
4955
}

acceptance/bundle/resource_deps/job_id/out.plan_delete.direct.json

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,12 @@
2828
}
2929
},
3030
"resources.jobs.foo": {
31+
"depends_on": [
32+
{
33+
"node": "resources.jobs.bar",
34+
"label": "${resources.jobs.bar.id}"
35+
}
36+
],
3137
"action": "delete",
3238
"remote_state": {
3339
"created_time": [UNIX_TIME_MILLIS][1],

acceptance/bundle/resource_deps/job_id/output.txt

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -65,19 +65,19 @@ Deploying resources...
6565
Updating deployment state...
6666
Deployment complete!
6767

68-
>>> print_requests.py --sort //jobs
68+
>>> print_requests.py //jobs
6969
{
7070
"method": "POST",
7171
"path": "/api/2.2/jobs/delete",
7272
"body": {
73-
"job_id": [BAR_ID]
73+
"job_id": [FOO_ID]
7474
}
7575
}
7676
{
7777
"method": "POST",
7878
"path": "/api/2.2/jobs/delete",
7979
"body": {
80-
"job_id": [FOO_ID]
80+
"job_id": [BAR_ID]
8181
}
8282
}
8383

acceptance/bundle/resource_deps/job_id/script

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,7 @@ echo "$bar_id:BAR_ID" >> ACC_REPLS
1313
cp empty.yml databricks.yml
1414
trace $CLI bundle plan -o json > out.plan_delete.$DATABRICKS_BUNDLE_ENGINE.json
1515
trace $CLI bundle deploy
16-
# TODO sorting requests should not be needed one we persist depends_on in state
17-
trace print_requests.py --sort //jobs
16+
trace print_requests.py //jobs
1817

1918
trace $CLI bundle destroy --auto-approve
2019
trace print_requests.py //jobs
Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,78 @@
1+
bundle:
2+
name: test-bundle
3+
4+
# Complex graph combining: chain, diamond, independent, multi-deps
5+
#
6+
# independent1 independent2
7+
#
8+
# chain_top -> chain_mid -> chain_bottom
9+
#
10+
# diamond_top -> diamond_left -> diamond_bottom
11+
# diamond_right ->
12+
#
13+
# multi_child -> multi_parent1
14+
# -> multi_parent2
15+
16+
resources:
17+
jobs:
18+
# Independent nodes (no dependencies)
19+
independent1:
20+
name: job independent1
21+
independent2:
22+
name: job independent2
23+
24+
# Chain: chain_top -> chain_mid -> chain_bottom
25+
chain_bottom:
26+
name: job chain_bottom
27+
chain_mid:
28+
name: job chain_mid
29+
tasks:
30+
- task_key: t1
31+
run_job_task:
32+
job_id: ${resources.jobs.chain_bottom.id}
33+
chain_top:
34+
name: job chain_top
35+
tasks:
36+
- task_key: t1
37+
run_job_task:
38+
job_id: ${resources.jobs.chain_mid.id}
39+
40+
# Diamond: diamond_top -> diamond_left/right -> diamond_bottom
41+
diamond_bottom:
42+
name: job diamond_bottom
43+
diamond_left:
44+
name: job diamond_left
45+
tasks:
46+
- task_key: t1
47+
run_job_task:
48+
job_id: ${resources.jobs.diamond_bottom.id}
49+
diamond_right:
50+
name: job diamond_right
51+
tasks:
52+
- task_key: t1
53+
run_job_task:
54+
job_id: ${resources.jobs.diamond_bottom.id}
55+
diamond_top:
56+
name: job diamond_top
57+
tasks:
58+
- task_key: t1
59+
run_job_task:
60+
job_id: ${resources.jobs.diamond_left.id}
61+
- task_key: t2
62+
run_job_task:
63+
job_id: ${resources.jobs.diamond_right.id}
64+
65+
# Multi-deps: multi_child -> multi_parent1, multi_parent2
66+
multi_parent1:
67+
name: job multi_parent1
68+
multi_parent2:
69+
name: job multi_parent2
70+
multi_child:
71+
name: job multi_child
72+
tasks:
73+
- task_key: t1
74+
run_job_task:
75+
job_id: ${resources.jobs.multi_parent1.id}
76+
- task_key: t2
77+
run_job_task:
78+
job_id: ${resources.jobs.multi_parent2.id}

0 commit comments

Comments
 (0)