Skip to content

Commit

Permalink
Cover with tests
Browse files Browse the repository at this point in the history
  • Loading branch information
nfx committed Dec 20, 2021
1 parent 537fc36 commit 74c0c8b
Show file tree
Hide file tree
Showing 9 changed files with 135 additions and 44 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
# Version changelog

## 0.4.2

* Added `DBC` format support for `databricks_notebook` ([#989](https://github.com/databrickslabs/terraform-provider-databricks/pull/989)).

## 0.4.1

* Added `databricks_library` resource to install library on `databricks_cluster` ([#904](https://github.com/databrickslabs/terraform-provider-databricks/pull/904)).
Expand Down
11 changes: 10 additions & 1 deletion docs/resources/notebook.md
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,16 @@ resource "databricks_notebook" "notebook" {
language = "PYTHON"
}
```


You can also manage [Databricks Archives](https://docs.databricks.com/notebooks/notebooks-manage.html#databricks-archive) to import the whole folders of notebooks statically. Whenever you update `.dbc` file, terraform-managed notebook folder is removed and replaced with contents of the new `.dbc` file. You are strongly advised to use `.dbc` format only with `source` attribute of the resource:

```hcl
resource "databricks_notebook" "lesson" {
source = "${path.module}/IntroNotebooks.dbc"
path = "/Shared/Intro"
}
```

## Argument Reference

-> **Note** Notebook on Databricks workspace would only be changed, if Terraform stage did change. This means that any manual changes to managed notebook won't be overwritten by Terraform, if there's no local change to notebook sources. Notebooks are identified by their path, so changing notebook's name manually on the workspace and then applying Terraform state would result in creation of notebook from Terraform state.
Expand Down
2 changes: 1 addition & 1 deletion permissions/acceptance/api_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -267,7 +267,7 @@ func TestAccPermissionsNotebooks(t *testing.T) {

notebookPath := fmt.Sprintf("%s/Dummy", notebookDir)

err = workspaceAPI.Create(workspace.ImportRequest{
err = workspaceAPI.Create(workspace.ImportPath{
Path: notebookPath,
Content: "MSsx",
Format: "SOURCE",
Expand Down
6 changes: 0 additions & 6 deletions workspace/data_notebook_paths.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,6 @@ func DataSourceNotebookPaths() *schema.Resource {
return diag.FromErr(err)
}
d.SetId(path)
if err = d.Set("recursive", recursive); err != nil {
return diag.FromErr(err)
}
if err = d.Set("path", path); err != nil {
return diag.FromErr(err)
}
var notebookPathList []map[string]string
for _, v := range notebookList {
notebookPathMap := map[string]string{}
Expand Down
43 changes: 37 additions & 6 deletions workspace/data_notebook_paths_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,10 @@ import (
"testing"

"github.com/databrickslabs/terraform-provider-databricks/qa"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

func TestDataSourceNotebookPaths(t *testing.T) {
d, err := qa.ResourceFixture{
qa.ResourceFixture{
Fixtures: []qa.HTTPFixture{
{
Method: "GET",
Expand Down Expand Up @@ -53,7 +51,40 @@ func TestDataSourceNotebookPaths(t *testing.T) {
"path": "/a/b/c",
"recursive": true,
},
}.Apply(t)
require.NoError(t, err)
assert.Equal(t, "/a/b/c", d.Id())
}.ApplyNoError(t)
}

func TestDataSourceNotebookPaths_NoRecursive(t *testing.T) {
qa.ResourceFixture{
Fixtures: []qa.HTTPFixture{
{
Method: "GET",
Resource: "/api/2.0/workspace/list?path=%2Fa%2Fb%2Fc",
Response: objectList{
Objects: []ObjectStatus{
{
ObjectID: 988,
ObjectType: Notebook,
Language: Python,
Path: "/a/b/c/d/e",
},
{
ObjectID: 989,
ObjectType: Notebook,
Language: SQL,
Path: "/a/b/c/d/f",
},
},
},
},
},
Read: true,
NonWritable: true,
Resource: DataSourceNotebookPaths(),
ID: ".",
State: map[string]interface{}{
"path": "/a/b/c",
"recursive": false,
},
}.ApplyNoError(t)
}
2 changes: 1 addition & 1 deletion workspace/data_notebook_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ func TestDataSourceNotebook(t *testing.T) {
{
Method: "GET",
Resource: "/api/2.0/workspace/export?format=SOURCE&path=%2Fa%2Fb%2Fc",
Response: NotebookContent{
Response: ExportPath{
Content: "SGVsbG8gd29ybGQK",
},
},
Expand Down
4 changes: 2 additions & 2 deletions workspace/resource_directory_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ func TestResourceDirectoryDelete(t *testing.T) {
Method: http.MethodPost,
Resource: "/api/2.0/workspace/delete",
Status: http.StatusOK,
ExpectedRequest: NotebookDeleteRequest{Path: path, Recursive: delete_recursive},
ExpectedRequest: DeletePath{Path: path, Recursive: delete_recursive},
},
},
Resource: ResourceDirectory(),
Expand Down Expand Up @@ -172,7 +172,7 @@ func TestResourceDirectoryDelete_Error(t *testing.T) {
{
Method: "POST",
Resource: "/api/2.0/workspace/delete",
ExpectedRequest: NotebookDeleteRequest{Path: path, Recursive: false},
ExpectedRequest: DeletePath{Path: path, Recursive: false},
Response: common.APIErrorBody{
ErrorCode: "INVALID_REQUEST",
Message: "Internal error happened",
Expand Down
44 changes: 22 additions & 22 deletions workspace/resource_notebook.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,22 +45,22 @@ type ObjectStatus struct {
Language string `json:"language,omitempty"`
}

// NotebookContent contains the base64 content of the notebook
type NotebookContent struct {
// ExportPath contains the base64 content of the notebook
type ExportPath struct {
Content string `json:"content,omitempty"`
}

// ImportRequest contains the payload to import a notebook
type ImportRequest struct {
// ImportPath contains the payload to import a notebook
type ImportPath struct {
Content string `json:"content"`
Path string `json:"path"`
Language string `json:"language,omitempty"`
Format string `json:"format,omitempty"`
Overwrite bool `json:"overwrite,omitempty"`
}

// NotebookDeleteRequest contains the payload to delete a notebook
type NotebookDeleteRequest struct {
// DeletePath contains the payload to delete a notebook
type DeletePath struct {
Path string `json:"path,omitempty"`
Recursive bool `json:"recursive,omitempty"`
}
Expand All @@ -83,7 +83,7 @@ type NotebooksAPI struct {
var mtx = &sync.Mutex{}

// Create creates a notebook given the content and path
func (a NotebooksAPI) Create(r ImportRequest) error {
func (a NotebooksAPI) Create(r ImportPath) error {
mtx.Lock()
defer mtx.Unlock()
return a.client.Post(a.context, "/workspace/import", r, nil)
Expand All @@ -105,7 +105,7 @@ type workspacePathRequest struct {

// Export returns the notebook content as a base64 string
func (a NotebooksAPI) Export(path string, format string) (string, error) {
var notebookContent NotebookContent
var notebookContent ExportPath
err := a.client.Get(a.context, "/workspace/export", workspacePathRequest{
Format: format,
Path: path,
Expand Down Expand Up @@ -174,7 +174,7 @@ func (a NotebooksAPI) list(path string) ([]ObjectStatus, error) {
func (a NotebooksAPI) Delete(path string, recursive bool) error {
mtx.Lock()
defer mtx.Unlock()
return a.client.Post(a.context, "/workspace/delete", NotebookDeleteRequest{
return a.client.Post(a.context, "/workspace/delete", DeletePath{
Path: path,
Recursive: recursive,
}, nil)
Expand Down Expand Up @@ -245,7 +245,7 @@ func ResourceNotebook() *schema.Resource {
return err
}
}
createNotebook := ImportRequest{
createNotebook := ImportPath{
Content: base64.StdEncoding.EncodeToString(content),
Language: d.Get("language").(string),
Format: d.Get("format").(string),
Expand Down Expand Up @@ -284,26 +284,26 @@ func ResourceNotebook() *schema.Resource {
if err != nil {
return err
}
err = notebooksAPI.Create(ImportRequest{
Content: base64.StdEncoding.EncodeToString(content),
Language: d.Get("language").(string),
Format: d.Get("format").(string),
Overwrite: true,
Path: d.Id(),
})
// INVALID_PARAMETER_VALUE: Overwrite cannot be used for source format when importing a folder
if err != nil && strings.Contains(err.Error(), "Overwrite cannot be used") {
format := d.Get("format").(string)
if format == "DBC" {
// Overwrite cannot be used for source format when importing a folder
err = notebooksAPI.Delete(d.Id(), true)
if err != nil {
return err
}
return notebooksAPI.Create(ImportRequest{
return notebooksAPI.Create(ImportPath{
Content: base64.StdEncoding.EncodeToString(content),
Format: "DBC",
Format: format,
Path: d.Id(),
})
}
return err
return notebooksAPI.Create(ImportPath{
Content: base64.StdEncoding.EncodeToString(content),
Language: d.Get("language").(string),
Format: format,
Overwrite: true,
Path: d.Id(),
})
},
Delete: func(ctx context.Context, d *schema.ResourceData, c *common.DatabricksClient) error {
return NewNotebooksAPI(ctx, c).Delete(d.Id(), true)
Expand Down
63 changes: 58 additions & 5 deletions workspace/resource_notebook_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ func TestResourceNotebookDelete(t *testing.T) {
Method: http.MethodPost,
Resource: "/api/2.0/workspace/delete",
Status: http.StatusOK,
ExpectedRequest: NotebookDeleteRequest{Path: path, Recursive: true},
ExpectedRequest: DeletePath{Path: path, Recursive: true},
},
},
Resource: ResourceNotebook(),
Expand Down Expand Up @@ -112,7 +112,7 @@ func TestResourceNotebookCreate(t *testing.T) {
{
Method: http.MethodPost,
Resource: "/api/2.0/workspace/import",
ExpectedRequest: ImportRequest{
ExpectedRequest: ImportPath{
Content: "YWJjCg==",
Path: "/foo/path.py",
Language: "PYTHON",
Expand All @@ -123,7 +123,7 @@ func TestResourceNotebookCreate(t *testing.T) {
{
Method: http.MethodGet,
Resource: "/api/2.0/workspace/export?format=SOURCE&path=%2Ffoo%2Fpath.py",
Response: NotebookContent{
Response: ExportPath{
Content: "YWJjCg==",
},
},
Expand Down Expand Up @@ -156,7 +156,7 @@ func TestResourceNotebookCreateSource(t *testing.T) {
{
Method: http.MethodPost,
Resource: "/api/2.0/workspace/import",
ExpectedRequest: ImportRequest{
ExpectedRequest: ImportPath{
Content: "LS0gRGF0YWJyaWNrcyBub3RlYm9vayBzb3VyY2UKU0VMRUNUIDEwKjIwC" +
"gotLSBDT01NQU5EIC0tLS0tLS0tLS0KClNFTEVDVCAyMCoxMDAKCi0tIE" +
"NPTU1BTkQgLS0tLS0tLS0tLQoKCg==",
Expand Down Expand Up @@ -240,7 +240,7 @@ func TestResourceNotebookUpdate(t *testing.T) {
{
Method: "POST",
Resource: "/api/2.0/workspace/import",
ExpectedRequest: ImportRequest{
ExpectedRequest: ImportPath{
Format: "SOURCE",
Overwrite: true,
Content: "YWJjCg==",
Expand Down Expand Up @@ -270,3 +270,56 @@ func TestResourceNotebookUpdate(t *testing.T) {
Update: true,
}.ApplyNoError(t)
}

func TestResourceNotebookUpdate_DBC(t *testing.T) {
qa.ResourceFixture{
Fixtures: []qa.HTTPFixture{
{
Method: "POST",
Resource: "/api/2.0/workspace/delete",
ExpectedRequest: DeletePath{
Recursive: true,
Path: "abc",
},
},
{
Method: "POST",
Resource: "/api/2.0/workspace/import",
ExpectedRequest: ImportPath{
Format: "DBC",
Content: "YWJjCg==",
Path: "abc",
},
},
{
Method: http.MethodGet,
Resource: "/api/2.0/workspace/get-status?path=abc",
Response: ObjectStatus{
ObjectID: 4567,
ObjectType: Directory,
Path: "abc",
},
},
},
Resource: ResourceNotebook(),
State: map[string]interface{}{
"content_base64": "YWJjCg==",

// technically language is not needed, but makes the test simpler
"language": "PYTHON",
"format": "DBC",
"path": "/path.py",
},
ID: "abc",
RequiresNew: true,
Update: true,
}.ApplyNoError(t)
}

func TestNotebookLanguageSuppressSourceDiff(t *testing.T) {
r := ResourceNotebook()
d := r.TestResourceData()
d.Set("source", "this.PY")
suppress := r.Schema["language"].DiffSuppressFunc
assert.True(t, suppress("language", Python, Python, d))
}

0 comments on commit 74c0c8b

Please sign in to comment.