Add check for file exists incase of conflicting remote names (#244)

Before:
```
shreyas.goenka@THW32HFW6T deco-538-pipeline-error % bricks bundle deploy
Error: both myNb.py and myNb.sql point to the same remote file location myNb. Please remove one of them from your local project
```
Even though myNb.sql was created by renaming myNb.py

Now deployments are successful
This commit is contained in:
shreyas-goenka 2023-03-10 11:52:45 +01:00 committed by GitHub
parent c4c8f944f3
commit 316a006125
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 50 additions and 7 deletions

View File

@ -165,13 +165,17 @@ func loadOrNewSnapshot(opts *SyncOptions) (*Snapshot, error) {
} }
func (s *Snapshot) diff(all []fileset.File) (change diff, err error) { func (s *Snapshot) diff(all []fileset.File) (change diff, err error) {
currentFilenames := map[string]bool{}
lastModifiedTimes := s.LastUpdatedTimes lastModifiedTimes := s.LastUpdatedTimes
remoteToLocalNames := s.RemoteToLocalNames remoteToLocalNames := s.RemoteToLocalNames
localToRemoteNames := s.LocalToRemoteNames localToRemoteNames := s.LocalToRemoteNames
// set of files currently present in the local file system and tracked by git
localFileSet := map[string]struct{}{}
for _, f := range all {
localFileSet[f.Relative] = struct{}{}
}
for _, f := range all { for _, f := range all {
// create set of current files to figure out if removals are needed
currentFilenames[f.Relative] = true
// get current modified timestamp // get current modified timestamp
modified := f.Modified() modified := f.Modified()
lastSeenModified, seen := lastModifiedTimes[f.Relative] lastSeenModified, seen := lastModifiedTimes[f.Relative]
@ -207,11 +211,13 @@ func (s *Snapshot) diff(all []fileset.File) (change diff, err error) {
change.delete = append(change.delete, oldRemoteName) change.delete = append(change.delete, oldRemoteName)
delete(remoteToLocalNames, oldRemoteName) delete(remoteToLocalNames, oldRemoteName)
} }
// We cannot allow two local files in the project to point to the same // We cannot allow two local files in the project to point to the same
// remote path // remote path
oldLocalName, ok := remoteToLocalNames[remoteName] prevLocalName, ok := remoteToLocalNames[remoteName]
if ok && oldLocalName != f.Relative { _, prevLocalFileExists := localFileSet[prevLocalName]
return change, fmt.Errorf("both %s and %s point to the same remote file location %s. Please remove one of them from your local project", oldLocalName, f.Relative, remoteName) if ok && prevLocalName != f.Relative && prevLocalFileExists {
return change, fmt.Errorf("both %s and %s point to the same remote file location %s. Please remove one of them from your local project", prevLocalName, f.Relative, remoteName)
} }
localToRemoteNames[f.Relative] = remoteName localToRemoteNames[f.Relative] = remoteName
remoteToLocalNames[remoteName] = f.Relative remoteToLocalNames[remoteName] = f.Relative
@ -220,7 +226,7 @@ func (s *Snapshot) diff(all []fileset.File) (change diff, err error) {
// figure out files in the snapshot.lastModifiedTimes, but not on local // figure out files in the snapshot.lastModifiedTimes, but not on local
// filesystem. These will be deleted // filesystem. These will be deleted
for localName := range lastModifiedTimes { for localName := range lastModifiedTimes {
_, exists := currentFilenames[localName] _, exists := localFileSet[localName]
if exists { if exists {
continue continue
} }

View File

@ -249,6 +249,43 @@ func TestErrorWhenIdenticalRemoteName(t *testing.T) {
assert.ErrorContains(t, err, "both foo and foo.py point to the same remote file location foo. Please remove one of them from your local project") assert.ErrorContains(t, err, "both foo and foo.py point to the same remote file location foo. Please remove one of them from your local project")
} }
func TestNoErrorRenameWithIdenticalRemoteName(t *testing.T) {
// Create temp project dir
projectDir := t.TempDir()
fileSet, err := git.NewFileSet(projectDir)
require.NoError(t, err)
state := Snapshot{
LastUpdatedTimes: make(map[string]time.Time),
LocalToRemoteNames: make(map[string]string),
RemoteToLocalNames: make(map[string]string),
}
// upload should work since they point to different destinations
pythonFoo := testfile.CreateFile(t, filepath.Join(projectDir, "foo.py"))
defer pythonFoo.Close(t)
pythonFoo.Overwrite(t, "# Databricks notebook source\n")
files, err := fileSet.All()
assert.NoError(t, err)
change, err := state.diff(files)
assert.NoError(t, err)
assert.Len(t, change.delete, 0)
assert.Len(t, change.put, 1)
assert.Contains(t, change.put, "foo.py")
pythonFoo.Remove(t)
sqlFoo := testfile.CreateFile(t, filepath.Join(projectDir, "foo.sql"))
defer sqlFoo.Close(t)
sqlFoo.Overwrite(t, "-- Databricks notebook source\n")
files, err = fileSet.All()
assert.NoError(t, err)
change, err = state.diff(files)
assert.NoError(t, err)
assert.Len(t, change.delete, 1)
assert.Len(t, change.put, 1)
assert.Contains(t, change.put, "foo.sql")
assert.Contains(t, change.delete, "foo")
}
func defaultOptions(t *testing.T) *SyncOptions { func defaultOptions(t *testing.T) *SyncOptions {
return &SyncOptions{ return &SyncOptions{
Host: "www.foobar.com", Host: "www.foobar.com",