Skip to content

Commit f915956

Browse files
denikdenik
authored
direct: parse state via json.RawMessage into appropriate type (#4138)
## Changes Instead of parsing into any and then converting into the target type, we now store json.RawMessage and then parse into target type when needed. ## Why One issue it fixes is that JSON parser by default parses integers as float64, losing precision. This was found by @varundeepsaini and a fix is proposed in #4136 This PR is more general though, fully relying on the type to parse itself and it should more efficient as well as we're not walking the type twice & we're not having 2 copies of it. ## Tests New test where int64 field is initialized with large numbers. It fails on main.
1 parent 7c60885 commit f915956

File tree

12 files changed

+321
-27
lines changed

12 files changed

+321
-27
lines changed

‎acceptance/bundle/resources/jobs/big_id/databricks.yml‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
# Note, I'm not testing jobs here. I'm testing how encoder/decoders work with int64, job_id is just a field that came up.
2+
resources:
3+
jobs:
4+
foo:
5+
tasks:
6+
- task_key: max(int64)
7+
run_job_task:
8+
job_id: 9223372036854775807
9+
- task_key: max(int64) - 1
10+
run_job_task:
11+
job_id: 9223372036854775806
12+
- task_key: max(int64) - 2
13+
run_job_task:
14+
job_id: 9223372036854775805
15+
- task_key: min(int64)
16+
run_job_task:
17+
job_id: -9223372036854775808
18+
- task_key: min(int64) - 4
19+
run_job_task:
20+
job_id: -9223372036854775804

‎acceptance/bundle/resources/jobs/big_id/out.plan.direct.json‎

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
{
2+
"plan_version": 1,
3+
"cli_version": "[DEV_VERSION]",
4+
"plan": {
5+
"resources.jobs.foo": {
6+
"action": "create",
7+
"new_state": {
8+
"value": {
9+
"deployment": {
10+
"kind": "BUNDLE",
11+
"metadata_file_path": "/Workspace/Users/[USERNAME]/.bundle/test-bundle/default/state/metadata.json"
12+
},
13+
"edit_mode": "UI_LOCKED",
14+
"format": "MULTI_TASK",
15+
"max_concurrent_runs": 1,
16+
"name": "Untitled",
17+
"queue": {
18+
"enabled": true
19+
},
20+
"tasks": [
21+
{
22+
"run_job_task": {
23+
"job_id": [MAX_INT_64]
24+
},
25+
"task_key": "max(int64)"
26+
},
27+
{
28+
"run_job_task": {
29+
"job_id": [MAX_INT_64_MINUS_1]
30+
},
31+
"task_key": "max(int64) - 1"
32+
},
33+
{
34+
"run_job_task": {
35+
"job_id": [MAX_INT_64_MINUS_2]
36+
},
37+
"task_key": "max(int64) - 2"
38+
},
39+
{
40+
"run_job_task": {
41+
"job_id": [MIN_INT_64]
42+
},
43+
"task_key": "min(int64)"
44+
},
45+
{
46+
"run_job_task": {
47+
"job_id": [MIN_INT_64_MINUS_4]
48+
},
49+
"task_key": "min(int64) - 4"
50+
}
51+
]
52+
}
53+
}
54+
}
55+
}
56+
}

‎acceptance/bundle/resources/jobs/big_id/out.state.direct.json‎

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
{
2+
"state_version": 1,
3+
"cli_version": "[DEV_VERSION]",
4+
"lineage": "[UUID]",
5+
"serial": 2,
6+
"state": {
7+
"resources.jobs.foo": {
8+
"__id__": "[NUMID]",
9+
"state": {
10+
"deployment": {
11+
"kind": "BUNDLE",
12+
"metadata_file_path": "/Workspace/Users/[USERNAME]/.bundle/test-bundle/default/state/metadata.json"
13+
},
14+
"edit_mode": "UI_LOCKED",
15+
"format": "MULTI_TASK",
16+
"max_concurrent_runs": 1,
17+
"name": "Untitled",
18+
"queue": {
19+
"enabled": true
20+
},
21+
"tasks": [
22+
{
23+
"run_job_task": {
24+
"job_id": [MAX_INT_64]
25+
},
26+
"task_key": "max(int64)"
27+
},
28+
{
29+
"run_job_task": {
30+
"job_id": [MAX_INT_64_MINUS_1]
31+
},
32+
"task_key": "max(int64) - 1"
33+
},
34+
{
35+
"run_job_task": {
36+
"job_id": [MAX_INT_64_MINUS_2]
37+
},
38+
"task_key": "max(int64) - 2"
39+
},
40+
{
41+
"run_job_task": {
42+
"job_id": [MIN_INT_64]
43+
},
44+
"task_key": "min(int64)"
45+
},
46+
{
47+
"run_job_task": {
48+
"job_id": [MIN_INT_64_MINUS_4]
49+
},
50+
"task_key": "min(int64) - 4"
51+
}
52+
]
53+
}
54+
}
55+
}
56+
}

‎acceptance/bundle/resources/jobs/big_id/out.test.toml‎

Lines changed: 5 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎acceptance/bundle/resources/jobs/big_id/out.validate.json‎

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
{
2+
"jobs": {
3+
"foo": {
4+
"deployment": {
5+
"kind": "BUNDLE",
6+
"metadata_file_path": "/Workspace/Users/[USERNAME]/.bundle/test-bundle/default/state/metadata.json"
7+
},
8+
"edit_mode": "UI_LOCKED",
9+
"format": "MULTI_TASK",
10+
"max_concurrent_runs": 1,
11+
"name": "Untitled",
12+
"queue": {
13+
"enabled": true
14+
},
15+
"tasks": [
16+
{
17+
"run_job_task": {
18+
"job_id": [MAX_INT_64]
19+
},
20+
"task_key": "max(int64)"
21+
},
22+
{
23+
"run_job_task": {
24+
"job_id": [MAX_INT_64_MINUS_1]
25+
},
26+
"task_key": "max(int64) - 1"
27+
},
28+
{
29+
"run_job_task": {
30+
"job_id": [MAX_INT_64_MINUS_2]
31+
},
32+
"task_key": "max(int64) - 2"
33+
},
34+
{
35+
"run_job_task": {
36+
"job_id": [MIN_INT_64]
37+
},
38+
"task_key": "min(int64)"
39+
},
40+
{
41+
"run_job_task": {
42+
"job_id": [MIN_INT_64_MINUS_4]
43+
},
44+
"task_key": "min(int64) - 4"
45+
}
46+
]
47+
}
48+
}
49+
}

‎acceptance/bundle/resources/jobs/big_id/output.txt‎

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,80 @@
1+
2+
>>> [CLI] bundle validate -o json
3+
4+
>>> [CLI] bundle plan -o json
5+
Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/test-bundle/default/files...
6+
Deploying resources...
7+
Updating deployment state...
8+
Deployment complete!
9+
10+
>>> print_requests.py //jobs
11+
{
12+
"method": "POST",
13+
"path": "/api/2.2/jobs/create",
14+
"body": {
15+
"deployment": {
16+
"kind": "BUNDLE",
17+
"metadata_file_path": "/Workspace/Users/[USERNAME]/.bundle/test-bundle/default/state/metadata.json"
18+
},
19+
"edit_mode": "UI_LOCKED",
20+
"format": "MULTI_TASK",
21+
"max_concurrent_runs": 1,
22+
"name": "Untitled",
23+
"queue": {
24+
"enabled": true
25+
},
26+
"tasks": [
27+
{
28+
"run_job_task": {
29+
"job_id": [MAX_INT_64]
30+
},
31+
"task_key": "max(int64)"
32+
},
33+
{
34+
"run_job_task": {
35+
"job_id": [MAX_INT_64_MINUS_1]
36+
},
37+
"task_key": "max(int64) - 1"
38+
},
39+
{
40+
"run_job_task": {
41+
"job_id": [MAX_INT_64_MINUS_2]
42+
},
43+
"task_key": "max(int64) - 2"
44+
},
45+
{
46+
"run_job_task": {
47+
"job_id": [MIN_INT_64]
48+
},
49+
"task_key": "min(int64)"
50+
},
51+
{
52+
"run_job_task": {
53+
"job_id": [MIN_INT_64_MINUS_4]
54+
},
55+
"task_key": "min(int64) - 4"
56+
}
57+
]
58+
}
59+
}
60+
61+
>>> [CLI] bundle plan
62+
Plan: 0 to add, 0 to change, 0 to delete, 1 unchanged
63+
64+
>>> [CLI] bundle destroy --auto-approve
65+
The following resources will be deleted:
66+
delete resources.jobs.foo
67+
68+
All files and directories at the following location will be deleted: /Workspace/Users/[USERNAME]/.bundle/test-bundle/default
69+
70+
Deleting files...
71+
Destroy complete!
72+
73+
>>> print_requests.py //jobs
74+
{
75+
"method": "POST",
76+
"path": "/api/2.2/jobs/delete",
77+
"body": {
78+
"job_id": [NUMID]
79+
}
80+
}

‎acceptance/bundle/resources/jobs/big_id/script‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
trace $CLI bundle validate -o json | jq .resources > out.validate.json
2+
trace $CLI bundle plan -o json > out.plan.$DATABRICKS_BUNDLE_ENGINE.json
3+
$CLI bundle deploy
4+
trace print_requests.py //jobs
5+
print_state.py > out.state.$DATABRICKS_BUNDLE_ENGINE.json
6+
trace $CLI bundle plan | contains.py '0 to add, 0 to change, 0 to delete'
7+
trace $CLI bundle destroy --auto-approve
8+
trace print_requests.py //jobs

‎acceptance/bundle/resources/jobs/big_id/test.toml‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
# terraform fails with:
2+
# panic: Error reading level state: strconv.ParseInt: parsing "[NUMID]": value out of range
3+
EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ['direct']
4+
5+
[[Repls]]
6+
Old = '9223372036854775807'
7+
New = '[MAX_INT_64]'
8+
9+
[[Repls]]
10+
Old = '9223372036854775806'
11+
New = '[MAX_INT_64_MINUS_1]'
12+
13+
[[Repls]]
14+
Old = '9223372036854775805'
15+
New = '[MAX_INT_64_MINUS_2]'
16+
17+
[[Repls]]
18+
Old = '-9223372036854775808'
19+
New = '[MIN_INT_64]'
20+
21+
[[Repls]]
22+
Old = '-9223372036854775804'
23+
New = '[MIN_INT_64_MINUS_4]'

‎bundle/direct/apply.go‎

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
package direct
22

33
import (
4-
"bytes"
54
"context"
65
"encoding/json"
76
"errors"
@@ -210,15 +209,9 @@ func (d *DeploymentUnit) Resize(ctx context.Context, db *dstate.DeploymentState,
210209
return nil
211210
}
212211

213-
func typeConvert(destType reflect.Type, src any) (any, error) {
214-
raw, err := json.Marshal(src)
215-
if err != nil {
216-
return nil, fmt.Errorf("marshalling: %w", err)
217-
}
218-
212+
func parseState(destType reflect.Type, raw json.RawMessage) (any, error) {
219213
destPtr := reflect.New(destType).Interface()
220-
dec := json.NewDecoder(bytes.NewReader(raw))
221-
err = dec.Decode(destPtr)
214+
err := json.Unmarshal(raw, destPtr)
222215
if err != nil {
223216
return nil, fmt.Errorf("unmarshalling into %s: %w", destType, err)
224217
}

‎bundle/direct/bundle_plan.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,7 @@ func (b *DeploymentBundle) CalculatePlan(ctx context.Context, client *databricks
133133
return false
134134
}
135135

136-
savedState, err := typeConvert(adapter.StateType(), dbentry.State)
136+
savedState, err := parseState(adapter.StateType(), dbentry.State)
137137
if err != nil {
138138
logdiag.LogError(ctx, fmt.Errorf("%s: interpreting state: %w", errorPrefix, err))
139139
return false

0 commit comments

Comments
 (0)