From 673a7b207476144ba9c7646ce7682ee3ff572c58 Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Tue, 4 Mar 2025 16:36:15 +0100 Subject: [PATCH 01/10] Do not modify/create .gitignore in bundle root --- bundle/bundle.go | 14 +++++++ libs/git/fileset.go | 4 -- libs/git/fileset_test.go | 33 --------------- libs/git/repository.go | 4 -- libs/git/view.go | 34 ---------------- libs/git/view_test.go | 88 ---------------------------------------- libs/sync/sync.go | 5 --- libs/sync/sync_test.go | 9 ---- 8 files changed, 14 insertions(+), 177 deletions(-) diff --git a/bundle/bundle.go b/bundle/bundle.go index 9cb8916f51..35748ebdd8 100644 --- a/bundle/bundle.go +++ b/bundle/bundle.go @@ -198,6 +198,20 @@ func (b *Bundle) CacheDir(ctx context.Context, paths ...string) (string, error) return "", err } + gitignorePath := filepath.Join(b.BundleRootPath, ".databricks", ".gitignore") + file, err := os.OpenFile(gitignorePath, os.O_CREATE|os.O_EXCL|os.O_WRONLY, 0o644) + if err == nil { + defer file.Close() + _, err = file.WriteString("*\n") + if err != nil { + log.Debugf(ctx, "Error writing to %s: %s", gitignorePath, err) + } + } else { + if !os.IsExist(err) { + log.Debugf(ctx, "Failed to create %s: %s", gitignorePath, err) + } + } + return dir, nil } diff --git a/libs/git/fileset.go b/libs/git/fileset.go index 8391548c9c..7c0c372c95 100644 --- a/libs/git/fileset.go +++ b/libs/git/fileset.go @@ -43,7 +43,3 @@ func (f *FileSet) Files() ([]fileset.File, error) { f.view.repo.taintIgnoreRules() return f.fileset.Files() } - -func (f *FileSet) EnsureValidGitIgnoreExists() error { - return f.view.EnsureValidGitIgnoreExists() -} diff --git a/libs/git/fileset_test.go b/libs/git/fileset_test.go index 6d239edf59..cd85ee810d 100644 --- a/libs/git/fileset_test.go +++ b/libs/git/fileset_test.go @@ -1,10 +1,8 @@ package git import ( - "os" "path" "path/filepath" - "strings" "testing" "github.com/databricks/cli/libs/vfs" @@ -51,34 +49,3 @@ func TestFileSetNonCleanRoot(t *testing.T) { require.NoError(t, err) assert.Len(t, files, 3) } - -func TestFileSetAddsCacheDirToGitIgnore(t *testing.T) { - projectDir := t.TempDir() - fileSet, err := NewFileSetAtRoot(vfs.MustNew(projectDir)) - require.NoError(t, err) - err = fileSet.EnsureValidGitIgnoreExists() - require.NoError(t, err) - - gitIgnorePath := filepath.Join(projectDir, ".gitignore") - assert.FileExists(t, gitIgnorePath) - fileBytes, err := os.ReadFile(gitIgnorePath) - assert.NoError(t, err) - assert.Contains(t, string(fileBytes), ".databricks") -} - -func TestFileSetDoesNotCacheDirToGitIgnoreIfAlreadyPresent(t *testing.T) { - projectDir := t.TempDir() - gitIgnorePath := filepath.Join(projectDir, ".gitignore") - - fileSet, err := NewFileSetAtRoot(vfs.MustNew(projectDir)) - require.NoError(t, err) - err = os.WriteFile(gitIgnorePath, []byte(".databricks"), 0o644) - require.NoError(t, err) - - err = fileSet.EnsureValidGitIgnoreExists() - require.NoError(t, err) - - b, err := os.ReadFile(gitIgnorePath) - require.NoError(t, err) - assert.Equal(t, 1, strings.Count(string(b), ".databricks")) -} diff --git a/libs/git/repository.go b/libs/git/repository.go index b94297ab98..76539b46f0 100644 --- a/libs/git/repository.go +++ b/libs/git/repository.go @@ -255,7 +255,3 @@ func NewRepository(rootDir vfs.Path) (*Repository, error) { return repo, nil } - -func (r *Repository) addIgnoreRule(rule ignoreRules) { - r.ignore["."] = append(r.ignore["."], rule) -} diff --git a/libs/git/view.go b/libs/git/view.go index db22dfc5d4..b6bcd07fa9 100644 --- a/libs/git/view.go +++ b/libs/git/view.go @@ -99,37 +99,3 @@ func NewView(worktreeRoot, root vfs.Path) (*View, error) { func NewViewAtRoot(root vfs.Path) (*View, error) { return NewView(root, root) } - -func (v *View) EnsureValidGitIgnoreExists() error { - ign, err := v.IgnoreDirectory(".databricks") - if err != nil { - return err - } - - // return early if .databricks is already being ignored - if ign { - return nil - } - - // Create .gitignore with .databricks entry - gitIgnorePath := filepath.Join(v.repo.Root(), v.targetPath, ".gitignore") - file, err := os.OpenFile(gitIgnorePath, os.O_APPEND|os.O_WRONLY|os.O_CREATE, 0o644) - if err != nil { - return err - } - defer file.Close() - - // Hard code .databricks ignore pattern so that we never sync it (irrespective) - // of .gitignore patterns - v.repo.addIgnoreRule(newStringIgnoreRules([]string{ - ".databricks", - })) - - _, err = file.WriteString("\n.databricks\n") - if err != nil { - return err - } - - v.repo.taintIgnoreRules() - return nil -} diff --git a/libs/git/view_test.go b/libs/git/view_test.go index 96881fdeee..9d7a4cdecb 100644 --- a/libs/git/view_test.go +++ b/libs/git/view_test.go @@ -209,100 +209,12 @@ func TestViewABInTempDir(t *testing.T) { assert.False(t, tv.Ignore("newfile")) } -func TestViewDoesNotChangeGitignoreIfCacheDirAlreadyIgnoredAtRoot(t *testing.T) { - expected, err := os.ReadFile("./testdata_view_ignore/.gitignore") - require.NoError(t, err) - - repoPath := createFakeRepo(t, "testdata_view_ignore") - - // Since root .gitignore already has .databricks, there should be no edits - // to root .gitignore - v, err := NewViewAtRoot(vfs.MustNew(repoPath)) - require.NoError(t, err) - - err = v.EnsureValidGitIgnoreExists() - require.NoError(t, err) - - actual, err := os.ReadFile(filepath.Join(repoPath, ".gitignore")) - require.NoError(t, err) - - assert.Equal(t, string(expected), string(actual)) -} - -func TestViewDoesNotChangeGitignoreIfCacheDirAlreadyIgnoredInSubdir(t *testing.T) { - expected, err := os.ReadFile("./testdata_view_ignore/a/.gitignore") - require.NoError(t, err) - - repoPath := createFakeRepo(t, "testdata_view_ignore") - - // Since root .gitignore already has .databricks, there should be no edits - // to a/.gitignore - v, err := NewView(vfs.MustNew(repoPath), vfs.MustNew(filepath.Join(repoPath, "a"))) - require.NoError(t, err) - - err = v.EnsureValidGitIgnoreExists() - require.NoError(t, err) - - actual, err := os.ReadFile(filepath.Join(repoPath, v.targetPath, ".gitignore")) - require.NoError(t, err) - - assert.Equal(t, string(expected), string(actual)) -} - -func TestViewAddsGitignoreWithCacheDir(t *testing.T) { - repoPath := createFakeRepo(t, "testdata") - err := os.Remove(filepath.Join(repoPath, ".gitignore")) - assert.NoError(t, err) - - // Since root .gitignore was deleted, new view adds .databricks to root .gitignore - v, err := NewViewAtRoot(vfs.MustNew(repoPath)) - require.NoError(t, err) - - err = v.EnsureValidGitIgnoreExists() - require.NoError(t, err) - - actual, err := os.ReadFile(filepath.Join(repoPath, ".gitignore")) - require.NoError(t, err) - - assert.Contains(t, string(actual), "\n.databricks\n") -} - -func TestViewAddsGitignoreWithCacheDirAtSubdir(t *testing.T) { - repoPath := createFakeRepo(t, "testdata") - err := os.Remove(filepath.Join(repoPath, ".gitignore")) - require.NoError(t, err) - - // Since root .gitignore was deleted, new view adds .databricks to a/.gitignore - v, err := NewView(vfs.MustNew(repoPath), vfs.MustNew(filepath.Join(repoPath, "a"))) - require.NoError(t, err) - - err = v.EnsureValidGitIgnoreExists() - require.NoError(t, err) - - actual, err := os.ReadFile(filepath.Join(repoPath, v.targetPath, ".gitignore")) - require.NoError(t, err) - - // created .gitignore has cache dir listed - assert.Contains(t, string(actual), "\n.databricks\n") - assert.NoFileExists(t, filepath.Join(repoPath, ".gitignore")) -} - func TestViewAlwaysIgnoresCacheDir(t *testing.T) { repoPath := createFakeRepo(t, "testdata") v, err := NewViewAtRoot(vfs.MustNew(repoPath)) require.NoError(t, err) - err = v.EnsureValidGitIgnoreExists() - require.NoError(t, err) - - // Delete root .gitignore which contains .databricks entry - err = os.Remove(filepath.Join(repoPath, ".gitignore")) - require.NoError(t, err) - - // taint rules to reload .gitignore - v.repo.taintIgnoreRules() - // assert .databricks is still being ignored ign1, err := v.IgnoreDirectory(".databricks") require.NoError(t, err) diff --git a/libs/sync/sync.go b/libs/sync/sync.go index f13fa934ad..4ab43b16ad 100644 --- a/libs/sync/sync.go +++ b/libs/sync/sync.go @@ -69,11 +69,6 @@ func New(ctx context.Context, opts SyncOptions) (*Sync, error) { return nil, err } - err = fileSet.EnsureValidGitIgnoreExists() - if err != nil { - return nil, err - } - includeFileSet, err := fileset.NewGlobSet(opts.LocalRoot, opts.Include) if err != nil { return nil, err diff --git a/libs/sync/sync_test.go b/libs/sync/sync_test.go index f30431770a..175bae40de 100644 --- a/libs/sync/sync_test.go +++ b/libs/sync/sync_test.go @@ -40,9 +40,6 @@ func TestGetFileSet(t *testing.T) { fileSet, err := git.NewFileSetAtRoot(root) require.NoError(t, err) - err = fileSet.EnsureValidGitIgnoreExists() - require.NoError(t, err) - inc, err := fileset.NewGlobSet(root, []string{}) require.NoError(t, err) @@ -106,9 +103,6 @@ func TestRecursiveExclude(t *testing.T) { fileSet, err := git.NewFileSetAtRoot(root) require.NoError(t, err) - err = fileSet.EnsureValidGitIgnoreExists() - require.NoError(t, err) - inc, err := fileset.NewGlobSet(root, []string{}) require.NoError(t, err) @@ -136,9 +130,6 @@ func TestNegateExclude(t *testing.T) { fileSet, err := git.NewFileSetAtRoot(root) require.NoError(t, err) - err = fileSet.EnsureValidGitIgnoreExists() - require.NoError(t, err) - inc, err := fileset.NewGlobSet(root, []string{}) require.NoError(t, err) From 9294f51113a0d1bbf8f6f100a422fed8d1bbb3f2 Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Tue, 4 Mar 2025 17:08:56 +0100 Subject: [PATCH 02/10] record in acc tests --- .../dbt-sql/output/my_dbt_sql/.databricks/out.gitignore | 1 + .../classic/output/my_default_python/.databricks/out.gitignore | 1 + acceptance/bundle/templates/default-python/classic/script | 1 + .../templates/default-python/serverless-customcatalog/output.txt | 1 + .../output/my_default_python/.databricks/out.gitignore | 1 + .../default-sql/output/my_default_sql/.databricks/out.gitignore | 1 + .../output/my_jobs_as_code/.databricks/out.gitignore | 1 + 7 files changed, 7 insertions(+) create mode 100644 acceptance/bundle/templates/dbt-sql/output/my_dbt_sql/.databricks/out.gitignore create mode 100644 acceptance/bundle/templates/default-python/classic/output/my_default_python/.databricks/out.gitignore create mode 100644 acceptance/bundle/templates/default-python/serverless/output/my_default_python/.databricks/out.gitignore create mode 100644 acceptance/bundle/templates/default-sql/output/my_default_sql/.databricks/out.gitignore create mode 100644 acceptance/bundle/templates/experimental-jobs-as-code/output/my_jobs_as_code/.databricks/out.gitignore diff --git a/acceptance/bundle/templates/dbt-sql/output/my_dbt_sql/.databricks/out.gitignore b/acceptance/bundle/templates/dbt-sql/output/my_dbt_sql/.databricks/out.gitignore new file mode 100644 index 0000000000..72e8ffc0db --- /dev/null +++ b/acceptance/bundle/templates/dbt-sql/output/my_dbt_sql/.databricks/out.gitignore @@ -0,0 +1 @@ +* diff --git a/acceptance/bundle/templates/default-python/classic/output/my_default_python/.databricks/out.gitignore b/acceptance/bundle/templates/default-python/classic/output/my_default_python/.databricks/out.gitignore new file mode 100644 index 0000000000..72e8ffc0db --- /dev/null +++ b/acceptance/bundle/templates/default-python/classic/output/my_default_python/.databricks/out.gitignore @@ -0,0 +1 @@ +* diff --git a/acceptance/bundle/templates/default-python/classic/script b/acceptance/bundle/templates/default-python/classic/script index 7e5524065a..2c3e0871e2 100644 --- a/acceptance/bundle/templates/default-python/classic/script +++ b/acceptance/bundle/templates/default-python/classic/script @@ -6,6 +6,7 @@ trace $CLI bundle validate -t prod # Do not affect this repository's git behaviour #2318 mv .gitignore out.gitignore +mv .databricks/.gitignore .databricks/out.gitignore cd ../../ diff --git a/acceptance/bundle/templates/default-python/serverless-customcatalog/output.txt b/acceptance/bundle/templates/default-python/serverless-customcatalog/output.txt index 30726013bf..ae1f8ee271 100644 --- a/acceptance/bundle/templates/default-python/serverless-customcatalog/output.txt +++ b/acceptance/bundle/templates/default-python/serverless-customcatalog/output.txt @@ -10,6 +10,7 @@ Please refer to the README.md file for "getting started" instructions. See also the documentation at https://docs.databricks.com/dev-tools/bundles/index.html. >>> diff.py [TESTROOT]/bundle/templates/default-python/serverless-customcatalog/../serverless/output output/ +Only in [TESTROOT]/bundle/templates/default-python/serverless-customcatalog/../serverless/output: my_default_python/.databricks/out.gitignore --- [TESTROOT]/bundle/templates/default-python/serverless-customcatalog/../serverless/output/my_default_python/resources/my_default_python.pipeline.yml +++ output/my_default_python/resources/my_default_python.pipeline.yml @@ -4,6 +4,5 @@ diff --git a/acceptance/bundle/templates/default-python/serverless/output/my_default_python/.databricks/out.gitignore b/acceptance/bundle/templates/default-python/serverless/output/my_default_python/.databricks/out.gitignore new file mode 100644 index 0000000000..72e8ffc0db --- /dev/null +++ b/acceptance/bundle/templates/default-python/serverless/output/my_default_python/.databricks/out.gitignore @@ -0,0 +1 @@ +* diff --git a/acceptance/bundle/templates/default-sql/output/my_default_sql/.databricks/out.gitignore b/acceptance/bundle/templates/default-sql/output/my_default_sql/.databricks/out.gitignore new file mode 100644 index 0000000000..72e8ffc0db --- /dev/null +++ b/acceptance/bundle/templates/default-sql/output/my_default_sql/.databricks/out.gitignore @@ -0,0 +1 @@ +* diff --git a/acceptance/bundle/templates/experimental-jobs-as-code/output/my_jobs_as_code/.databricks/out.gitignore b/acceptance/bundle/templates/experimental-jobs-as-code/output/my_jobs_as_code/.databricks/out.gitignore new file mode 100644 index 0000000000..72e8ffc0db --- /dev/null +++ b/acceptance/bundle/templates/experimental-jobs-as-code/output/my_jobs_as_code/.databricks/out.gitignore @@ -0,0 +1 @@ +* From 742ecebc847d2276db8f878e5cbb460a6c258a88 Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Tue, 4 Mar 2025 17:20:31 +0100 Subject: [PATCH 03/10] update sync_test.go --- libs/sync/sync_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libs/sync/sync_test.go b/libs/sync/sync_test.go index 175bae40de..d8d8b6838f 100644 --- a/libs/sync/sync_test.go +++ b/libs/sync/sync_test.go @@ -74,7 +74,7 @@ func TestGetFileSet(t *testing.T) { fileList, err = s.GetFileList(ctx) require.NoError(t, err) - require.Len(t, fileList, 2) + require.Len(t, fileList, 1) inc, err = fileset.NewGlobSet(root, []string{"./.databricks/*.go"}) require.NoError(t, err) @@ -92,7 +92,7 @@ func TestGetFileSet(t *testing.T) { fileList, err = s.GetFileList(ctx) require.NoError(t, err) - require.Len(t, fileList, 11) + require.Len(t, fileList, 10) } func TestRecursiveExclude(t *testing.T) { From 5485e6290e8c91233916d62f9be75bd6b9705dbb Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Tue, 4 Mar 2025 18:37:24 +0100 Subject: [PATCH 04/10] update integration test --- integration/cmd/sync/sync_test.go | 73 ++++++++++++++----------------- 1 file changed, 34 insertions(+), 39 deletions(-) diff --git a/integration/cmd/sync/sync_test.go b/integration/cmd/sync/sync_test.go index 88e6ed89ad..00d95a0c4c 100644 --- a/integration/cmd/sync/sync_test.go +++ b/integration/cmd/sync/sync_test.go @@ -230,16 +230,15 @@ func (a *syncTest) snapshotContains(files []string) { func TestSyncFullFileSync(t *testing.T) { ctx, assertSync := setupSyncTest(t, "--full", "--watch") - // .gitignore is created by the sync process to enforce .databricks is not synced assertSync.waitForCompletionMarker() - assertSync.remoteDirContent(ctx, "", append(repoFiles, ".gitignore")) + assertSync.remoteDirContent(ctx, "", repoFiles) // New file localFilePath := filepath.Join(assertSync.localRoot, "foo.txt") f := testfile.CreateFile(t, localFilePath) defer f.Close(t) assertSync.waitForCompletionMarker() - assertSync.remoteDirContent(ctx, "", append(repoFiles, "foo.txt", ".gitignore")) + assertSync.remoteDirContent(ctx, "", append(repoFiles, "foo.txt")) assertSync.remoteFileContent(ctx, "foo.txt", "") // Write to file @@ -255,24 +254,23 @@ func TestSyncFullFileSync(t *testing.T) { // delete f.Remove(t) assertSync.waitForCompletionMarker() - assertSync.remoteDirContent(ctx, "", append(repoFiles, ".gitignore")) + assertSync.remoteDirContent(ctx, "", repoFiles) } func TestSyncIncrementalFileSync(t *testing.T) { ctx, assertSync := setupSyncTest(t, "--watch") - // .gitignore is created by the sync process to enforce .databricks is not synced assertSync.waitForCompletionMarker() - assertSync.remoteDirContent(ctx, "", append(repoFiles, ".gitignore")) + assertSync.remoteDirContent(ctx, "", repoFiles) // New file localFilePath := filepath.Join(assertSync.localRoot, "foo.txt") f := testfile.CreateFile(t, localFilePath) defer f.Close(t) assertSync.waitForCompletionMarker() - assertSync.remoteDirContent(ctx, "", append(repoFiles, "foo.txt", ".gitignore")) + assertSync.remoteDirContent(ctx, "", append(repoFiles, "foo.txt")) assertSync.remoteFileContent(ctx, "foo.txt", "") - assertSync.snapshotContains(append(repoFiles, "foo.txt", ".gitignore")) + assertSync.snapshotContains(append(repoFiles, "foo.txt")) // Write to file f.Overwrite(t, `{"statement": "Mi Gente"}`) @@ -287,16 +285,15 @@ func TestSyncIncrementalFileSync(t *testing.T) { // delete f.Remove(t) assertSync.waitForCompletionMarker() - assertSync.remoteDirContent(ctx, "", append(repoFiles, ".gitignore")) - assertSync.snapshotContains(append(repoFiles, ".gitignore")) + assertSync.remoteDirContent(ctx, "", repoFiles) + assertSync.snapshotContains(repoFiles) } func TestSyncNestedFolderSync(t *testing.T) { ctx, assertSync := setupSyncTest(t, "--watch") - // .gitignore is created by the sync process to enforce .databricks is not synced assertSync.waitForCompletionMarker() - assertSync.remoteDirContent(ctx, "", append(repoFiles, ".gitignore")) + assertSync.remoteDirContent(ctx, "", repoFiles) // New file localFilePath := filepath.Join(assertSync.localRoot, "dir1/dir2/dir3/foo.txt") @@ -305,25 +302,24 @@ func TestSyncNestedFolderSync(t *testing.T) { f := testfile.CreateFile(t, localFilePath) defer f.Close(t) assertSync.waitForCompletionMarker() - assertSync.remoteDirContent(ctx, "", append(repoFiles, ".gitignore", "dir1")) + assertSync.remoteDirContent(ctx, "", append(repoFiles, "dir1")) assertSync.remoteDirContent(ctx, "dir1", []string{"dir2"}) assertSync.remoteDirContent(ctx, "dir1/dir2", []string{"dir3"}) assertSync.remoteDirContent(ctx, "dir1/dir2/dir3", []string{"foo.txt"}) - assertSync.snapshotContains(append(repoFiles, ".gitignore", "dir1/dir2/dir3/foo.txt")) + assertSync.snapshotContains(append(repoFiles, "dir1/dir2/dir3/foo.txt")) // delete f.Remove(t) assertSync.waitForCompletionMarker() assertSync.remoteNotExist(ctx, "dir1") - assertSync.snapshotContains(append(repoFiles, ".gitignore")) + assertSync.snapshotContains(repoFiles) } func TestSyncNestedFolderDoesntFailOnNonEmptyDirectory(t *testing.T) { ctx, assertSync := setupSyncTest(t, "--watch") - // .gitignore is created by the sync process to enforce .databricks is not synced assertSync.waitForCompletionMarker() - assertSync.remoteDirContent(ctx, "", append(repoFiles, ".gitignore")) + assertSync.remoteDirContent(ctx, "", repoFiles) // New file localFilePath := filepath.Join(assertSync.localRoot, "dir1/dir2/dir3/foo.txt") @@ -353,9 +349,8 @@ func TestSyncNestedFolderDoesntFailOnNonEmptyDirectory(t *testing.T) { func TestSyncNestedSpacePlusAndHashAreEscapedSync(t *testing.T) { ctx, assertSync := setupSyncTest(t, "--watch") - // .gitignore is created by the sync process to enforce .databricks is not synced assertSync.waitForCompletionMarker() - assertSync.remoteDirContent(ctx, "", append(repoFiles, ".gitignore")) + assertSync.remoteDirContent(ctx, "", repoFiles) // New file localFilePath := filepath.Join(assertSync.localRoot, "dir1/a b+c/c+d e/e+f g#i.txt") @@ -364,17 +359,17 @@ func TestSyncNestedSpacePlusAndHashAreEscapedSync(t *testing.T) { f := testfile.CreateFile(t, localFilePath) defer f.Close(t) assertSync.waitForCompletionMarker() - assertSync.remoteDirContent(ctx, "", append(repoFiles, ".gitignore", "dir1")) + assertSync.remoteDirContent(ctx, "", append(repoFiles, "dir1")) assertSync.remoteDirContent(ctx, "dir1", []string{"a b+c"}) assertSync.remoteDirContent(ctx, "dir1/a b+c", []string{"c+d e"}) assertSync.remoteDirContent(ctx, "dir1/a b+c/c+d e", []string{"e+f g#i.txt"}) - assertSync.snapshotContains(append(repoFiles, ".gitignore", "dir1/a b+c/c+d e/e+f g#i.txt")) + assertSync.snapshotContains(append(repoFiles, "dir1/a b+c/c+d e/e+f g#i.txt")) // delete f.Remove(t) assertSync.waitForCompletionMarker() assertSync.remoteNotExist(ctx, "dir1/a b+c/c+d e") - assertSync.snapshotContains(append(repoFiles, ".gitignore")) + assertSync.snapshotContains(repoFiles) } // This is a check for the edge case when a user does the following: @@ -395,23 +390,23 @@ func TestSyncIncrementalFileOverwritesFolder(t *testing.T) { f := testfile.CreateFile(t, localFilePath) defer f.Close(t) assertSync.waitForCompletionMarker() - assertSync.remoteDirContent(ctx, "", append(repoFiles, ".gitignore", "foo")) + assertSync.remoteDirContent(ctx, "", append(repoFiles, "foo")) assertSync.remoteDirContent(ctx, "foo", []string{"bar.txt"}) - assertSync.snapshotContains(append(repoFiles, ".gitignore", "foo/bar.txt")) + assertSync.snapshotContains(append(repoFiles, "foo/bar.txt")) // delete foo/bar.txt f.Remove(t) os.Remove(filepath.Join(assertSync.localRoot, "foo")) assertSync.waitForCompletionMarker() assertSync.remoteNotExist(ctx, "foo") - assertSync.snapshotContains(append(repoFiles, ".gitignore")) + assertSync.snapshotContains(repoFiles) f2 := testfile.CreateFile(t, filepath.Join(assertSync.localRoot, "foo")) defer f2.Close(t) assertSync.waitForCompletionMarker() - assertSync.remoteDirContent(ctx, "", append(repoFiles, ".gitignore", "foo")) + assertSync.remoteDirContent(ctx, "", append(repoFiles, "foo")) assertSync.objectType(ctx, "foo", "FILE") - assertSync.snapshotContains(append(repoFiles, ".gitignore", "foo")) + assertSync.snapshotContains(append(repoFiles, "foo")) } func TestSyncIncrementalSyncPythonNotebookToFile(t *testing.T) { @@ -425,23 +420,23 @@ func TestSyncIncrementalSyncPythonNotebookToFile(t *testing.T) { // notebook was uploaded properly assertSync.waitForCompletionMarker() - assertSync.remoteDirContent(ctx, "", append(repoFiles, ".gitignore", "foo")) + assertSync.remoteDirContent(ctx, "", append(repoFiles, "foo")) assertSync.objectType(ctx, "foo", "NOTEBOOK") assertSync.language(ctx, "foo", "PYTHON") - assertSync.snapshotContains(append(repoFiles, ".gitignore", "foo.py")) + assertSync.snapshotContains(append(repoFiles, "foo.py")) // convert to vanilla python file f.Overwrite(t, "# No longer a python notebook") assertSync.waitForCompletionMarker() assertSync.objectType(ctx, "foo.py", "FILE") - assertSync.remoteDirContent(ctx, "", append(repoFiles, ".gitignore", "foo.py")) - assertSync.snapshotContains(append(repoFiles, ".gitignore", "foo.py")) + assertSync.remoteDirContent(ctx, "", append(repoFiles, "foo.py")) + assertSync.snapshotContains(append(repoFiles, "foo.py")) // delete the vanilla python file f.Remove(t) assertSync.waitForCompletionMarker() - assertSync.remoteDirContent(ctx, "", append(repoFiles, ".gitignore")) - assertSync.snapshotContains(append(repoFiles, ".gitignore")) + assertSync.remoteDirContent(ctx, "", repoFiles) + assertSync.snapshotContains(repoFiles) } func TestSyncIncrementalSyncFileToPythonNotebook(t *testing.T) { @@ -454,17 +449,17 @@ func TestSyncIncrementalSyncFileToPythonNotebook(t *testing.T) { assertSync.waitForCompletionMarker() // assert file upload - assertSync.remoteDirContent(ctx, "", append(repoFiles, ".gitignore", "foo.py")) + assertSync.remoteDirContent(ctx, "", append(repoFiles, "foo.py")) assertSync.objectType(ctx, "foo.py", "FILE") - assertSync.snapshotContains(append(repoFiles, ".gitignore", "foo.py")) + assertSync.snapshotContains(append(repoFiles, "foo.py")) // convert to notebook f.Overwrite(t, "# Databricks notebook source") assertSync.waitForCompletionMarker() assertSync.objectType(ctx, "foo", "NOTEBOOK") assertSync.language(ctx, "foo", "PYTHON") - assertSync.remoteDirContent(ctx, "", append(repoFiles, ".gitignore", "foo")) - assertSync.snapshotContains(append(repoFiles, ".gitignore", "foo.py")) + assertSync.remoteDirContent(ctx, "", append(repoFiles, "foo")) + assertSync.snapshotContains(append(repoFiles, "foo.py")) } func TestSyncIncrementalSyncPythonNotebookDelete(t *testing.T) { @@ -478,14 +473,14 @@ func TestSyncIncrementalSyncPythonNotebookDelete(t *testing.T) { assertSync.waitForCompletionMarker() // notebook was uploaded properly - assertSync.remoteDirContent(ctx, "", append(repoFiles, ".gitignore", "foo")) + assertSync.remoteDirContent(ctx, "", append(repoFiles, "foo")) assertSync.objectType(ctx, "foo", "NOTEBOOK") assertSync.language(ctx, "foo", "PYTHON") // Delete notebook f.Remove(t) assertSync.waitForCompletionMarker() - assertSync.remoteDirContent(ctx, "", append(repoFiles, ".gitignore")) + assertSync.remoteDirContent(ctx, "", repoFiles) } func TestSyncEnsureRemotePathIsUsableIfRepoDoesntExist(t *testing.T) { From 26aea2e3401f08aa8d2cbe3261e63d25f83cdc1e Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Wed, 5 Mar 2025 09:19:15 +0100 Subject: [PATCH 05/10] Also create .databricks in SnapshotPath; fix tests --- .../output/my_dbt_sql/.databricks/out.gitignore | 1 - acceptance/bundle/templates/dbt-sql/script | 1 + .../my_default_python/.databricks/out.gitignore | 1 - .../templates/default-python/classic/script | 2 +- .../serverless-customcatalog/output.txt | 1 - .../my_default_python/.databricks/out.gitignore | 1 - .../templates/default-python/serverless/script | 1 + acceptance/bundle/templates/default-sql/script | 1 + .../my_jobs_as_code/.databricks/out.gitignore | 1 - .../templates/experimental-jobs-as-code/script | 1 + bundle/bundle.go | 16 ++-------------- integration/cmd/sync/sync_test.go | 2 +- libs/sync/snapshot.go | 1 + libs/sync/sync.go | 3 +++ 14 files changed, 12 insertions(+), 21 deletions(-) delete mode 100644 acceptance/bundle/templates/dbt-sql/output/my_dbt_sql/.databricks/out.gitignore delete mode 100644 acceptance/bundle/templates/default-python/classic/output/my_default_python/.databricks/out.gitignore delete mode 100644 acceptance/bundle/templates/default-python/serverless/output/my_default_python/.databricks/out.gitignore delete mode 100644 acceptance/bundle/templates/experimental-jobs-as-code/output/my_jobs_as_code/.databricks/out.gitignore diff --git a/acceptance/bundle/templates/dbt-sql/output/my_dbt_sql/.databricks/out.gitignore b/acceptance/bundle/templates/dbt-sql/output/my_dbt_sql/.databricks/out.gitignore deleted file mode 100644 index 72e8ffc0db..0000000000 --- a/acceptance/bundle/templates/dbt-sql/output/my_dbt_sql/.databricks/out.gitignore +++ /dev/null @@ -1 +0,0 @@ -* diff --git a/acceptance/bundle/templates/dbt-sql/script b/acceptance/bundle/templates/dbt-sql/script index 3a2660de5c..427f655a6a 100644 --- a/acceptance/bundle/templates/dbt-sql/script +++ b/acceptance/bundle/templates/dbt-sql/script @@ -6,3 +6,4 @@ trace $CLI bundle validate -t prod # Do not affect this repository's git behaviour #2318 mv .gitignore out.gitignore +rm .databricks/.gitignore diff --git a/acceptance/bundle/templates/default-python/classic/output/my_default_python/.databricks/out.gitignore b/acceptance/bundle/templates/default-python/classic/output/my_default_python/.databricks/out.gitignore deleted file mode 100644 index 72e8ffc0db..0000000000 --- a/acceptance/bundle/templates/default-python/classic/output/my_default_python/.databricks/out.gitignore +++ /dev/null @@ -1 +0,0 @@ -* diff --git a/acceptance/bundle/templates/default-python/classic/script b/acceptance/bundle/templates/default-python/classic/script index 2c3e0871e2..c8a843b4f2 100644 --- a/acceptance/bundle/templates/default-python/classic/script +++ b/acceptance/bundle/templates/default-python/classic/script @@ -6,7 +6,7 @@ trace $CLI bundle validate -t prod # Do not affect this repository's git behaviour #2318 mv .gitignore out.gitignore -mv .databricks/.gitignore .databricks/out.gitignore +rm -fr .databricks cd ../../ diff --git a/acceptance/bundle/templates/default-python/serverless-customcatalog/output.txt b/acceptance/bundle/templates/default-python/serverless-customcatalog/output.txt index ae1f8ee271..30726013bf 100644 --- a/acceptance/bundle/templates/default-python/serverless-customcatalog/output.txt +++ b/acceptance/bundle/templates/default-python/serverless-customcatalog/output.txt @@ -10,7 +10,6 @@ Please refer to the README.md file for "getting started" instructions. See also the documentation at https://docs.databricks.com/dev-tools/bundles/index.html. >>> diff.py [TESTROOT]/bundle/templates/default-python/serverless-customcatalog/../serverless/output output/ -Only in [TESTROOT]/bundle/templates/default-python/serverless-customcatalog/../serverless/output: my_default_python/.databricks/out.gitignore --- [TESTROOT]/bundle/templates/default-python/serverless-customcatalog/../serverless/output/my_default_python/resources/my_default_python.pipeline.yml +++ output/my_default_python/resources/my_default_python.pipeline.yml @@ -4,6 +4,5 @@ diff --git a/acceptance/bundle/templates/default-python/serverless/output/my_default_python/.databricks/out.gitignore b/acceptance/bundle/templates/default-python/serverless/output/my_default_python/.databricks/out.gitignore deleted file mode 100644 index 72e8ffc0db..0000000000 --- a/acceptance/bundle/templates/default-python/serverless/output/my_default_python/.databricks/out.gitignore +++ /dev/null @@ -1 +0,0 @@ -* diff --git a/acceptance/bundle/templates/default-python/serverless/script b/acceptance/bundle/templates/default-python/serverless/script index e5fcb77415..d7b047fec8 100644 --- a/acceptance/bundle/templates/default-python/serverless/script +++ b/acceptance/bundle/templates/default-python/serverless/script @@ -6,3 +6,4 @@ trace $CLI bundle validate -t prod # Do not affect this repository's git behaviour #2318 mv .gitignore out.gitignore +rm .databricks/.gitignore diff --git a/acceptance/bundle/templates/default-sql/script b/acceptance/bundle/templates/default-sql/script index 7ea0d863c0..ffa205df6a 100644 --- a/acceptance/bundle/templates/default-sql/script +++ b/acceptance/bundle/templates/default-sql/script @@ -6,3 +6,4 @@ trace $CLI bundle validate -t prod # Do not affect this repository's git behaviour #2318 mv .gitignore out.gitignore +mv .databricks/.gitignore .databricks/out.gitignore diff --git a/acceptance/bundle/templates/experimental-jobs-as-code/output/my_jobs_as_code/.databricks/out.gitignore b/acceptance/bundle/templates/experimental-jobs-as-code/output/my_jobs_as_code/.databricks/out.gitignore deleted file mode 100644 index 72e8ffc0db..0000000000 --- a/acceptance/bundle/templates/experimental-jobs-as-code/output/my_jobs_as_code/.databricks/out.gitignore +++ /dev/null @@ -1 +0,0 @@ -* diff --git a/acceptance/bundle/templates/experimental-jobs-as-code/script b/acceptance/bundle/templates/experimental-jobs-as-code/script index 08e48fc5f7..9b74435877 100644 --- a/acceptance/bundle/templates/experimental-jobs-as-code/script +++ b/acceptance/bundle/templates/experimental-jobs-as-code/script @@ -11,3 +11,4 @@ rm -fr .venv resources/__pycache__ uv.lock my_jobs_as_code.egg-info # Do not affect this repository's git behaviour #2318 mv .gitignore out.gitignore +rm -fr .databricks diff --git a/bundle/bundle.go b/bundle/bundle.go index 35748ebdd8..2f1e8b8d7c 100644 --- a/bundle/bundle.go +++ b/bundle/bundle.go @@ -21,6 +21,7 @@ import ( "github.com/databricks/cli/libs/fileset" "github.com/databricks/cli/libs/locker" "github.com/databricks/cli/libs/log" + libsync "github.com/databricks/cli/libs/sync" "github.com/databricks/cli/libs/tags" "github.com/databricks/cli/libs/terraform" "github.com/databricks/cli/libs/vfs" @@ -198,20 +199,7 @@ func (b *Bundle) CacheDir(ctx context.Context, paths ...string) (string, error) return "", err } - gitignorePath := filepath.Join(b.BundleRootPath, ".databricks", ".gitignore") - file, err := os.OpenFile(gitignorePath, os.O_CREATE|os.O_EXCL|os.O_WRONLY, 0o644) - if err == nil { - defer file.Close() - _, err = file.WriteString("*\n") - if err != nil { - log.Debugf(ctx, "Error writing to %s: %s", gitignorePath, err) - } - } else { - if !os.IsExist(err) { - log.Debugf(ctx, "Failed to create %s: %s", gitignorePath, err) - } - } - + libsync.WriteGitIgnore(ctx, filepath.Join(b.BundleRootPath, ".databricks")) return dir, nil } diff --git a/integration/cmd/sync/sync_test.go b/integration/cmd/sync/sync_test.go index 00d95a0c4c..337db8fcab 100644 --- a/integration/cmd/sync/sync_test.go +++ b/integration/cmd/sync/sync_test.go @@ -224,7 +224,7 @@ func (a *syncTest) snapshotContains(files []string) { _, ok := s.LastModifiedTimes[filePath] assert.True(a.t, ok, "%s not in snapshot file: %v", filePath, s.LastModifiedTimes) } - assert.Equal(a.t, len(files), len(s.LastModifiedTimes)) + assert.Equal(a.t, len(files), len(s.LastModifiedTimes), "files=%s s.LastModifiedTimes=%s", files, s.LastModifiedTimes) } func TestSyncFullFileSync(t *testing.T) { diff --git a/libs/sync/snapshot.go b/libs/sync/snapshot.go index a596531b93..0af7908024 100644 --- a/libs/sync/snapshot.go +++ b/libs/sync/snapshot.go @@ -95,6 +95,7 @@ func SnapshotPath(opts *SyncOptions) (string, error) { return "", fmt.Errorf("failed to create config directory: %s", err) } } + fileName := GetFileName(opts.Host, opts.RemotePath) return filepath.Join(snapshotDir, fileName), nil } diff --git a/libs/sync/sync.go b/libs/sync/sync.go index 4ab43b16ad..b4dcbe6a1b 100644 --- a/libs/sync/sync.go +++ b/libs/sync/sync.go @@ -4,6 +4,7 @@ import ( "context" "errors" "fmt" + "path/filepath" stdsync "sync" "time" @@ -69,6 +70,8 @@ func New(ctx context.Context, opts SyncOptions) (*Sync, error) { return nil, err } + WriteGitIgnore(ctx, filepath.Join(opts.LocalRoot.Native(), ".databricks")) + includeFileSet, err := fileset.NewGlobSet(opts.LocalRoot, opts.Include) if err != nil { return nil, err From 90efc36a596e09e572eb7a2c4535ea4b96a0f2ca Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Wed, 5 Mar 2025 10:16:22 +0100 Subject: [PATCH 06/10] restore explicitly adding .databricks --- libs/git/repository.go | 4 ++++ libs/git/view.go | 17 +++++++++++++++-- libs/sync/sync.go | 3 --- libs/sync/sync_test.go | 4 ++-- 4 files changed, 21 insertions(+), 7 deletions(-) diff --git a/libs/git/repository.go b/libs/git/repository.go index 76539b46f0..b94297ab98 100644 --- a/libs/git/repository.go +++ b/libs/git/repository.go @@ -255,3 +255,7 @@ func NewRepository(rootDir vfs.Path) (*Repository, error) { return repo, nil } + +func (r *Repository) addIgnoreRule(rule ignoreRules) { + r.ignore["."] = append(r.ignore["."], rule) +} diff --git a/libs/git/view.go b/libs/git/view.go index b6bcd07fa9..142cc49479 100644 --- a/libs/git/view.go +++ b/libs/git/view.go @@ -90,12 +90,25 @@ func NewView(worktreeRoot, root vfs.Path) (*View, error) { target = strings.TrimPrefix(target, string(os.PathSeparator)) target = path.Clean(filepath.ToSlash(target)) - return &View{ + result := &View{ repo: repo, targetPath: target, - }, nil + } + + result.SetupDefaults() + return result, nil } func NewViewAtRoot(root vfs.Path) (*View, error) { return NewView(root, root) } + +func (v *View) SetupDefaults() { + // Hard code .databricks ignore pattern so that we never sync it (irrespective) + // of .gitignore patterns + v.repo.addIgnoreRule(newStringIgnoreRules([]string{ + ".databricks", + })) + + v.repo.taintIgnoreRules() +} diff --git a/libs/sync/sync.go b/libs/sync/sync.go index b4dcbe6a1b..4ab43b16ad 100644 --- a/libs/sync/sync.go +++ b/libs/sync/sync.go @@ -4,7 +4,6 @@ import ( "context" "errors" "fmt" - "path/filepath" stdsync "sync" "time" @@ -70,8 +69,6 @@ func New(ctx context.Context, opts SyncOptions) (*Sync, error) { return nil, err } - WriteGitIgnore(ctx, filepath.Join(opts.LocalRoot.Native(), ".databricks")) - includeFileSet, err := fileset.NewGlobSet(opts.LocalRoot, opts.Include) if err != nil { return nil, err diff --git a/libs/sync/sync_test.go b/libs/sync/sync_test.go index d8d8b6838f..1b54982754 100644 --- a/libs/sync/sync_test.go +++ b/libs/sync/sync_test.go @@ -56,7 +56,7 @@ func TestGetFileSet(t *testing.T) { fileList, err := s.GetFileList(ctx) require.NoError(t, err) - require.Len(t, fileList, 10) + require.Len(t, fileList, 9) inc, err = fileset.NewGlobSet(root, []string{}) require.NoError(t, err) @@ -119,7 +119,7 @@ func TestRecursiveExclude(t *testing.T) { fileList, err := s.GetFileList(ctx) require.NoError(t, err) - require.Len(t, fileList, 7) + require.Len(t, fileList, 6) } func TestNegateExclude(t *testing.T) { From cec80a21df3b8b23e89a258c18f510b9b02adf43 Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Wed, 5 Mar 2025 10:22:21 +0100 Subject: [PATCH 07/10] clean up ws --- libs/sync/snapshot.go | 1 - 1 file changed, 1 deletion(-) diff --git a/libs/sync/snapshot.go b/libs/sync/snapshot.go index 0af7908024..a596531b93 100644 --- a/libs/sync/snapshot.go +++ b/libs/sync/snapshot.go @@ -95,7 +95,6 @@ func SnapshotPath(opts *SyncOptions) (string, error) { return "", fmt.Errorf("failed to create config directory: %s", err) } } - fileName := GetFileName(opts.Host, opts.RemotePath) return filepath.Join(snapshotDir, fileName), nil } From 8dbb5a2a2060ca4d8dd15b8436421bd049d32bd7 Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Wed, 5 Mar 2025 10:25:32 +0100 Subject: [PATCH 08/10] restore write in sync --- bundle/bundle.go | 2 +- libs/sync/sync.go | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/bundle/bundle.go b/bundle/bundle.go index 2f1e8b8d7c..dad99d6e1a 100644 --- a/bundle/bundle.go +++ b/bundle/bundle.go @@ -199,7 +199,7 @@ func (b *Bundle) CacheDir(ctx context.Context, paths ...string) (string, error) return "", err } - libsync.WriteGitIgnore(ctx, filepath.Join(b.BundleRootPath, ".databricks")) + libsync.WriteGitIgnore(ctx, b.BundleRootPath) return dir, nil } diff --git a/libs/sync/sync.go b/libs/sync/sync.go index 4ab43b16ad..4d14f745a9 100644 --- a/libs/sync/sync.go +++ b/libs/sync/sync.go @@ -69,6 +69,8 @@ func New(ctx context.Context, opts SyncOptions) (*Sync, error) { return nil, err } + WriteGitIgnore(ctx, opts.LocalRoot.Native()) + includeFileSet, err := fileset.NewGlobSet(opts.LocalRoot, opts.Include) if err != nil { return nil, err From 32318c22b33a5923c071749b42bee83afcda7628 Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Wed, 5 Mar 2025 10:48:35 +0100 Subject: [PATCH 09/10] add missing libs/sync/gitignore.go --- libs/sync/gitignore.go | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) create mode 100644 libs/sync/gitignore.go diff --git a/libs/sync/gitignore.go b/libs/sync/gitignore.go new file mode 100644 index 0000000000..b3888a0cce --- /dev/null +++ b/libs/sync/gitignore.go @@ -0,0 +1,26 @@ +package sync + +import ( + "context" + "os" + "path/filepath" + + "github.com/databricks/cli/libs/log" +) + +func WriteGitIgnore(ctx context.Context, dir string) { + gitignorePath := filepath.Join(dir, ".databricks", ".gitignore") + file, err := os.OpenFile(gitignorePath, os.O_CREATE|os.O_EXCL|os.O_WRONLY, 0o644) + if err != nil { + if os.IsExist(err) { + return + } + log.Debugf(ctx, "Failed to create %s: %s", gitignorePath, err) + } + + defer file.Close() + _, err = file.WriteString("*\n") + if err != nil { + log.Debugf(ctx, "Error writing to %s: %s", gitignorePath, err) + } +} From c2536d542f47e9ee3b0be0fb7ea08a24501c47b0 Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Wed, 5 Mar 2025 11:29:29 +0100 Subject: [PATCH 10/10] use 'rm .databricks/.gitignore' instead of 'rm -fr' (it acts as an assert); add a comment re default-sql --- acceptance/bundle/templates/default-python/classic/script | 2 +- acceptance/bundle/templates/default-sql/script | 2 ++ acceptance/bundle/templates/experimental-jobs-as-code/script | 2 +- 3 files changed, 4 insertions(+), 2 deletions(-) diff --git a/acceptance/bundle/templates/default-python/classic/script b/acceptance/bundle/templates/default-python/classic/script index c8a843b4f2..955e059250 100644 --- a/acceptance/bundle/templates/default-python/classic/script +++ b/acceptance/bundle/templates/default-python/classic/script @@ -6,7 +6,7 @@ trace $CLI bundle validate -t prod # Do not affect this repository's git behaviour #2318 mv .gitignore out.gitignore -rm -fr .databricks +rm .databricks/.gitignore cd ../../ diff --git a/acceptance/bundle/templates/default-sql/script b/acceptance/bundle/templates/default-sql/script index ffa205df6a..01b393a8cd 100644 --- a/acceptance/bundle/templates/default-sql/script +++ b/acceptance/bundle/templates/default-sql/script @@ -6,4 +6,6 @@ trace $CLI bundle validate -t prod # Do not affect this repository's git behaviour #2318 mv .gitignore out.gitignore + +# Only for this test (default-sql), record .databricks/.gitignore in the output mv .databricks/.gitignore .databricks/out.gitignore diff --git a/acceptance/bundle/templates/experimental-jobs-as-code/script b/acceptance/bundle/templates/experimental-jobs-as-code/script index 9b74435877..ed9a07dd50 100644 --- a/acceptance/bundle/templates/experimental-jobs-as-code/script +++ b/acceptance/bundle/templates/experimental-jobs-as-code/script @@ -11,4 +11,4 @@ rm -fr .venv resources/__pycache__ uv.lock my_jobs_as_code.egg-info # Do not affect this repository's git behaviour #2318 mv .gitignore out.gitignore -rm -fr .databricks +rm .databricks/.gitignore