[Feature] Skip calling Read after Create/Update operations for notebooks (#4173)

## Changes
<!-- Summary of your changes that are easy to understand -->

It's based on #4190.

It was found that the `import` API returns `object_id` as a result of
its execution, so we don't really need to call the `Read` operation that
will call `get-status` to fill all attributes (when we create a notebook
using the `SOURCE` format).

The other formats (`DBC` and `JUPYTER`) will still call the `Read` to
fill in the missing parameters, such as `language`.

It also changed the `language` to `optional,computed` so we can safely
set the value and don't trigger the diff.

This should help when we import a large number of notebooks, i.e., when
applying exported resources.

## Tests
<!-- 
How is this tested? Please see the checklist below and also describe any
other relevant tests
-->

- [x] `make test` run locally
- [ ] ~relevant change in `docs/` folder~
- [x] covered with integration tests in `internal/acceptance`
- [x] relevant acceptance tests are passing
- [ ] ~using Go SDK~

---------

Co-authored-by: Omer Lachish <[email protected]>

Author: alexott

exporter/importables.go

@@ -1485,7 +1485,15 @@ var resourcesMap map[string]importable = map[string]importable{
 			ic.emitWorkspaceObjectParentDirectory(r)
 			return r.Data.Set("source", fileName)
 		},
-		ShouldOmitField: shouldOmitMd5Field,
+		ShouldOmitField: func(ic *importContext, pathString string, as *schema.Schema, d *schema.ResourceData) bool {
+			switch pathString {
+			case "language":
+				return d.Get("language") == ""
+			case "format":
+				return d.Get("format") == "SOURCE"
+			}
+			return shouldOmitMd5Field(ic, pathString, as, d)
+		},
 		Depends: []reference{
 			{Path: "source", File: true},
 			{Path: "path", Resource: "databricks_directory", MatchType: MatchLongestPrefix,

internal/acceptance/notebook_test.go

@@ -17,3 +17,31 @@ func TestAccNotebookResourceScalability(t *testing.T) {
 		}`,
 	})
 }
+
+func TestAccNotebookResourceDbcUpdate(t *testing.T) {
+	WorkspaceLevel(t, Step{
+		Template: `resource "databricks_notebook" "this" {
+			source = "{var.CWD}/../../workspace/acceptance/testdata/acc-test-update1.dbc"
+			path = "/Shared/provider-test/dbc_{var.STICKY_RANDOM}"
+		}`,
+	}, Step{
+		Template: `resource "databricks_notebook" "this" {
+			source = "{var.CWD}/../../workspace/acceptance/testdata/acc-test-update2.dbc"
+			path = "/Shared/provider-test/dbc_{var.STICKY_RANDOM}"
+		}`,
+	})
+}
+
+func TestAccNotebookResourceJupiterUpdate(t *testing.T) {
+	WorkspaceLevel(t, Step{
+		Template: `resource "databricks_notebook" "this" {
+			source = "{var.CWD}/../../workspace/acceptance/testdata/acc-test-update1.ipynb"
+			path = "/Shared/provider-test/jupiter_{var.STICKY_RANDOM}"
+		}`,
+	}, Step{
+		Template: `resource "databricks_notebook" "this" {
+			source = "{var.CWD}/../../workspace/acceptance/testdata/acc-test-update2.ipynb"
+			path = "/Shared/provider-test/jupiter_{var.STICKY_RANDOM}"
+		}`,
+	})
+}

workspace/acceptance/testdata/README.md

@@ -0,0 +1,9 @@
+# Test files for acceptance tests
+
+- Test to emulate notebook update logic for notebooks in `JUPYTER` format:
+  - `acc-test-update1.ipynb` - original file used for notebook creation
+  - `acc-test-update2.ipynb` - modified notebook, triggering notebook update.
+- Test to emulate notebook update logic for notebooks in `DBC` format:
+  - `acc-test-update1.dbc` - original file used for notebook creation
+  - `acc-test-update2.dbc` - modified notebook, triggering notebook update.
+

workspace/acceptance/testdata/acc-test-update1.dbc

@@ -0,0 +1,42 @@
+{
+ "cells": [
+  {
+   "cell_type": "code",
+   "execution_count": 0,
+   "metadata": {
+    "application/vnd.databricks.v1+cell": {
+     "cellMetadata": {},
+     "inputWidgets": {},
+     "nuid": "b921d27d-a2bd-4ec8-bc2d-92ccc095246f",
+     "showTitle": false,
+     "tableResultSettingsMap": {},
+     "title": ""
+    }
+   },
+   "outputs": [],
+   "source": [
+    "print(\"Hello World\")"
+   ]
+  }
+ ],
+ "metadata": {
+  "application/vnd.databricks.v1+notebook": {
+   "dashboards": [],
+   "environmentMetadata": {
+    "base_environment": "",
+    "client": "1"
+   },
+   "language": "python",
+   "notebookMetadata": {
+    "pythonIndentUnit": 4
+   },
+   "notebookName": "1",
+   "widgets": {}
+  },
+  "language_info": {
+   "name": "python"
+  }
+ },
+ "nbformat": 4,
+ "nbformat_minor": 0
+}

workspace/acceptance/testdata/acc-test-update1.ipynb

@@ -0,0 +1,60 @@
+{
+ "cells": [
+  {
+   "cell_type": "code",
+   "execution_count": 0,
+   "metadata": {
+    "application/vnd.databricks.v1+cell": {
+     "cellMetadata": {},
+     "inputWidgets": {},
+     "nuid": "769d5d72-59f3-417b-9f8c-08785a9c4423",
+     "showTitle": false,
+     "tableResultSettingsMap": {},
+     "title": ""
+    }
+   },
+   "outputs": [],
+   "source": [
+    "print(\"First Hello\")"
+   ]
+  },
+  {
+   "cell_type": "code",
+   "execution_count": 0,
+   "metadata": {
+    "application/vnd.databricks.v1+cell": {
+     "cellMetadata": {},
+     "inputWidgets": {},
+     "nuid": "b921d27d-a2bd-4ec8-bc2d-92ccc095246f",
+     "showTitle": false,
+     "tableResultSettingsMap": {},
+     "title": ""
+    }
+   },
+   "outputs": [],
+   "source": [
+    "print(\"Hello World\")"
+   ]
+  }
+ ],
+ "metadata": {
+  "application/vnd.databricks.v1+notebook": {
+   "dashboards": [],
+   "environmentMetadata": {
+    "base_environment": "",
+    "client": "1"
+   },
+   "language": "python",
+   "notebookMetadata": {
+    "pythonIndentUnit": 4
+   },
+   "notebookName": "1",
+   "widgets": {}
+  },
+  "language_info": {
+   "name": "python"
+  }
+ },
+ "nbformat": 4,
+ "nbformat_minor": 0
+}

workspace/acceptance/testdata/acc-test-update2.dbc

@@ -63,6 +63,10 @@ type ObjectStatus struct {
 	ResourceId            string                 `json:"resource_id,omitempty"`
 }
 
+type ImportResponse struct {
+	ObjectID int64 `json:"object_id,omitempty"`
+}
+
 // ExportPath contains the base64 content of the notebook
 type ExportPath struct {
 	Content string `json:"content,omitempty"`
@@ -101,12 +105,14 @@ type NotebooksAPI struct {
 var mtx = &sync.Mutex{}
 
 // Create creates a notebook given the content and path
-func (a NotebooksAPI) Create(r ImportPath) error {
+func (a NotebooksAPI) Create(r ImportPath) (ImportResponse, error) {
 	if r.Format == "DBC" {
 		mtx.Lock()
 		defer mtx.Unlock()
 	}
-	return a.client.Post(a.context, "/workspace/import", r, nil)
+	var response ImportResponse
+	err := a.client.Post(a.context, "/workspace/import", r, &response)
+	return response, err
 }
 
 // Read returns the notebook metadata and not the contents
@@ -215,31 +221,29 @@ func (a NotebooksAPI) Delete(path string, recursive bool) error {
 	}, nil)
 }
 
+func setComputedProperties(d *schema.ResourceData, c *common.DatabricksClient) {
+	d.Set("url", c.FormatURL("#workspace", d.Id()))
+	d.Set("workspace_path", "/Workspace"+d.Id())
+}
+
 // ResourceNotebook manages notebooks
 func ResourceNotebook() common.Resource {
 	s := FileContentSchema(map[string]*schema.Schema{
 		"language": {
 			Type:     schema.TypeString,
 			Optional: true,
+			Computed: true, // we need it because it will be filled by the provider or backend
 			ValidateFunc: validation.StringInSlice([]string{
 				Scala,
 				Python,
 				R,
 				SQL,
 			}, false),
-			DiffSuppressFunc: func(k, old, new string, d *schema.ResourceData) bool {
-				source := d.Get("source").(string)
-				if source == "" {
-					return false
-				}
-				ext := strings.ToLower(filepath.Ext(source))
-				return old == extMap[ext].Language
-			},
 		},
 		"format": {
 			Type:     schema.TypeString,
 			Optional: true,
-			Default:  "SOURCE",
+			Computed: true,
 			ValidateFunc: validation.StringInSlice([]string{
 				"SOURCE",
 				"DBC",
@@ -270,6 +274,9 @@ func ResourceNotebook() common.Resource {
 	return common.Resource{
 		Schema:        s,
 		SchemaVersion: 1,
+		CanSkipReadAfterCreateAndUpdate: func(d *schema.ResourceData) bool {
+			return d.Get("format").(string) == "SOURCE"
+		},
 		Create: func(ctx context.Context, d *schema.ResourceData, c *common.DatabricksClient) error {
 			content, err := ReadContent(d)
 			if err != nil {
@@ -284,6 +291,10 @@ func ResourceNotebook() common.Resource {
 				Path:      path,
 				Overwrite: true,
 			}
+			if createNotebook.Format == "" && createNotebook.Language != "" {
+				createNotebook.Format = "SOURCE"
+				d.Set("format", createNotebook.Format)
+			}
 			if createNotebook.Language == "" {
 				// TODO: check what happens with empty source
 				ext := strings.ToLower(filepath.Ext(d.Get("source").(string)))
@@ -293,8 +304,9 @@ func ResourceNotebook() common.Resource {
 				createNotebook.Overwrite = extMap[ext].Overwrite
 				// by default it's SOURCE, but for DBC we have to change it
 				d.Set("format", createNotebook.Format)
+				d.Set("language", createNotebook.Language)
 			}
-			err = notebooksAPI.Create(createNotebook)
+			resp, err := notebooksAPI.Create(createNotebook)
 			if err != nil {
 				if isParentDoesntExistError(err) {
 					parent := filepath.ToSlash(filepath.Dir(path))
@@ -303,16 +315,32 @@ func ResourceNotebook() common.Resource {
 					if err != nil {
 						return err
 					}
-					err = notebooksAPI.Create(createNotebook)
+					resp, err = notebooksAPI.Create(createNotebook)
 				}
 				if err != nil {
 					return err
 				}
 			}
+			if d.Get("object_type").(string) == "" {
+				d.Set("object_type", Notebook)
+			}
+			d.Set("object_id", resp.ObjectID)
 			d.SetId(path)
+			setComputedProperties(d, c)
 			return nil
 		},
 		Read: func(ctx context.Context, d *schema.ResourceData, c *common.DatabricksClient) error {
+			oldFormat := d.Get("format").(string)
+			if oldFormat == "" {
+				source := d.Get("source").(string)
+				// check if `source` is set, and if it is, use file extension to determine format
+				if source != "" {
+					ext := strings.ToLower(filepath.Ext(source))
+					oldFormat = extMap[ext].Format
+				} else {
+					oldFormat = "SOURCE"
+				}
+			}
 			w, err := c.WorkspaceClient()
 			if err != nil {
 				return err
@@ -323,9 +351,12 @@ func ResourceNotebook() common.Resource {
 			if err != nil {
 				return err
 			}
-			d.Set("url", c.FormatURL("#workspace", d.Id()))
-			d.Set("workspace_path", "/Workspace"+objectStatus.Path)
-			return common.StructToData(objectStatus, s, d)
+			setComputedProperties(d, c)
+			err = common.StructToData(objectStatus, s, d)
+			if err != nil {
+				return err
+			}
+			return d.Set("format", oldFormat)
 		},
 		Update: func(ctx context.Context, d *schema.ResourceData, c *common.DatabricksClient) error {
 			notebooksAPI := NewNotebooksAPI(ctx, c)
@@ -334,25 +365,33 @@ func ResourceNotebook() common.Resource {
 				return err
 			}
 			format := d.Get("format").(string)
+			var resp ImportResponse
 			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(ImportPath{
+				resp, err = notebooksAPI.Create(ImportPath{
 					Content: base64.StdEncoding.EncodeToString(content),
 					Format:  format,
 					Path:    d.Id(),
 				})
+			} else {
+				resp, err = notebooksAPI.Create(ImportPath{
+					Content:   base64.StdEncoding.EncodeToString(content),
+					Language:  d.Get("language").(string),
+					Format:    format,
+					Overwrite: true,
+					Path:      d.Id(),
+				})
 			}
-			return notebooksAPI.Create(ImportPath{
-				Content:   base64.StdEncoding.EncodeToString(content),
-				Language:  d.Get("language").(string),
-				Format:    format,
-				Overwrite: true,
-				Path:      d.Id(),
-			})
+			if err != nil {
+				return err
+			}
+			d.Set("object_id", resp.ObjectID)
+			setComputedProperties(d, c)
+			return nil
 		},
 		Delete: func(ctx context.Context, d *schema.ResourceData, c *common.DatabricksClient) error {
 			objType := d.Get("object_type")