From b388f4a0dca193fcc76e5c6319920b69b5551210 Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Wed, 12 Apr 2023 16:54:36 +0200 Subject: [PATCH] Make all workspace paths string fields (#327) ## Changes These are unlikely to ever be DBFS paths so we can remove this level of indirection to simplify. **Note:** this is a breaking change. Downstream usage of these fields must be updated. ## Tests Existing tests pass. --- bundle/artifacts/notebook/build.go | 2 +- .../config/mutator/default_workspace_paths.go | 14 ++++----- .../mutator/default_workspace_paths_test.go | 28 +++++++----------- .../config/mutator/default_workspace_root.go | 4 +-- .../mutator/default_workspace_root_test.go | 2 +- .../config/mutator/expand_workspace_root.go | 4 +-- .../mutator/expand_workspace_root_test.go | 10 +++---- bundle/config/mutator/translate_paths.go | 2 +- bundle/config/mutator/translate_paths_test.go | 12 ++------ bundle/config/workspace.go | 29 +++++-------------- bundle/deploy/files/delete.go | 6 ++-- bundle/deploy/files/sync.go | 2 +- bundle/deploy/lock/acquire.go | 2 +- bundle/deploy/terraform/state_pull.go | 2 +- bundle/deploy/terraform/state_push.go | 2 +- cmd/bundle/sync.go | 2 +- cmd/sync/sync.go | 2 +- cmd/sync/sync_test.go | 4 +-- 18 files changed, 51 insertions(+), 78 deletions(-) diff --git a/bundle/artifacts/notebook/build.go b/bundle/artifacts/notebook/build.go index bccd0ba5..6ba9f3b0 100644 --- a/bundle/artifacts/notebook/build.go +++ b/bundle/artifacts/notebook/build.go @@ -64,7 +64,7 @@ func (m *build) Apply(_ context.Context, b *bundle.Bundle) ([]bundle.Mutator, er } // Check that an artifact path is defined. - remotePath := b.Config.Workspace.ArtifactPath.Workspace + remotePath := b.Config.Workspace.ArtifactsPath if remotePath == "" { return nil, fmt.Errorf("remote artifact path not configured") } diff --git a/bundle/config/mutator/default_workspace_paths.go b/bundle/config/mutator/default_workspace_paths.go index 5ef5efc2..c11e4fb9 100644 --- a/bundle/config/mutator/default_workspace_paths.go +++ b/bundle/config/mutator/default_workspace_paths.go @@ -20,21 +20,21 @@ func (m *defineDefaultWorkspacePaths) Name() string { } func (m *defineDefaultWorkspacePaths) Apply(ctx context.Context, b *bundle.Bundle) ([]bundle.Mutator, error) { - root := b.Config.Workspace.Root + root := b.Config.Workspace.RootPath if root == "" { return nil, fmt.Errorf("unable to define default workspace paths: workspace root not defined") } - if !b.Config.Workspace.FilePath.IsSet() { - b.Config.Workspace.FilePath.Workspace = path.Join(root, "files") + if b.Config.Workspace.FilesPath == "" { + b.Config.Workspace.FilesPath = path.Join(root, "files") } - if !b.Config.Workspace.ArtifactPath.IsSet() { - b.Config.Workspace.ArtifactPath.Workspace = path.Join(root, "artifacts") + if b.Config.Workspace.ArtifactsPath == "" { + b.Config.Workspace.ArtifactsPath = path.Join(root, "artifacts") } - if !b.Config.Workspace.StatePath.IsSet() { - b.Config.Workspace.StatePath.Workspace = path.Join(root, "state") + if b.Config.Workspace.StatePath == "" { + b.Config.Workspace.StatePath = path.Join(root, "state") } return nil, nil diff --git a/bundle/config/mutator/default_workspace_paths_test.go b/bundle/config/mutator/default_workspace_paths_test.go index 6d026159..2a2f8f34 100644 --- a/bundle/config/mutator/default_workspace_paths_test.go +++ b/bundle/config/mutator/default_workspace_paths_test.go @@ -15,37 +15,31 @@ func TestDefineDefaultWorkspacePaths(t *testing.T) { bundle := &bundle.Bundle{ Config: config.Root{ Workspace: config.Workspace{ - Root: "/", + RootPath: "/", }, }, } _, err := mutator.DefineDefaultWorkspacePaths().Apply(context.Background(), bundle) require.NoError(t, err) - assert.Equal(t, "/files", bundle.Config.Workspace.FilePath.Workspace) - assert.Equal(t, "/artifacts", bundle.Config.Workspace.ArtifactPath.Workspace) - assert.Equal(t, "/state", bundle.Config.Workspace.StatePath.Workspace) + assert.Equal(t, "/files", bundle.Config.Workspace.FilesPath) + assert.Equal(t, "/artifacts", bundle.Config.Workspace.ArtifactsPath) + assert.Equal(t, "/state", bundle.Config.Workspace.StatePath) } func TestDefineDefaultWorkspacePathsAlreadySet(t *testing.T) { bundle := &bundle.Bundle{ Config: config.Root{ Workspace: config.Workspace{ - Root: "/", - FilePath: config.PathLike{ - Workspace: "/foo/bar", - }, - ArtifactPath: config.PathLike{ - Workspace: "/foo/bar", - }, - StatePath: config.PathLike{ - Workspace: "/foo/bar", - }, + RootPath: "/", + FilesPath: "/foo/bar", + ArtifactsPath: "/foo/bar", + StatePath: "/foo/bar", }, }, } _, err := mutator.DefineDefaultWorkspacePaths().Apply(context.Background(), bundle) require.NoError(t, err) - assert.Equal(t, "/foo/bar", bundle.Config.Workspace.FilePath.Workspace) - assert.Equal(t, "/foo/bar", bundle.Config.Workspace.ArtifactPath.Workspace) - assert.Equal(t, "/foo/bar", bundle.Config.Workspace.StatePath.Workspace) + assert.Equal(t, "/foo/bar", bundle.Config.Workspace.FilesPath) + assert.Equal(t, "/foo/bar", bundle.Config.Workspace.ArtifactsPath) + assert.Equal(t, "/foo/bar", bundle.Config.Workspace.StatePath) } diff --git a/bundle/config/mutator/default_workspace_root.go b/bundle/config/mutator/default_workspace_root.go index 0b57abc9..448a894e 100644 --- a/bundle/config/mutator/default_workspace_root.go +++ b/bundle/config/mutator/default_workspace_root.go @@ -19,7 +19,7 @@ func (m *defineDefaultWorkspaceRoot) Name() string { } func (m *defineDefaultWorkspaceRoot) Apply(ctx context.Context, b *bundle.Bundle) ([]bundle.Mutator, error) { - if b.Config.Workspace.Root != "" { + if b.Config.Workspace.RootPath != "" { return nil, nil } @@ -31,7 +31,7 @@ func (m *defineDefaultWorkspaceRoot) Apply(ctx context.Context, b *bundle.Bundle return nil, fmt.Errorf("unable to define default workspace root: bundle environment not selected") } - b.Config.Workspace.Root = fmt.Sprintf( + b.Config.Workspace.RootPath = fmt.Sprintf( "~/.bundle/%s/%s", b.Config.Bundle.Name, b.Config.Bundle.Environment, diff --git a/bundle/config/mutator/default_workspace_root_test.go b/bundle/config/mutator/default_workspace_root_test.go index b2dd1fd4..cb0d9b17 100644 --- a/bundle/config/mutator/default_workspace_root_test.go +++ b/bundle/config/mutator/default_workspace_root_test.go @@ -22,5 +22,5 @@ func TestDefaultWorkspaceRoot(t *testing.T) { } _, err := mutator.DefineDefaultWorkspaceRoot().Apply(context.Background(), bundle) require.NoError(t, err) - assert.Equal(t, "~/.bundle/name/environment", bundle.Config.Workspace.Root) + assert.Equal(t, "~/.bundle/name/environment", bundle.Config.Workspace.RootPath) } diff --git a/bundle/config/mutator/expand_workspace_root.go b/bundle/config/mutator/expand_workspace_root.go index cb111ce2..b1bb7458 100644 --- a/bundle/config/mutator/expand_workspace_root.go +++ b/bundle/config/mutator/expand_workspace_root.go @@ -21,7 +21,7 @@ func (m *expandWorkspaceRoot) Name() string { } func (m *expandWorkspaceRoot) Apply(ctx context.Context, b *bundle.Bundle) ([]bundle.Mutator, error) { - root := b.Config.Workspace.Root + root := b.Config.Workspace.RootPath if root == "" { return nil, fmt.Errorf("unable to expand workspace root: workspace root not defined") } @@ -33,7 +33,7 @@ func (m *expandWorkspaceRoot) Apply(ctx context.Context, b *bundle.Bundle) ([]bu if strings.HasPrefix(root, "~/") { home := fmt.Sprintf("/Users/%s", currentUser.UserName) - b.Config.Workspace.Root = path.Join(home, root[2:]) + b.Config.Workspace.RootPath = path.Join(home, root[2:]) } return nil, nil diff --git a/bundle/config/mutator/expand_workspace_root_test.go b/bundle/config/mutator/expand_workspace_root_test.go index 30745877..557eb4dc 100644 --- a/bundle/config/mutator/expand_workspace_root_test.go +++ b/bundle/config/mutator/expand_workspace_root_test.go @@ -19,13 +19,13 @@ func TestExpandWorkspaceRoot(t *testing.T) { CurrentUser: &scim.User{ UserName: "jane@doe.com", }, - Root: "~/foo", + RootPath: "~/foo", }, }, } _, err := mutator.ExpandWorkspaceRoot().Apply(context.Background(), bundle) require.NoError(t, err) - assert.Equal(t, "/Users/jane@doe.com/foo", bundle.Config.Workspace.Root) + assert.Equal(t, "/Users/jane@doe.com/foo", bundle.Config.Workspace.RootPath) } func TestExpandWorkspaceRootDoesNothing(t *testing.T) { @@ -35,13 +35,13 @@ func TestExpandWorkspaceRootDoesNothing(t *testing.T) { CurrentUser: &scim.User{ UserName: "jane@doe.com", }, - Root: "/Users/charly@doe.com/foo", + RootPath: "/Users/charly@doe.com/foo", }, }, } _, err := mutator.ExpandWorkspaceRoot().Apply(context.Background(), bundle) require.NoError(t, err) - assert.Equal(t, "/Users/charly@doe.com/foo", bundle.Config.Workspace.Root) + assert.Equal(t, "/Users/charly@doe.com/foo", bundle.Config.Workspace.RootPath) } func TestExpandWorkspaceRootWithoutRoot(t *testing.T) { @@ -62,7 +62,7 @@ func TestExpandWorkspaceRootWithoutCurrentUser(t *testing.T) { bundle := &bundle.Bundle{ Config: config.Root{ Workspace: config.Workspace{ - Root: "~/foo", + RootPath: "~/foo", }, }, } diff --git a/bundle/config/mutator/translate_paths.go b/bundle/config/mutator/translate_paths.go index fe253452..fa1851f4 100644 --- a/bundle/config/mutator/translate_paths.go +++ b/bundle/config/mutator/translate_paths.go @@ -64,7 +64,7 @@ func (m *translatePaths) rewritePath( } // Prefix remote path with its remote root path. - remotePath = path.Join(b.Config.Workspace.FilePath.Workspace, filepath.ToSlash(remotePath)) + remotePath = path.Join(b.Config.Workspace.FilesPath, filepath.ToSlash(remotePath)) // Convert local path into workspace path via specified function. interp, err := fn(*p, localPath, filepath.ToSlash(remotePath)) diff --git a/bundle/config/mutator/translate_paths_test.go b/bundle/config/mutator/translate_paths_test.go index aff0d71f..87ec4b16 100644 --- a/bundle/config/mutator/translate_paths_test.go +++ b/bundle/config/mutator/translate_paths_test.go @@ -41,9 +41,7 @@ func TestTranslatePaths(t *testing.T) { Config: config.Root{ Path: dir, Workspace: config.Workspace{ - FilePath: config.PathLike{ - Workspace: "/bundle", - }, + FilesPath: "/bundle", }, Resources: config.Resources{ Jobs: map[string]*resources.Job{ @@ -177,9 +175,7 @@ func TestTranslatePathsInSubdirectories(t *testing.T) { Config: config.Root{ Path: dir, Workspace: config.Workspace{ - FilePath: config.PathLike{ - Workspace: "/bundle", - }, + FilesPath: "/bundle", }, Resources: config.Resources{ Jobs: map[string]*resources.Job{ @@ -242,9 +238,7 @@ func TestTranslatePathsOutsideBundleRoot(t *testing.T) { Config: config.Root{ Path: dir, Workspace: config.Workspace{ - FilePath: config.PathLike{ - Workspace: "/bundle", - }, + FilesPath: "/bundle", }, Resources: config.Resources{ Jobs: map[string]*resources.Job{ diff --git a/bundle/config/workspace.go b/bundle/config/workspace.go index 85e8905e..d39e4413 100644 --- a/bundle/config/workspace.go +++ b/bundle/config/workspace.go @@ -10,19 +10,6 @@ import ( "github.com/databricks/databricks-sdk-go/service/scim" ) -type PathLike struct { - // Workspace contains a WSFS path. - Workspace string `json:"workspace,omitempty"` - - // DBFS contains a DBFS path. - DBFS string `json:"dbfs,omitempty"` -} - -// IsSet returns whether either path is non-nil. -func (p PathLike) IsSet() bool { - return p.Workspace != "" || p.DBFS != "" -} - // Workspace defines configurables at the workspace level. type Workspace struct { // Unified authentication attributes. @@ -52,22 +39,22 @@ type Workspace struct { // This is set after configuration initialization. CurrentUser *scim.User `json:"current_user,omitempty" bundle:"readonly"` - // Remote base path for deployment state, for artifacts, as synchronization target. + // Remote workspace base path for deployment state, for artifacts, as synchronization target. // This defaults to "~/.bundle/${bundle.name}/${bundle.environment}" where "~" expands to // the current user's home directory in the workspace (e.g. `/Users/jane@doe.com`). - Root string `json:"root,omitempty"` + RootPath string `json:"root_path,omitempty"` - // Remote path to synchronize local files to. + // Remote workspace path to synchronize local files to. // This defaults to "${workspace.root}/files". - FilePath PathLike `json:"file_path,omitempty"` + FilesPath string `json:"file_path,omitempty"` - // Remote path for build artifacts. + // Remote workspace path for build artifacts. // This defaults to "${workspace.root}/artifacts". - ArtifactPath PathLike `json:"artifact_path,omitempty"` + ArtifactsPath string `json:"artifact_path,omitempty"` - // Remote path for deployment state. + // Remote workspace path for deployment state. // This defaults to "${workspace.root}/state". - StatePath PathLike `json:"state_path,omitempty"` + StatePath string `json:"state_path,omitempty"` } func (w *Workspace) Client() (*databricks.WorkspaceClient, error) { diff --git a/bundle/deploy/files/delete.go b/bundle/deploy/files/delete.go index 673adbb4..8a6dcd01 100644 --- a/bundle/deploy/files/delete.go +++ b/bundle/deploy/files/delete.go @@ -30,9 +30,9 @@ func (m *delete) Apply(ctx context.Context, b *bundle.Bundle) ([]bundle.Mutator, } red := color.New(color.FgRed).SprintFunc() - fmt.Fprintf(os.Stderr, "\nRemote directory %s will be deleted\n", b.Config.Workspace.Root) + fmt.Fprintf(os.Stderr, "\nRemote directory %s will be deleted\n", b.Config.Workspace.RootPath) if !b.AutoApprove { - proceed, err := logger.Ask(fmt.Sprintf("%s and all files in it will be %s Proceed?: ", b.Config.Workspace.Root, red("deleted permanently!"))) + proceed, err := logger.Ask(fmt.Sprintf("%s and all files in it will be %s Proceed?: ", b.Config.Workspace.RootPath, red("deleted permanently!"))) if err != nil { return nil, err } @@ -42,7 +42,7 @@ func (m *delete) Apply(ctx context.Context, b *bundle.Bundle) ([]bundle.Mutator, } err := b.WorkspaceClient().Workspace.Delete(ctx, workspace.Delete{ - Path: b.Config.Workspace.Root, + Path: b.Config.Workspace.RootPath, Recursive: true, }) if err != nil { diff --git a/bundle/deploy/files/sync.go b/bundle/deploy/files/sync.go index 262a205f..824e0b10 100644 --- a/bundle/deploy/files/sync.go +++ b/bundle/deploy/files/sync.go @@ -16,7 +16,7 @@ func getSync(ctx context.Context, b *bundle.Bundle) (*sync.Sync, error) { opts := sync.SyncOptions{ LocalPath: b.Config.Path, - RemotePath: b.Config.Workspace.FilePath.Workspace, + RemotePath: b.Config.Workspace.FilesPath, Full: false, SnapshotBasePath: cacheDir, diff --git a/bundle/deploy/lock/acquire.go b/bundle/deploy/lock/acquire.go index 5069ea76..e399147a 100644 --- a/bundle/deploy/lock/acquire.go +++ b/bundle/deploy/lock/acquire.go @@ -20,7 +20,7 @@ func (m *acquire) Name() string { func (m *acquire) init(b *bundle.Bundle) error { user := b.Config.Workspace.CurrentUser.UserName - dir := b.Config.Workspace.StatePath.Workspace + dir := b.Config.Workspace.StatePath l, err := locker.CreateLocker(user, dir, b.WorkspaceClient()) if err != nil { return err diff --git a/bundle/deploy/terraform/state_pull.go b/bundle/deploy/terraform/state_pull.go index 04a6b2eb..93ae3248 100644 --- a/bundle/deploy/terraform/state_pull.go +++ b/bundle/deploy/terraform/state_pull.go @@ -19,7 +19,7 @@ func (l *statePull) Name() string { } func (l *statePull) Apply(ctx context.Context, b *bundle.Bundle) ([]bundle.Mutator, error) { - f, err := filer.NewWorkspaceFilesClient(b.WorkspaceClient(), b.Config.Workspace.StatePath.Workspace) + f, err := filer.NewWorkspaceFilesClient(b.WorkspaceClient(), b.Config.Workspace.StatePath) if err != nil { return nil, err } diff --git a/bundle/deploy/terraform/state_push.go b/bundle/deploy/terraform/state_push.go index 03c1bc4c..43ae9171 100644 --- a/bundle/deploy/terraform/state_push.go +++ b/bundle/deploy/terraform/state_push.go @@ -17,7 +17,7 @@ func (l *statePush) Name() string { } func (l *statePush) Apply(ctx context.Context, b *bundle.Bundle) ([]bundle.Mutator, error) { - f, err := filer.NewWorkspaceFilesClient(b.WorkspaceClient(), b.Config.Workspace.StatePath.Workspace) + f, err := filer.NewWorkspaceFilesClient(b.WorkspaceClient(), b.Config.Workspace.StatePath) if err != nil { return nil, err } diff --git a/cmd/bundle/sync.go b/cmd/bundle/sync.go index a74b4bda..6888cd4c 100644 --- a/cmd/bundle/sync.go +++ b/cmd/bundle/sync.go @@ -20,7 +20,7 @@ func syncOptionsFromBundle(cmd *cobra.Command, b *bundle.Bundle) (*sync.SyncOpti opts := sync.SyncOptions{ LocalPath: b.Config.Path, - RemotePath: b.Config.Workspace.FilePath.Workspace, + RemotePath: b.Config.Workspace.FilesPath, Full: full, PollInterval: interval, diff --git a/cmd/sync/sync.go b/cmd/sync/sync.go index 76381fc8..61188fb2 100644 --- a/cmd/sync/sync.go +++ b/cmd/sync/sync.go @@ -29,7 +29,7 @@ func syncOptionsFromBundle(cmd *cobra.Command, args []string, b *bundle.Bundle) opts := sync.SyncOptions{ LocalPath: b.Config.Path, - RemotePath: b.Config.Workspace.FilePath.Workspace, + RemotePath: b.Config.Workspace.FilesPath, Full: full, PollInterval: interval, diff --git a/cmd/sync/sync_test.go b/cmd/sync/sync_test.go index a5161e09..7b0420d4 100644 --- a/cmd/sync/sync_test.go +++ b/cmd/sync/sync_test.go @@ -22,9 +22,7 @@ func TestSyncOptionsFromBundle(t *testing.T) { }, Workspace: config.Workspace{ - FilePath: config.PathLike{ - Workspace: "/Users/jane@doe.com/path", - }, + FilesPath: "/Users/jane@doe.com/path", }, }, }