diff --git a/.nextchanges/bundles/notebook-detect-first-line.md b/.nextchanges/bundles/notebook-detect-first-line.md new file mode 100644 index 00000000000..7faa2256689 --- /dev/null +++ b/.nextchanges/bundles/notebook-detect-first-line.md @@ -0,0 +1 @@ +* Detect notebooks with the same rule as the workspace: a `.py`, `.sql`, `.scala` or `.r` file is a notebook when its first line contains `Databricks notebook source`, for example after a byte order mark. Previously such a file was tracked as a plain file, so removing it locally left the remote notebook behind. ([#7290](https://github.com/databricks/cli/pull/7290)) diff --git a/acceptance/bundle/sync/replace-node-type/output.txt b/acceptance/bundle/sync/replace-node-type/output.txt index bb144a93bcd..a048e55c572 100644 --- a/acceptance/bundle/sync/replace-node-type/output.txt +++ b/acceptance/bundle/sync/replace-node-type/output.txt @@ -24,12 +24,9 @@ FILE plain.sql === Remove both local files and sync again >>> [CLI] bundle sync --output text -Action: DELETE: bom.sql, plain.sql -Deleted bom.sql +Action: DELETE: bom, plain.sql +Deleted bom Deleted plain.sql Initial Sync Complete -=== BUG: bom remains after sync reports deleting bom.sql - >>> list -NOTEBOOK bom diff --git a/acceptance/bundle/sync/replace-node-type/script b/acceptance/bundle/sync/replace-node-type/script index 11446a48db6..eca6be849c3 100644 --- a/acceptance/bundle/sync/replace-node-type/script +++ b/acceptance/bundle/sync/replace-node-type/script @@ -27,6 +27,4 @@ trace list title "Remove both local files and sync again\n" rm bom.sql plain.sql trace $CLI bundle sync --output text 2>&1 | sort - -title "BUG: bom remains after sync reports deleting bom.sql\n" trace list diff --git a/acceptance/bundle/sync/replace-node-type/test.toml b/acceptance/bundle/sync/replace-node-type/test.toml index 757f921d422..aab5e85edef 100644 --- a/acceptance/bundle/sync/replace-node-type/test.toml +++ b/acceptance/bundle/sync/replace-node-type/test.toml @@ -1,5 +1,3 @@ -Badness = "After a BOM-prefixed SQL notebook is removed locally, sync reports deleting bom.sql but leaves the remote notebook bom. The CLI does not detect the file as a notebook, so the snapshot keeps tracking it as bom.sql" - Cloud = true Env.MSYS_NO_PATHCONV = "1" Ignore = [".databricks"] diff --git a/libs/notebook/detect.go b/libs/notebook/detect.go index d9be548a5c6..0f8c872b297 100644 --- a/libs/notebook/detect.go +++ b/libs/notebook/detect.go @@ -22,8 +22,11 @@ type FileInfoWithWorkspaceObjectInfo interface { WorkspaceObjectInfo() workspace.ObjectInfo } -// Maximum length in bytes of the notebook header. -const headerLength = 32 +// Marker the workspace looks for to import a source file as a notebook. +const notebookSourceMarker = "Databricks notebook source" + +// The workspace reads the first line out of this many leading bytes. +const headerLength = 200 // file wraps an fs.File and implements a few helper methods such that // they don't need to be inlined in the [DetectWithFS] function below. @@ -127,28 +130,20 @@ func DetectBytes(name string, content []byte) (notebook bool, language workspace // detectSource reports whether a source file with the given name and first line is a notebook. func detectSource(name, fileHeader string) (notebook bool, language workspace.Language) { - header := "" - - // Determine which header to expect based on filename extension. - ext := strings.ToLower(filepath.Ext(name)) - switch ext { + switch strings.ToLower(filepath.Ext(name)) { case ExtensionPython: - header = `# Databricks notebook source` language = workspace.LanguagePython case ExtensionR: - header = `# Databricks notebook source` language = workspace.LanguageR case ExtensionScala: - header = "// Databricks notebook source" language = workspace.LanguageScala case ExtensionSql: - header = "-- Databricks notebook source" language = workspace.LanguageSql default: return false, "" } - if fileHeader != header { + if !strings.Contains(fileHeader, notebookSourceMarker) { return false, "" } diff --git a/libs/notebook/detect_test.go b/libs/notebook/detect_test.go index c774612c9bf..0c0b7203ca5 100644 --- a/libs/notebook/detect_test.go +++ b/libs/notebook/detect_test.go @@ -1,6 +1,7 @@ package notebook import ( + "bytes" "io/fs" "os" "path/filepath" @@ -140,6 +141,10 @@ func TestDetectBytes(t *testing.T) { }{ {"nb.py", []byte("# Databricks notebook source\nprint(1)\n"), true, workspace.LanguagePython}, {"nb.sql", []byte("-- Databricks notebook source\nSELECT 1\n"), true, workspace.LanguageSql}, + {"marker_mid_line.sql", []byte("-- MAGIC Databricks notebook source\nSELECT 1\n"), true, workspace.LanguageSql}, + {"other_comment.py", []byte("// Databricks notebook source\nprint(1)\n"), true, workspace.LanguagePython}, + {"second_line.py", []byte("print(1)\n# Databricks notebook source\n"), false, ""}, + {"past_buffer.py", append(bytes.Repeat([]byte("#"), headerLength), []byte(" Databricks notebook source\n")...), false, ""}, {"nb.ipynb", ipynb, true, workspace.LanguagePython}, {"plain.py", []byte("print(1)\n"), false, ""}, {"space.geniespace.json", []byte("{}"), false, ""},