From 35c3d9fa4e01c52c036135d81c0d22e6f4531ef2 Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Thu, 26 Jan 2023 19:55:38 +0100 Subject: [PATCH] Add workspace paths (#179) The workspace root path is a base path for bundle storage. If not specified, it defaults to `~/.bundle/name/environment`. This default, or other paths starting with `~` are expanded to the current user's home directory. The configuration also includes fields for the files path, artifacts path, and state path. By default, these are nested under the root path, but can be overridden if needed. --- .../config/mutator/default_artifact_path.go | 40 ----------- .../mutator/default_artifact_path_test.go | 50 ------------- .../config/mutator/default_workspace_paths.go | 41 +++++++++++ .../mutator/default_workspace_paths_test.go | 51 +++++++++++++ .../config/mutator/default_workspace_root.go | 40 +++++++++++ .../mutator/default_workspace_root_test.go | 26 +++++++ .../config/mutator/expand_workspace_root.go | 40 +++++++++++ .../mutator/expand_workspace_root_test.go | 71 +++++++++++++++++++ bundle/config/workspace.go | 19 +++-- bundle/phases/initialize.go | 4 +- 10 files changed, 287 insertions(+), 95 deletions(-) delete mode 100644 bundle/config/mutator/default_artifact_path.go delete mode 100644 bundle/config/mutator/default_artifact_path_test.go create mode 100644 bundle/config/mutator/default_workspace_paths.go create mode 100644 bundle/config/mutator/default_workspace_paths_test.go create mode 100644 bundle/config/mutator/default_workspace_root.go create mode 100644 bundle/config/mutator/default_workspace_root_test.go create mode 100644 bundle/config/mutator/expand_workspace_root.go create mode 100644 bundle/config/mutator/expand_workspace_root_test.go diff --git a/bundle/config/mutator/default_artifact_path.go b/bundle/config/mutator/default_artifact_path.go deleted file mode 100644 index e4d9edc1..00000000 --- a/bundle/config/mutator/default_artifact_path.go +++ /dev/null @@ -1,40 +0,0 @@ -package mutator - -import ( - "context" - "fmt" - "path" - - "github.com/databricks/bricks/bundle" -) - -type defaultArtifactPath struct{} - -// DefaultArtifactPath configures the artifact path if it isn't already set. -func DefaultArtifactPath() bundle.Mutator { - return &defaultArtifactPath{} -} - -func (m *defaultArtifactPath) Name() string { - return "DefaultArtifactPath" -} - -func (m *defaultArtifactPath) Apply(ctx context.Context, b *bundle.Bundle) ([]bundle.Mutator, error) { - if b.Config.Workspace.ArtifactPath.IsSet() { - return nil, nil - } - - me := b.Config.Workspace.CurrentUser - if me == nil { - return nil, fmt.Errorf("cannot configured default artifact path if current user isn't set") - } - - // We assume we deal with notebooks only for the time being. - // When we need to upload wheel files or other non-notebook files, - // the workspace must support "Files in Workspace". - // If it doesn't, we need to resort to storing artifacts on DBFS. - home := fmt.Sprintf("/Users/%s", me.UserName) - root := path.Join(home, ".bundle", b.Config.Bundle.Name, b.Config.Bundle.Environment, "artifacts") - b.Config.Workspace.ArtifactPath.Workspace = root - return nil, nil -} diff --git a/bundle/config/mutator/default_artifact_path_test.go b/bundle/config/mutator/default_artifact_path_test.go deleted file mode 100644 index 08205681..00000000 --- a/bundle/config/mutator/default_artifact_path_test.go +++ /dev/null @@ -1,50 +0,0 @@ -package mutator_test - -import ( - "context" - "testing" - - "github.com/databricks/bricks/bundle" - "github.com/databricks/bricks/bundle/config" - "github.com/databricks/bricks/bundle/config/mutator" - "github.com/databricks/databricks-sdk-go/service/scim" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" -) - -func TestDefaultArtifactPath(t *testing.T) { - bundle := &bundle.Bundle{ - Config: config.Root{ - Bundle: config.Bundle{ - Name: "name", - Environment: "environment", - }, - Workspace: config.Workspace{ - CurrentUser: &scim.User{ - UserName: "foo@bar.com", - }, - }, - }, - } - _, err := mutator.DefaultArtifactPath().Apply(context.Background(), bundle) - require.NoError(t, err) - assert.Equal(t, "/Users/foo@bar.com/.bundle/name/environment/artifacts", bundle.Config.Workspace.ArtifactPath.Workspace) -} - -func TestDefaultArtifactPathAlreadySet(t *testing.T) { - bundle := &bundle.Bundle{ - Config: config.Root{ - Workspace: config.Workspace{ - ArtifactPath: config.PathLike{ - Workspace: "/foo/bar", - }, - CurrentUser: &scim.User{ - UserName: "foo@bar.com", - }, - }, - }, - } - _, err := mutator.DefaultArtifactPath().Apply(context.Background(), bundle) - require.NoError(t, err) - assert.Equal(t, "/foo/bar", bundle.Config.Workspace.ArtifactPath.Workspace) -} diff --git a/bundle/config/mutator/default_workspace_paths.go b/bundle/config/mutator/default_workspace_paths.go new file mode 100644 index 00000000..5ef5efc2 --- /dev/null +++ b/bundle/config/mutator/default_workspace_paths.go @@ -0,0 +1,41 @@ +package mutator + +import ( + "context" + "fmt" + "path" + + "github.com/databricks/bricks/bundle" +) + +type defineDefaultWorkspacePaths struct{} + +// DefineDefaultWorkspacePaths sets workspace paths if they aren't already set. +func DefineDefaultWorkspacePaths() bundle.Mutator { + return &defineDefaultWorkspacePaths{} +} + +func (m *defineDefaultWorkspacePaths) Name() string { + return "DefaultWorkspacePaths" +} + +func (m *defineDefaultWorkspacePaths) Apply(ctx context.Context, b *bundle.Bundle) ([]bundle.Mutator, error) { + root := b.Config.Workspace.Root + 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.ArtifactPath.IsSet() { + b.Config.Workspace.ArtifactPath.Workspace = path.Join(root, "artifacts") + } + + if !b.Config.Workspace.StatePath.IsSet() { + b.Config.Workspace.StatePath.Workspace = 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 new file mode 100644 index 00000000..6d026159 --- /dev/null +++ b/bundle/config/mutator/default_workspace_paths_test.go @@ -0,0 +1,51 @@ +package mutator_test + +import ( + "context" + "testing" + + "github.com/databricks/bricks/bundle" + "github.com/databricks/bricks/bundle/config" + "github.com/databricks/bricks/bundle/config/mutator" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestDefineDefaultWorkspacePaths(t *testing.T) { + bundle := &bundle.Bundle{ + Config: config.Root{ + Workspace: config.Workspace{ + Root: "/", + }, + }, + } + _, 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) +} + +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", + }, + }, + }, + } + _, 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) +} diff --git a/bundle/config/mutator/default_workspace_root.go b/bundle/config/mutator/default_workspace_root.go new file mode 100644 index 00000000..0b57abc9 --- /dev/null +++ b/bundle/config/mutator/default_workspace_root.go @@ -0,0 +1,40 @@ +package mutator + +import ( + "context" + "fmt" + + "github.com/databricks/bricks/bundle" +) + +type defineDefaultWorkspaceRoot struct{} + +// DefineDefaultWorkspaceRoot defines the default workspace root path. +func DefineDefaultWorkspaceRoot() bundle.Mutator { + return &defineDefaultWorkspaceRoot{} +} + +func (m *defineDefaultWorkspaceRoot) Name() string { + return "DefineDefaultWorkspaceRoot" +} + +func (m *defineDefaultWorkspaceRoot) Apply(ctx context.Context, b *bundle.Bundle) ([]bundle.Mutator, error) { + if b.Config.Workspace.Root != "" { + return nil, nil + } + + if b.Config.Bundle.Name == "" { + return nil, fmt.Errorf("unable to define default workspace root: bundle name not defined") + } + + if b.Config.Bundle.Environment == "" { + return nil, fmt.Errorf("unable to define default workspace root: bundle environment not selected") + } + + b.Config.Workspace.Root = fmt.Sprintf( + "~/.bundle/%s/%s", + b.Config.Bundle.Name, + b.Config.Bundle.Environment, + ) + return nil, nil +} diff --git a/bundle/config/mutator/default_workspace_root_test.go b/bundle/config/mutator/default_workspace_root_test.go new file mode 100644 index 00000000..b2dd1fd4 --- /dev/null +++ b/bundle/config/mutator/default_workspace_root_test.go @@ -0,0 +1,26 @@ +package mutator_test + +import ( + "context" + "testing" + + "github.com/databricks/bricks/bundle" + "github.com/databricks/bricks/bundle/config" + "github.com/databricks/bricks/bundle/config/mutator" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestDefaultWorkspaceRoot(t *testing.T) { + bundle := &bundle.Bundle{ + Config: config.Root{ + Bundle: config.Bundle{ + Name: "name", + Environment: "environment", + }, + }, + } + _, err := mutator.DefineDefaultWorkspaceRoot().Apply(context.Background(), bundle) + require.NoError(t, err) + assert.Equal(t, "~/.bundle/name/environment", bundle.Config.Workspace.Root) +} diff --git a/bundle/config/mutator/expand_workspace_root.go b/bundle/config/mutator/expand_workspace_root.go new file mode 100644 index 00000000..cb111ce2 --- /dev/null +++ b/bundle/config/mutator/expand_workspace_root.go @@ -0,0 +1,40 @@ +package mutator + +import ( + "context" + "fmt" + "path" + "strings" + + "github.com/databricks/bricks/bundle" +) + +type expandWorkspaceRoot struct{} + +// ExpandWorkspaceRoot expands ~ if present in the workspace root. +func ExpandWorkspaceRoot() bundle.Mutator { + return &expandWorkspaceRoot{} +} + +func (m *expandWorkspaceRoot) Name() string { + return "ExpandWorkspaceRoot" +} + +func (m *expandWorkspaceRoot) Apply(ctx context.Context, b *bundle.Bundle) ([]bundle.Mutator, error) { + root := b.Config.Workspace.Root + if root == "" { + return nil, fmt.Errorf("unable to expand workspace root: workspace root not defined") + } + + currentUser := b.Config.Workspace.CurrentUser + if currentUser == nil || currentUser.UserName == "" { + return nil, fmt.Errorf("unable to expand workspace root: current user not set") + } + + if strings.HasPrefix(root, "~/") { + home := fmt.Sprintf("/Users/%s", currentUser.UserName) + b.Config.Workspace.Root = 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 new file mode 100644 index 00000000..30745877 --- /dev/null +++ b/bundle/config/mutator/expand_workspace_root_test.go @@ -0,0 +1,71 @@ +package mutator_test + +import ( + "context" + "testing" + + "github.com/databricks/bricks/bundle" + "github.com/databricks/bricks/bundle/config" + "github.com/databricks/bricks/bundle/config/mutator" + "github.com/databricks/databricks-sdk-go/service/scim" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestExpandWorkspaceRoot(t *testing.T) { + bundle := &bundle.Bundle{ + Config: config.Root{ + Workspace: config.Workspace{ + CurrentUser: &scim.User{ + UserName: "jane@doe.com", + }, + Root: "~/foo", + }, + }, + } + _, err := mutator.ExpandWorkspaceRoot().Apply(context.Background(), bundle) + require.NoError(t, err) + assert.Equal(t, "/Users/jane@doe.com/foo", bundle.Config.Workspace.Root) +} + +func TestExpandWorkspaceRootDoesNothing(t *testing.T) { + bundle := &bundle.Bundle{ + Config: config.Root{ + Workspace: config.Workspace{ + CurrentUser: &scim.User{ + UserName: "jane@doe.com", + }, + Root: "/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) +} + +func TestExpandWorkspaceRootWithoutRoot(t *testing.T) { + bundle := &bundle.Bundle{ + Config: config.Root{ + Workspace: config.Workspace{ + CurrentUser: &scim.User{ + UserName: "jane@doe.com", + }, + }, + }, + } + _, err := mutator.ExpandWorkspaceRoot().Apply(context.Background(), bundle) + require.Error(t, err) +} + +func TestExpandWorkspaceRootWithoutCurrentUser(t *testing.T) { + bundle := &bundle.Bundle{ + Config: config.Root{ + Workspace: config.Workspace{ + Root: "~/foo", + }, + }, + } + _, err := mutator.ExpandWorkspaceRoot().Apply(context.Background(), bundle) + require.Error(t, err) +} diff --git a/bundle/config/workspace.go b/bundle/config/workspace.go index 66a4e345..90634025 100644 --- a/bundle/config/workspace.go +++ b/bundle/config/workspace.go @@ -47,11 +47,22 @@ type Workspace struct { // This is set after configuration initialization. CurrentUser *scim.User `json:"current_user,omitempty"` - // Remote path for artifacts. - // This can specify a workspace path, a DBFS path, or both. - // Some artifacts must be stored in the workspace (e.g. notebooks). - // Some artifacts must be stored on DBFS (e.g. wheels, JARs). + // Remote 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"` + + // Remote path to synchronize local files to. + // This defaults to "${workspace.root}/files". + FilePath PathLike `json:"file_path"` + + // Remote path for build artifacts. + // This defaults to "${workspace.root}/artifacts". ArtifactPath PathLike `json:"artifact_path"` + + // Remote path for deployment state. + // This defaults to "${workspace.root}/state". + StatePath PathLike `json:"state_path"` } func (w *Workspace) Client() (*databricks.WorkspaceClient, error) { diff --git a/bundle/phases/initialize.go b/bundle/phases/initialize.go index 5b404c85..c31ba843 100644 --- a/bundle/phases/initialize.go +++ b/bundle/phases/initialize.go @@ -15,7 +15,9 @@ func Initialize() bundle.Mutator { "initialize", []bundle.Mutator{ mutator.PopulateCurrentUser(), - mutator.DefaultArtifactPath(), + mutator.DefineDefaultWorkspaceRoot(), + mutator.ExpandWorkspaceRoot(), + mutator.DefineDefaultWorkspacePaths(), mutator.TranslateNotebookPaths(), interpolation.Interpolate( interpolation.IncludeLookupsInPath("bundle"),