Skip to content

Commit 6b72590

Browse files
nfxnfx
authored
Increase coverage for storage package (#992)
1 parent 71a6984 commit 6b72590

File tree

15 files changed

+527
-303
lines changed

15 files changed

+527
-303
lines changed

‎.vscode/testing.code-snippets‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,5 +54,12 @@
5454
"assert.NoError(t, err)"
5555
],
5656
"description": "assert.NoError"
57-
}
57+
},
58+
"err :=": {
59+
"prefix": "e",
60+
"body": [
61+
"err := "
62+
],
63+
"description": "err :="
64+
}
5865
}

‎clusters/clusters_api.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -445,7 +445,7 @@ type ClusterInfo struct {
445445
LastStateLossTime int64 `json:"last_state_loss_time,omitempty"`
446446
LastActivityTime int64 `json:"last_activity_time,omitempty"`
447447
ClusterMemoryMb int64 `json:"cluster_memory_mb,omitempty"`
448-
ClusterCores float32 `json:"cluster_cores,omitempty"`
448+
ClusterCores float64 `json:"cluster_cores,omitempty"`
449449
DefaultTags map[string]string `json:"default_tags"`
450450
ClusterLogStatus *LogSyncStatus `json:"cluster_log_status,omitempty"`
451451
TerminationReason *TerminationReason `json:"termination_reason,omitempty"`

‎exporter/importables_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -180,7 +180,7 @@ func TestDbfsFileCornerCases_ReadFail(t *testing.T) {
180180
err := resourcesMap["databricks_dbfs_file"].Body(ic, nil, &resource{
181181
ID: "a",
182182
})
183-
assert.EqualError(t, err, "nope")
183+
assert.EqualError(t, err, "cannot read a: nope")
184184
})
185185
}
186186

‎provider/provider.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ func DatabricksProvider() *schema.Provider {
7575
"databricks_metastore_data_access": catalog.ResourceDataAccessConfiguration(),
7676
"databricks_mlflow_experiment": mlflow.ResourceMLFlowExperiment(),
7777
"databricks_mlflow_model": mlflow.ResourceMLFlowModel(),
78-
"databricks_mount": storage.ResourceDatabricksMount(),
78+
"databricks_mount": storage.ResourceMount(),
7979
"databricks_mws_customer_managed_keys": mws.ResourceCustomerManagedKey(),
8080
"databricks_mws_credentials": mws.ResourceCredentials(),
8181
"databricks_mws_log_delivery": mws.ResourceLogDelivery(),

‎storage/dbfs.go‎

Lines changed: 21 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"bytes"
55
"context"
66
"encoding/base64"
7+
"fmt"
78

89
"github.com/databrickslabs/terraform-provider-databricks/common"
910
)
@@ -20,19 +21,19 @@ type FileInfo struct {
2021
FileSize int64 `json:"file_size,omitempty"`
2122
}
2223

23-
// CreateHandle contains the payload to create a handle which is a connection for uploading blocks of file data
24-
type CreateHandle struct {
24+
// createHandle contains the payload to create a handle which is a connection for uploading blocks of file data
25+
type createHandle struct {
2526
Path string `json:"path,omitempty"`
2627
Overwrite bool `json:"overwrite,omitempty"`
2728
}
2829

29-
// Handle contains the response from making an handle request
30-
type Handle struct {
30+
// handleResponse contains the response from making an handle request
31+
type handleResponse struct {
3132
Handle int64 `json:"handle,omitempty"`
3233
}
3334

34-
// AddBlock contains the payload to upload a block of base64 data to a handle
35-
type AddBlock struct {
35+
// addBlock contains the payload to upload a block of base64 data to a handle
36+
type addBlock struct {
3637
Data string `json:"data,omitempty"`
3738
Handle int64 `json:"handle,omitempty"`
3839
}
@@ -55,18 +56,19 @@ type DbfsAPI struct {
5556
}
5657

5758
// Create creates a file on DBFS
58-
func (a DbfsAPI) Create(path string, byteArr []byte, overwrite bool) (err error) {
59+
func (a DbfsAPI) Create(path string, contents []byte, overwrite bool) (err error) {
5960
handle, err := a.createHandle(path, overwrite)
6061
if err != nil {
62+
err = fmt.Errorf("cannot create handle: %w", err)
6163
return
6264
}
6365
defer func() {
6466
cerr := a.closeHandle(handle)
6567
if cerr != nil {
66-
err = cerr
68+
err = fmt.Errorf("cannot close handle: %w", cerr)
6769
}
6870
}()
69-
buffer := bytes.NewBuffer(byteArr)
71+
buffer := bytes.NewBuffer(contents)
7072
for {
7173
byteChunk := buffer.Next(1e6)
7274
if len(byteChunk) == 0 {
@@ -75,24 +77,24 @@ func (a DbfsAPI) Create(path string, byteArr []byte, overwrite bool) (err error)
7577
b64Data := base64.StdEncoding.EncodeToString(byteChunk)
7678
err = a.addBlock(b64Data, handle)
7779
if err != nil {
78-
return
80+
err = fmt.Errorf("cannot add block: %w", err)
7981
}
8082
}
8183
return
8284
}
8385

8486
func (a DbfsAPI) createHandle(path string, overwrite bool) (int64, error) {
85-
var h Handle
86-
err := a.client.Post(a.context, "/dbfs/create", CreateHandle{path, overwrite}, &h)
87+
var h handleResponse
88+
err := a.client.Post(a.context, "/dbfs/create", createHandle{path, overwrite}, &h)
8789
return h.Handle, err
8890
}
8991

9092
func (a DbfsAPI) addBlock(data string, handle int64) error {
91-
return a.client.Post(a.context, "/dbfs/add-block", AddBlock{data, handle}, nil)
93+
return a.client.Post(a.context, "/dbfs/add-block", addBlock{data, handle}, nil)
9294
}
9395

9496
func (a DbfsAPI) closeHandle(handle int64) error {
95-
return a.client.Post(a.context, "/dbfs/close", Handle{handle}, nil)
97+
return a.client.Post(a.context, "/dbfs/close", handleResponse{handle}, nil)
9698
}
9799

98100
// List returns a list of files in DBFS and the recursive flag lets you recursively list files
@@ -119,7 +121,7 @@ func (a DbfsAPI) recursiveAddPaths(path string, pathList *[]FileInfo) error {
119121
} else if v.IsDir {
120122
err := a.recursiveAddPaths(v.Path, pathList)
121123
if err != nil {
122-
return err
124+
return fmt.Errorf("cannot list subfolder: %w", err)
123125
}
124126
}
125127
}
@@ -131,17 +133,12 @@ func (a DbfsAPI) list(path string) ([]FileInfo, error) {
131133
err := a.client.Get(a.context, "/dbfs/list", map[string]interface{}{
132134
"path": path,
133135
}, &dbfsList)
136+
if err != nil {
137+
err = fmt.Errorf("cannot list %s: %w", path, err)
138+
}
134139
return dbfsList.Files, err
135140
}
136141

137-
// Move moves the file between DBFS locations via DBFS api
138-
func (a DbfsAPI) Move(src string, tgt string) error {
139-
return a.client.Post(a.context, "/dbfs/move", map[string]string{
140-
"source_path": src,
141-
"destination_path": tgt,
142-
}, nil)
143-
}
144-
145142
// Delete deletes a file in DBFS via API
146143
func (a DbfsAPI) Delete(path string, recursive bool) error {
147144
return a.client.Post(a.context, "/dbfs/delete", dbfsRequest{
@@ -165,7 +162,7 @@ func (a DbfsAPI) Read(path string) (content []byte, err error) {
165162
for fetchLoop {
166163
bytesRead, bytes, err := a.read(path, offSet, length)
167164
if err != nil {
168-
return content, err
165+
return content, fmt.Errorf("cannot read %s: %w", path, err)
169166
}
170167
if bytesRead == 0 || bytesRead < length {
171168
fetchLoop = false
@@ -203,10 +200,3 @@ func (a DbfsAPI) Status(path string) (f FileInfo, err error) {
203200
}, &f)
204201
return
205202
}
206-
207-
// Mkdirs makes the directories in DBFS include the parent paths
208-
func (a DbfsAPI) Mkdirs(path string) error {
209-
return a.client.Post(a.context, "/dbfs/mkdirs", map[string]interface{}{
210-
"path": path,
211-
}, nil)
212-
}

‎storage/dbfs_test.go‎

Lines changed: 134 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -8,11 +8,142 @@ import (
88
"testing"
99

1010
"github.com/databrickslabs/terraform-provider-databricks/common"
11+
"github.com/databrickslabs/terraform-provider-databricks/qa"
1112
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/acctest"
1213
"github.com/stretchr/testify/assert"
1314
)
1415

15-
func GenString(times int) []byte {
16+
func TestCreateFileFails(t *testing.T) {
17+
qa.HTTPFixturesApply(t, []qa.HTTPFixture{
18+
{
19+
Method: "POST",
20+
Resource: "/api/2.0/dbfs/create",
21+
ExpectedRequest: createHandle{
22+
Path: "/create-fails",
23+
Overwrite: true,
24+
},
25+
Status: 404,
26+
Response: common.NotFound("fails"),
27+
},
28+
{
29+
Method: "POST",
30+
Resource: "/api/2.0/dbfs/close",
31+
},
32+
}, func(ctx context.Context, client *common.DatabricksClient) {
33+
a := NewDbfsAPI(ctx, client)
34+
err := a.Create("/create-fails", []byte("abc"), true)
35+
assert.EqualError(t, err, "cannot create handle: fails")
36+
})
37+
}
38+
39+
func TestCreateFile_AddBlockFails(t *testing.T) {
40+
qa.HTTPFixturesApply(t, []qa.HTTPFixture{
41+
{
42+
Method: "POST",
43+
Resource: "/api/2.0/dbfs/create",
44+
ExpectedRequest: createHandle{
45+
Path: "/add-fails",
46+
Overwrite: true,
47+
},
48+
Response: handleResponse{123},
49+
},
50+
{
51+
Method: "POST",
52+
Resource: "/api/2.0/dbfs/add-block",
53+
ExpectedRequest: addBlock{
54+
Data: "YWJj",
55+
Handle: 123,
56+
},
57+
Status: 404,
58+
Response: common.NotFound("fails"),
59+
},
60+
{
61+
Method: "POST",
62+
Resource: "/api/2.0/dbfs/close",
63+
},
64+
}, func(ctx context.Context, client *common.DatabricksClient) {
65+
a := NewDbfsAPI(ctx, client)
66+
err := a.Create("/add-fails", []byte("abc"), true)
67+
assert.EqualError(t, err, "cannot add block: fails")
68+
})
69+
}
70+
71+
func TestCreateFile_CloseFails(t *testing.T) {
72+
qa.HTTPFixturesApply(t, []qa.HTTPFixture{
73+
{
74+
Method: "POST",
75+
Resource: "/api/2.0/dbfs/create",
76+
},
77+
{
78+
Method: "POST",
79+
Resource: "/api/2.0/dbfs/add-block",
80+
},
81+
{
82+
Method: "POST",
83+
Resource: "/api/2.0/dbfs/close",
84+
Status: 404,
85+
Response: common.NotFound("fails"),
86+
},
87+
}, func(ctx context.Context, client *common.DatabricksClient) {
88+
a := NewDbfsAPI(ctx, client)
89+
err := a.Create("/close-fails", []byte("abc"), true)
90+
assert.EqualError(t, err, "cannot close handle: fails")
91+
})
92+
}
93+
94+
func TestDbfsListRecursiveFails(t *testing.T) {
95+
qa.HTTPFixturesApply(t, []qa.HTTPFixture{
96+
{
97+
Method: "GET",
98+
Resource: "/api/2.0/dbfs/list?path=abc",
99+
Status: 404,
100+
Response: common.NotFound("fails"),
101+
},
102+
{
103+
Method: "GET",
104+
Resource: "/api/2.0/dbfs/list?path=sub",
105+
Response: FileList{
106+
Files: []FileInfo{
107+
{
108+
Path: "bcd",
109+
IsDir: true,
110+
},
111+
},
112+
},
113+
},
114+
{
115+
Method: "GET",
116+
ReuseRequest: true,
117+
Resource: "/api/2.0/dbfs/list?path=bcd",
118+
Status: 404,
119+
Response: common.NotFound("fails"),
120+
},
121+
}, func(ctx context.Context, client *common.DatabricksClient) {
122+
a := NewDbfsAPI(ctx, client)
123+
_, err := a.List("abc", true)
124+
assert.EqualError(t, err, "cannot list abc: fails")
125+
_, err = a.List("sub", true)
126+
assert.EqualError(t, err, "cannot list subfolder: cannot list bcd: fails")
127+
_, err = a.List("bcd", false)
128+
assert.EqualError(t, err, "cannot list bcd: fails")
129+
})
130+
}
131+
132+
func TestDbfsReadFails(t *testing.T) {
133+
qa.HTTPFixturesApply(t, []qa.HTTPFixture{
134+
{
135+
MatchAny: true,
136+
Status: 404,
137+
Response: common.NotFound("fails"),
138+
},
139+
}, func(ctx context.Context, client *common.DatabricksClient) {
140+
a := NewDbfsAPI(ctx, client)
141+
_, err := a.Read("abc")
142+
assert.EqualError(t, err, "cannot read abc: fails")
143+
})
144+
}
145+
146+
func genString(times int) []byte {
16147
var buf bytes.Buffer
17148
for i := 0; i < times; i++ {
18149
buf.WriteString("Hello world how are you doing?\n")
@@ -27,22 +158,16 @@ func TestAccCreateFile(t *testing.T) {
27158

28159
randomName := acctest.RandStringFromCharSet(10, acctest.CharSetAlphaNum)
29160
dir := "/client-test/" + randomName
30-
dir2 := dir + "/dir2/"
31161
path := dir + "/randomfile"
32162
path2 := dir + "/dir2/randomfile"
33163
path3 := dir + "/dir2/randomfile2"
34164

35-
randomStr := GenString(500)
165+
randomStr := genString(10)
36166
client := common.NewClientFromEnvironment()
37167

38168
dbfsAPI := NewDbfsAPI(context.Background(), client)
39-
err := dbfsAPI.Mkdirs(dir)
40-
assert.NoError(t, err, err)
41-
42-
err = dbfsAPI.Mkdirs(dir2)
43-
assert.NoError(t, err, err)
44169

45-
err = dbfsAPI.Create(path, randomStr, true)
170+
err := dbfsAPI.Create(path, randomStr, true)
46171
assert.NoError(t, err, err)
47172

48173
err = dbfsAPI.Create(path2, randomStr, true)

0 commit comments

Comments
 (0)