Postpone directory creation until upload start

This ensure that "bundle validate" does not create remote directory.

Extend empty_bundle_test.go to check if remote path is created prematurely
This commit is contained in:
Denis Bilenko 2024-12-04 20:48:48 +01:00
parent 2ee7d56ae6
commit 48048ddd55
4 changed files with 70 additions and 26 deletions

View File

@ -8,11 +8,17 @@ import (
"github.com/databricks/cli/internal/acc" "github.com/databricks/cli/internal/acc"
"github.com/google/uuid" "github.com/google/uuid"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require" "github.com/stretchr/testify/require"
) )
func TestAccEmptyBundleDeploy(t *testing.T) { func TestAccEmptyBundleDeploy(t *testing.T) {
ctx, _ := acc.WorkspaceTest(t) ctx, w := acc.WorkspaceTest(t)
uniqueId := uuid.New().String()
me, err := w.W.CurrentUser.Me(ctx)
require.NoError(t, err)
remoteRoot := fmt.Sprintf("/Workspace/Users/%s/.bundle/%s", me.UserName, uniqueId)
// create empty bundle // create empty bundle
tmpDir := t.TempDir() tmpDir := t.TempDir()
@ -20,11 +26,20 @@ func TestAccEmptyBundleDeploy(t *testing.T) {
require.NoError(t, err) require.NoError(t, err)
bundleRoot := fmt.Sprintf(`bundle: bundleRoot := fmt.Sprintf(`bundle:
name: %s`, uuid.New().String()) name: %s`, uniqueId)
_, err = f.WriteString(bundleRoot) _, err = f.WriteString(bundleRoot)
require.NoError(t, err) require.NoError(t, err)
f.Close() f.Close()
_, err = w.W.Workspace.GetStatusByPath(ctx, remoteRoot)
assert.ErrorContains(t, err, "doesn't exist")
mustValidateBundle(t, ctx, tmpDir)
// regression: "bundle validate" must not create a directory
_, err = w.W.Workspace.GetStatusByPath(ctx, remoteRoot)
require.ErrorContains(t, err, "doesn't exist")
// deploy empty bundle // deploy empty bundle
err = deployBundle(t, ctx, tmpDir) err = deployBundle(t, ctx, tmpDir)
require.NoError(t, err) require.NoError(t, err)
@ -33,4 +48,9 @@ func TestAccEmptyBundleDeploy(t *testing.T) {
err = destroyBundle(t, ctx, tmpDir) err = destroyBundle(t, ctx, tmpDir)
require.NoError(t, err) require.NoError(t, err)
}) })
// verify that remoteRoot was actually relevant location to test
_, err = w.W.Workspace.GetStatusByPath(ctx, remoteRoot)
assert.NoError(t, err)
} }

View File

@ -509,13 +509,15 @@ func TestAccSyncEnsureRemotePathIsUsableIfRepoDoesntExist(t *testing.T) {
// Hypothetical repo path doesn't exist. // Hypothetical repo path doesn't exist.
nonExistingRepoPath := fmt.Sprintf("/Repos/%s/%s", me.UserName, RandomName("doesnt-exist-")) nonExistingRepoPath := fmt.Sprintf("/Repos/%s/%s", me.UserName, RandomName("doesnt-exist-"))
err = sync.EnsureRemotePathIsUsable(ctx, wsc, nonExistingRepoPath, nil) remoteExists, err := sync.EnsureRemotePathIsUsable(ctx, wsc, nonExistingRepoPath, nil)
assert.ErrorContains(t, err, " does not exist; please create it first") assert.ErrorContains(t, err, " does not exist; please create it first")
assert.False(t, remoteExists)
// Paths nested under a hypothetical repo path should yield the same error. // Paths nested under a hypothetical repo path should yield the same error.
nestedPath := path.Join(nonExistingRepoPath, "nested/directory") nestedPath := path.Join(nonExistingRepoPath, "nested/directory")
err = sync.EnsureRemotePathIsUsable(ctx, wsc, nestedPath, nil) remoteExists, err = sync.EnsureRemotePathIsUsable(ctx, wsc, nestedPath, nil)
assert.ErrorContains(t, err, " does not exist; please create it first") assert.ErrorContains(t, err, " does not exist; please create it first")
assert.False(t, remoteExists)
} }
func TestAccSyncEnsureRemotePathIsUsableIfRepoExists(t *testing.T) { func TestAccSyncEnsureRemotePathIsUsableIfRepoExists(t *testing.T) {
@ -526,13 +528,15 @@ func TestAccSyncEnsureRemotePathIsUsableIfRepoExists(t *testing.T) {
_, remoteRepoPath := setupRepo(t, wsc, ctx) _, remoteRepoPath := setupRepo(t, wsc, ctx)
// Repo itself is usable. // Repo itself is usable.
err := sync.EnsureRemotePathIsUsable(ctx, wsc, remoteRepoPath, nil) remoteExists, err := sync.EnsureRemotePathIsUsable(ctx, wsc, remoteRepoPath, nil)
assert.NoError(t, err) assert.NoError(t, err)
assert.True(t, remoteExists)
// Path nested under repo path is usable. // Path nested under repo path is usable.
nestedPath := path.Join(remoteRepoPath, "nested/directory") nestedPath := path.Join(remoteRepoPath, "nested/directory")
err = sync.EnsureRemotePathIsUsable(ctx, wsc, nestedPath, nil) remoteExists, err = sync.EnsureRemotePathIsUsable(ctx, wsc, nestedPath, nil)
assert.NoError(t, err) assert.NoError(t, err)
assert.False(t, remoteExists)
// Verify that the directory has been created. // Verify that the directory has been created.
info, err := wsc.Workspace.GetStatusByPath(ctx, nestedPath) info, err := wsc.Workspace.GetStatusByPath(ctx, nestedPath)
@ -549,8 +553,9 @@ func TestAccSyncEnsureRemotePathIsUsableInWorkspace(t *testing.T) {
require.NoError(t, err) require.NoError(t, err)
remotePath := fmt.Sprintf("/Users/%s/%s", me.UserName, RandomName("ensure-path-exists-test-")) remotePath := fmt.Sprintf("/Users/%s/%s", me.UserName, RandomName("ensure-path-exists-test-"))
err = sync.EnsureRemotePathIsUsable(ctx, wsc, remotePath, me) remoteExists, err := sync.EnsureRemotePathIsUsable(ctx, wsc, remotePath, me)
assert.NoError(t, err) assert.NoError(t, err)
assert.False(t, remoteExists)
// Clean up directory after test. // Clean up directory after test.
defer func() { defer func() {
@ -560,8 +565,8 @@ func TestAccSyncEnsureRemotePathIsUsableInWorkspace(t *testing.T) {
assert.NoError(t, err) assert.NoError(t, err)
}() }()
// Verify that the directory has been created. // Verify that the directory has not been created.
info, err := wsc.Workspace.GetStatusByPath(ctx, remotePath) info, err := wsc.Workspace.GetStatusByPath(ctx, remotePath)
require.NoError(t, err) require.ErrorContains(t, err, "not exist")
require.Equal(t, workspace.ObjectTypeDirectory, info.ObjectType) require.Equal(t, workspace.ObjectTypeDirectory, info.ObjectType)
} }

View File

@ -24,7 +24,8 @@ func repoPathForPath(me *iam.User, remotePath string) string {
// EnsureRemotePathIsUsable checks if the specified path is nested under // EnsureRemotePathIsUsable checks if the specified path is nested under
// expected base paths and if it is a directory or repository. // expected base paths and if it is a directory or repository.
func EnsureRemotePathIsUsable(ctx context.Context, wsc *databricks.WorkspaceClient, remotePath string, me *iam.User) error { // Returns (doesRemoteExist, error)
func EnsureRemotePathIsUsable(ctx context.Context, wsc *databricks.WorkspaceClient, remotePath string, me *iam.User) (bool, error) {
var err error var err error
// TODO: we should cache CurrentUser.Me at the SDK level // TODO: we should cache CurrentUser.Me at the SDK level
@ -32,7 +33,7 @@ func EnsureRemotePathIsUsable(ctx context.Context, wsc *databricks.WorkspaceClie
if me == nil { if me == nil {
me, err = wsc.CurrentUser.Me(ctx) me, err = wsc.CurrentUser.Me(ctx)
if err != nil { if err != nil {
return err return false, err
} }
} }
@ -43,7 +44,7 @@ func EnsureRemotePathIsUsable(ctx context.Context, wsc *databricks.WorkspaceClie
if err != nil { if err != nil {
// We only deal with 404s below. // We only deal with 404s below.
if !apierr.IsMissing(err) { if !apierr.IsMissing(err) {
return err return false, err
} }
// If the path is nested under a repo, the repo has to exist. // If the path is nested under a repo, the repo has to exist.
@ -51,19 +52,12 @@ func EnsureRemotePathIsUsable(ctx context.Context, wsc *databricks.WorkspaceClie
repoPath := repoPathForPath(me, remotePath) repoPath := repoPathForPath(me, remotePath)
_, err = wsc.Workspace.GetStatusByPath(ctx, repoPath) _, err = wsc.Workspace.GetStatusByPath(ctx, repoPath)
if err != nil && apierr.IsMissing(err) { if err != nil && apierr.IsMissing(err) {
return fmt.Errorf("%s does not exist; please create it first", repoPath) return false, fmt.Errorf("%s does not exist; please create it first", repoPath)
} }
} }
// The workspace path doesn't exist. Create it and try again. return false, nil
err = wsc.Workspace.MkdirsByPath(ctx, remotePath)
if err != nil {
return fmt.Errorf("unable to create directory at %s: %w", remotePath, err)
}
info, err = wsc.Workspace.GetStatusByPath(ctx, remotePath)
if err != nil {
return err
}
} }
log.Debugf( log.Debugf(
@ -77,10 +71,23 @@ func EnsureRemotePathIsUsable(ctx context.Context, wsc *databricks.WorkspaceClie
// We expect the object at path to be a directory or a repo. // We expect the object at path to be a directory or a repo.
switch info.ObjectType { switch info.ObjectType {
case workspace.ObjectTypeDirectory: case workspace.ObjectTypeDirectory:
return nil return true, nil
case workspace.ObjectTypeRepo: case workspace.ObjectTypeRepo:
return nil return true, nil
} }
return fmt.Errorf("%s points to a %s", remotePath, strings.ToLower(info.ObjectType.String())) return true, fmt.Errorf("%s points to a %s", remotePath, strings.ToLower(info.ObjectType.String()))
}
func createRemotePath(ctx context.Context, wsc *databricks.WorkspaceClient, remotePath string) error {
// The workspace path doesn't exist. Create it and try again.
err := wsc.Workspace.MkdirsByPath(ctx, remotePath)
if err != nil {
return fmt.Errorf("unable to create directory at %s: %w", remotePath, err)
}
_, err = wsc.Workspace.GetStatusByPath(ctx, remotePath)
if err != nil {
return err
}
return nil
} }

View File

@ -59,6 +59,9 @@ type Sync struct {
// WaitGroup is automatically created when an output handler is provided in the SyncOptions. // WaitGroup is automatically created when an output handler is provided in the SyncOptions.
// Close call is required to ensure the output handler goroutine handles all events in time. // Close call is required to ensure the output handler goroutine handles all events in time.
outputWaitGroup *stdsync.WaitGroup outputWaitGroup *stdsync.WaitGroup
// If this flag is not set, we'll create remote directory before starting upload
remoteExists bool
} }
// New initializes and returns a new [Sync] instance. // New initializes and returns a new [Sync] instance.
@ -84,7 +87,7 @@ func New(ctx context.Context, opts SyncOptions) (*Sync, error) {
} }
// Verify that the remote path we're about to synchronize to is valid and allowed. // Verify that the remote path we're about to synchronize to is valid and allowed.
err = EnsureRemotePathIsUsable(ctx, opts.WorkspaceClient, opts.RemotePath, opts.CurrentUser) remoteExists, err := EnsureRemotePathIsUsable(ctx, opts.WorkspaceClient, opts.RemotePath, opts.CurrentUser)
if err != nil { if err != nil {
return nil, err return nil, err
} }
@ -141,6 +144,7 @@ func New(ctx context.Context, opts SyncOptions) (*Sync, error) {
notifier: notifier, notifier: notifier,
outputWaitGroup: outputWaitGroup, outputWaitGroup: outputWaitGroup,
seq: 0, seq: 0,
remoteExists: remoteExists,
}, nil }, nil
} }
@ -180,6 +184,14 @@ func (s *Sync) notifyComplete(ctx context.Context, d diff) {
// Returns the list of files tracked (and synchronized) by the syncer during the run, // Returns the list of files tracked (and synchronized) by the syncer during the run,
// and an error if any occurred. // and an error if any occurred.
func (s *Sync) RunOnce(ctx context.Context) ([]fileset.File, error) { func (s *Sync) RunOnce(ctx context.Context) ([]fileset.File, error) {
if !s.remoteExists {
err := createRemotePath(ctx, s.WorkspaceClient, s.RemotePath)
if err != nil {
return nil, err
}
s.remoteExists = true
}
files, err := s.GetFileList(ctx) files, err := s.GetFileList(ctx)
if err != nil { if err != nil {
return files, err return files, err