Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add command line autocomplete to the fs commands #1622

Merged
merged 43 commits into from
Aug 9, 2024
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
43 commits
Select commit Hold shift + click to select a range
0095d15
Add autocomplete to fs commands
andersrexdb Jul 22, 2024
e8e93fd
Add tests
andersrexdb Jul 24, 2024
300ab07
Lint
andersrexdb Jul 24, 2024
ba64fd3
Fix TestGlobFileset
andersrexdb Jul 24, 2024
5d6a348
Comments
andersrexdb Jul 24, 2024
e3f6a73
Add onlyDirs
andersrexdb Jul 24, 2024
787d563
Add HasWorkspaceClient
andersrexdb Jul 24, 2024
a43b4af
Add file+dir test
andersrexdb Jul 24, 2024
13eb013
PR comments
andersrexdb Jul 25, 2024
3856332
Programmatically add dbfs:/Volumes
andersrexdb Jul 25, 2024
c73c09b
PR feedback
andersrexdb Jul 27, 2024
363bd1d
Client
andersrexdb Jul 29, 2024
0716f31
Comment
andersrexdb Jul 29, 2024
0341e8b
Cleanup
andersrexdb Jul 29, 2024
a42f121
Support .\ local paths with Windows
andersrexdb Aug 2, 2024
6d10776
Handle local Windows paths
andersrexdb Aug 2, 2024
7c010cc
Support both separators on Windows
andersrexdb Aug 2, 2024
e0d2062
Fix test
andersrexdb Aug 2, 2024
d990fe2
Fix test 2
andersrexdb Aug 2, 2024
1868791
PR feedback
andersrexdb Aug 5, 2024
c4a406c
Remove goroutine
andersrexdb Aug 5, 2024
a909d1f
Doc comments
andersrexdb Aug 5, 2024
6091e0d
Use path and filepath for joining paths
andersrexdb Aug 5, 2024
1591896
Use struct for Validate
andersrexdb Aug 5, 2024
84229be
Lowercase validArgs
andersrexdb Aug 6, 2024
058183e
Add integration test
andersrexdb Aug 6, 2024
5df74cf
Fix error
andersrexdb Aug 6, 2024
0803381
DRY up test
andersrexdb Aug 6, 2024
fb90be9
Use fakeFiler in tests
andersrexdb Aug 6, 2024
d802ba7
Remove PrepFakeFiler
andersrexdb Aug 6, 2024
5261a2c
Remove call to current user
andersrexdb Aug 6, 2024
3514151
Add integration test
andersrexdb Aug 6, 2024
2057aad
Put back GetEnvOrSkipTest
andersrexdb Aug 6, 2024
25e4443
PR feedback
andersrexdb Aug 7, 2024
1b168ae
Cleanup
andersrexdb Aug 7, 2024
c2ae3f0
Simplify path.Dir logic
andersrexdb Aug 7, 2024
e5b32fc
Comments
andersrexdb Aug 7, 2024
81775a4
Cleanup
andersrexdb Aug 7, 2024
57ba288
Add windows test
andersrexdb Aug 7, 2024
0eac4e7
PR comments
andersrexdb Aug 9, 2024
6c85d51
Fix TestGlobFileset
andersrexdb Aug 9, 2024
324e4ab
Try again
andersrexdb Aug 9, 2024
354f4b9
Dont use ../filer in test
andersrexdb Aug 9, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions cmd/fs/cat.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,5 +30,7 @@ func newCatCommand() *cobra.Command {
return cmdio.Render(ctx, r)
}

cmd.ValidArgsFunction = getValidArgsFunction(1, false, filerForPath)

return cmd
}
3 changes: 3 additions & 0 deletions cmd/fs/cp.go
Original file line number Diff line number Diff line change
Expand Up @@ -200,5 +200,8 @@ func newCpCommand() *cobra.Command {
return c.cpFileToFile(sourcePath, targetPath)
}

// The copy command has two paths that can be completed (SOURCE_PATH & TARGET_PATH)
cmd.ValidArgsFunction = getValidArgsFunction(2, false, filerForPath)

return cmd
}
63 changes: 62 additions & 1 deletion cmd/fs/helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,9 @@ import (
"strings"

"github.com/databricks/cli/cmd/root"
"github.com/databricks/cli/libs/completer"
"github.com/databricks/cli/libs/filer"
"github.com/spf13/cobra"
)

func filerForPath(ctx context.Context, fullPath string) (filer.Filer, string, error) {
Expand Down Expand Up @@ -46,6 +48,65 @@ func filerForPath(ctx context.Context, fullPath string) (filer.Filer, string, er
return f, path, err
}

const dbfsPrefix string = "dbfs:/"

func isDbfsPath(path string) bool {
return strings.HasPrefix(path, "dbfs:/")
return strings.HasPrefix(path, dbfsPrefix)
}

func getValidArgsFunction(pathArgCount int, onlyDirs bool, filerForPathFunc func(ctx context.Context, fullPath string) (filer.Filer, string, error)) func(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) {
return func(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) {
andersrexdb marked this conversation as resolved.
Show resolved Hide resolved
cmd.SetContext(root.SkipPrompt(cmd.Context()))

err := mustWorkspaceClient(cmd, args) // TODO: Fix this
andersrexdb marked this conversation as resolved.
Show resolved Hide resolved
if err != nil {
return nil, cobra.ShellCompDirectiveError
}

isValidPrefix := isDbfsPath(toComplete)

if !isValidPrefix {
andersrexdb marked this conversation as resolved.
Show resolved Hide resolved
return []string{dbfsPrefix}, cobra.ShellCompDirectiveNoSpace
}

filer, path, err := filerForPathFunc(cmd.Context(), toComplete)
andersrexdb marked this conversation as resolved.
Show resolved Hide resolved
andersrexdb marked this conversation as resolved.
Show resolved Hide resolved
if err != nil {
return nil, cobra.ShellCompDirectiveError
}

wsc := root.WorkspaceClient(cmd.Context())
_, err = wsc.CurrentUser.Me(cmd.Context())
andersrexdb marked this conversation as resolved.
Show resolved Hide resolved
if err != nil {
return nil, cobra.ShellCompDirectiveError
}

completer := completer.NewCompleter(cmd.Context(), filer, onlyDirs)

if len(args) < pathArgCount {
andersrexdb marked this conversation as resolved.
Show resolved Hide resolved
completions, directive := completer.CompleteRemotePath(path)

// The completions will start with a "/", so we'll prefix
// them with the dbfsPrefix without a trailing "/" (dbfs:)
prefix := dbfsPrefix[:len(dbfsPrefix)-1]

// Add the prefix to the completions
for i, completion := range completions {
completions[i] = fmt.Sprintf("%s%s", prefix, completion)
}

return completions, directive
} else {
return nil, cobra.ShellCompDirectiveNoFileComp
}
}
}

// Wrapper for [root.MustWorkspaceClient] that disables loading authentication configuration from a bundle.
func mustWorkspaceClient(cmd *cobra.Command, args []string) error {
if root.HasWorkspaceClient(cmd.Context()) {
return nil
}
andersrexdb marked this conversation as resolved.
Show resolved Hide resolved

cmd.SetContext(root.SkipLoadBundle(cmd.Context()))
return root.MustWorkspaceClient(cmd, args)
}
97 changes: 97 additions & 0 deletions cmd/fs/helpers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,19 @@ package fs

import (
"context"
"errors"
"io/fs"
"runtime"
"testing"

"github.com/databricks/cli/cmd/root"
"github.com/databricks/cli/internal/testutil"
"github.com/databricks/cli/libs/filer"
"github.com/databricks/databricks-sdk-go/experimental/mocks"
"github.com/databricks/databricks-sdk-go/service/iam"
"github.com/spf13/cobra"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/mock"
"github.com/stretchr/testify/require"
)

Expand Down Expand Up @@ -60,3 +68,92 @@ func TestFilerForWindowsLocalPaths(t *testing.T) {
testWindowsFilerForPath(t, ctx, `d:\abc`)
testWindowsFilerForPath(t, ctx, `f:\abc\ef`)
}

func mockCurrentUserApi(m *mocks.MockWorkspaceClient, u *iam.User, e error) {
currentUserApi := m.GetMockCurrentUserAPI()
currentUserApi.EXPECT().Me(mock.AnythingOfType("*context.valueCtx")).Return(u, e)
}

func setupValidArgsFunctionTest(t *testing.T) (*mocks.MockWorkspaceClient, *cobra.Command) {
m := mocks.NewMockWorkspaceClient(t)
ctx := context.Background()
ctx = root.SetWorkspaceClient(ctx, m.WorkspaceClient)

cmd := &cobra.Command{}
cmd.SetContext(ctx)

return m, cmd
}

func TestGetValidArgsFunctionCompletion(t *testing.T) {
andersrexdb marked this conversation as resolved.
Show resolved Hide resolved
m, cmd := setupValidArgsFunctionTest(t)

mockCurrentUserApi(m, nil, nil)

mockFilerForPath := testutil.GetMockFilerForPath(t, []fs.DirEntry{
testutil.NewFakeDirEntry("dir", true),
testutil.NewFakeDirEntry("file", false),
})

validArgsFunction := getValidArgsFunction(1, false, mockFilerForPath)
completions, directive := validArgsFunction(cmd, []string{}, "dbfs:/")

assert.Equal(t, []string{"dbfs:/dir", "dbfs:/file"}, completions)
assert.Equal(t, cobra.ShellCompDirectiveNoSpace, directive)
}

func TestGetValidArgsFunctionCompletionOnlyDirs(t *testing.T) {
andersrexdb marked this conversation as resolved.
Show resolved Hide resolved
m, cmd := setupValidArgsFunctionTest(t)

mockCurrentUserApi(m, nil, nil)

mockFilerForPath := testutil.GetMockFilerForPath(t, []fs.DirEntry{
testutil.NewFakeDirEntry("dir", true),
testutil.NewFakeDirEntry("file", false),
})

validArgsFunction := getValidArgsFunction(1, true, mockFilerForPath)
completions, directive := validArgsFunction(cmd, []string{}, "dbfs:/")

assert.Equal(t, []string{"dbfs:/dir"}, completions)
assert.Equal(t, cobra.ShellCompDirectiveNoSpace, directive)
}

func TestGetValidArgsFunctionNoDbfsPath(t *testing.T) {
_, cmd := setupValidArgsFunctionTest(t)

mockFilerForPath := testutil.GetMockFilerForPath(t, nil)

validArgsFunction := getValidArgsFunction(1, true, mockFilerForPath)
completions, directive := validArgsFunction(cmd, []string{}, "/")

assert.Equal(t, []string{"dbfs:/"}, completions)
assert.Equal(t, cobra.ShellCompDirectiveNoSpace, directive)
}

func TestGetValidArgsFunctionNoCurrentUser(t *testing.T) {
m, cmd := setupValidArgsFunctionTest(t)

mockCurrentUserApi(m, nil, errors.New("Current User Error"))
mockFilerForPath := testutil.GetMockFilerForPath(t, nil)

validArgsFunction := getValidArgsFunction(1, true, mockFilerForPath)
completions, directive := validArgsFunction(cmd, []string{}, "dbfs:/")

assert.Nil(t, completions)
assert.Equal(t, cobra.ShellCompDirectiveError, directive)
}

func TestGetValidArgsFunctionNotCompletedArgument(t *testing.T) {
m, cmd := setupValidArgsFunctionTest(t)

mockCurrentUserApi(m, nil, nil)
mockFilerForPath := testutil.GetMockFilerForPath(t, nil)

// 0 here means we don't complete any arguments
validArgsFunction := getValidArgsFunction(0, true, mockFilerForPath)
completions, directive := validArgsFunction(cmd, []string{}, "dbfs:/")

assert.Nil(t, completions)
assert.Equal(t, cobra.ShellCompDirectiveNoFileComp, directive)
}
2 changes: 2 additions & 0 deletions cmd/fs/ls.go
Original file line number Diff line number Diff line change
Expand Up @@ -89,5 +89,7 @@ func newLsCommand() *cobra.Command {
`))
}

cmd.ValidArgsFunction = getValidArgsFunction(1, true, filerForPath)

return cmd
}
2 changes: 2 additions & 0 deletions cmd/fs/mkdir.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,5 +28,7 @@ func newMkdirCommand() *cobra.Command {
return f.Mkdir(ctx, path)
}

cmd.ValidArgsFunction = getValidArgsFunction(1, true, filerForPath)

return cmd
}
2 changes: 2 additions & 0 deletions cmd/fs/rm.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,5 +32,7 @@ func newRmCommand() *cobra.Command {
return f.Delete(ctx, path)
}

cmd.ValidArgsFunction = getValidArgsFunction(1, false, filerForPath)

return cmd
}
6 changes: 6 additions & 0 deletions cmd/root/auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,7 @@ func MustAccountClient(cmd *cobra.Command, args []string) error {
// Helper function to create a workspace client or prompt once if the given configuration is not valid.
func workspaceClientOrPrompt(ctx context.Context, cfg *config.Config, allowPrompt bool) (*databricks.WorkspaceClient, error) {
w, err := databricks.NewWorkspaceClient((*databricks.Config)(cfg))

if err == nil {
err = w.Config.Authenticate(emptyHttpRequest(ctx))
}
Expand Down Expand Up @@ -230,6 +231,11 @@ func SetWorkspaceClient(ctx context.Context, w *databricks.WorkspaceClient) cont
return context.WithValue(ctx, &workspaceClient, w)
}

func HasWorkspaceClient(ctx context.Context) bool {
andersrexdb marked this conversation as resolved.
Show resolved Hide resolved
_, ok := ctx.Value(&workspaceClient).(*databricks.WorkspaceClient)
return ok
}

func SetAccountClient(ctx context.Context, a *databricks.AccountClient) context.Context {
return context.WithValue(ctx, &accountClient, a)
}
Expand Down
6 changes: 3 additions & 3 deletions cmd/root/auth_options.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import (

type skipPrompt int

var skipPromptKey skipPrompt
var SkipPromptKey skipPrompt

// SkipPrompt allows to skip prompt for profile configuration in MustWorkspaceClient.
//
Expand All @@ -16,12 +16,12 @@ var skipPromptKey skipPrompt
// thus the Context object seems to be the only viable option for us to configure prompt behaviour based on
// the context it's executed from.
func SkipPrompt(ctx context.Context) context.Context {
return context.WithValue(ctx, skipPromptKey, true)
return context.WithValue(ctx, SkipPromptKey, true)
}

// shouldSkipPrompt returns whether or not [SkipPrompt] has been set on the specified context.
func shouldSkipPrompt(ctx context.Context) bool {
skipPrompt, ok := ctx.Value(skipPromptKey).(bool)
skipPrompt, ok := ctx.Value(SkipPromptKey).(bool)
return ok && skipPrompt
}

Expand Down
74 changes: 74 additions & 0 deletions internal/testutil/filer.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
package testutil

import (
"context"
"io/fs"
"testing"
"time"

mockfiler "github.com/databricks/cli/internal/mocks/libs/filer"
"github.com/databricks/cli/libs/filer"
"github.com/stretchr/testify/mock"
)

type fakeDirEntry struct {
fakeFileInfo
}

func (entry fakeDirEntry) Type() fs.FileMode {
typ := fs.ModePerm
if entry.dir {
typ |= fs.ModeDir
}
return typ
}

func (entry fakeDirEntry) Info() (fs.FileInfo, error) {
return entry.fakeFileInfo, nil
}

type fakeFileInfo struct {
name string
size int64
dir bool
mode fs.FileMode
}

func (info fakeFileInfo) Name() string {
return info.name
}

func (info fakeFileInfo) Size() int64 {
return info.size
}

func (info fakeFileInfo) Mode() fs.FileMode {
return info.mode
}

func (info fakeFileInfo) ModTime() time.Time {
return time.Now()
}

func (info fakeFileInfo) IsDir() bool {
return info.dir
}

func (info fakeFileInfo) Sys() any {
return nil
}

func NewFakeDirEntry(name string, dir bool) fs.DirEntry {
return fakeDirEntry{fakeFileInfo{name: name, dir: dir}}
}

func GetMockFilerForPath(t *testing.T, entries []fs.DirEntry) func(ctx context.Context, fullPath string) (filer.Filer, string, error) {
mockFiler := mockfiler.NewMockFiler(t)
if entries != nil {
mockFiler.EXPECT().ReadDir(mock.AnythingOfType("*context.valueCtx"), mock.Anything).Return(entries, nil)
}
mockFilerForPath := func(ctx context.Context, fullPath string) (filer.Filer, string, error) {
return mockFiler, "/", nil
}
return mockFilerForPath
}
Loading
Loading