Do not fail on git operations in bundle and sync, log warnings instead; add more tests.

- Change GitRepositoryInfo.WorktreeRoot to string
- Restore original warnings logs for when branch and commit could not be read
- Add new logs for when repository could not be read. However, if we can figure out git root then use it, even if parsing later fails.
This commit is contained in:
Denis Bilenko 2024-12-02 22:55:29 +01:00
parent d204a9380a
commit 8fe958f67b
5 changed files with 153 additions and 33 deletions

View File

@ -7,6 +7,7 @@ import (
"github.com/databricks/cli/bundle" "github.com/databricks/cli/bundle"
"github.com/databricks/cli/libs/diag" "github.com/databricks/cli/libs/diag"
"github.com/databricks/cli/libs/git" "github.com/databricks/cli/libs/git"
"github.com/databricks/cli/libs/vfs"
) )
type loadGitDetails struct{} type loadGitDetails struct{}
@ -20,12 +21,17 @@ func (m *loadGitDetails) Name() string {
} }
func (m *loadGitDetails) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { func (m *loadGitDetails) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics {
var diags diag.Diagnostics
info, err := git.FetchRepositoryInfo(ctx, b.BundleRoot, b.WorkspaceClient()) info, err := git.FetchRepositoryInfo(ctx, b.BundleRoot, b.WorkspaceClient())
if err != nil { if err != nil {
return diag.FromErr(err) diags = append(diags, diag.WarningFromErr(err)...)
} }
b.WorktreeRoot = info.WorktreeRoot if info.WorktreeRoot == "" {
b.WorktreeRoot = b.BundleRoot
} else {
b.WorktreeRoot = vfs.MustNew(info.WorktreeRoot)
}
b.Config.Bundle.Git.ActualBranch = info.CurrentBranch b.Config.Bundle.Git.ActualBranch = info.CurrentBranch
if b.Config.Bundle.Git.Branch == "" { if b.Config.Bundle.Git.Branch == "" {
@ -44,11 +50,11 @@ func (m *loadGitDetails) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagn
b.Config.Bundle.Git.OriginURL = info.OriginURL b.Config.Bundle.Git.OriginURL = info.OriginURL
} }
relBundlePath, err := filepath.Rel(info.WorktreeRoot.Native(), b.BundleRoot.Native()) relBundlePath, err := filepath.Rel(b.WorktreeRoot.Native(), b.BundleRoot.Native())
if err != nil { if err != nil {
return diag.FromErr(err) diags = append(diags, diag.FromErr(err)...)
} }
b.Config.Bundle.Git.BundleRootPath = filepath.ToSlash(relBundlePath) b.Config.Bundle.Git.BundleRootPath = filepath.ToSlash(relBundlePath)
return nil return diags
} }

View File

@ -13,6 +13,7 @@ import (
"github.com/databricks/cli/cmd/root" "github.com/databricks/cli/cmd/root"
"github.com/databricks/cli/libs/flags" "github.com/databricks/cli/libs/flags"
"github.com/databricks/cli/libs/git" "github.com/databricks/cli/libs/git"
"github.com/databricks/cli/libs/log"
"github.com/databricks/cli/libs/sync" "github.com/databricks/cli/libs/sync"
"github.com/databricks/cli/libs/vfs" "github.com/databricks/cli/libs/vfs"
"github.com/spf13/cobra" "github.com/spf13/cobra"
@ -67,12 +68,17 @@ func (f *syncFlags) syncOptionsFromArgs(cmd *cobra.Command, args []string) (*syn
localRoot := vfs.MustNew(args[0]) localRoot := vfs.MustNew(args[0])
info, err := git.FetchRepositoryInfo(ctx, localRoot, client) info, err := git.FetchRepositoryInfo(ctx, localRoot, client)
if err != nil { if err != nil {
return nil, err log.Warnf(ctx, "Failed to read git info: %s", err)
}
if info.WorktreeRoot == "" {
info.WorktreeRoot = localRoot.Native()
} }
opts := sync.SyncOptions{ opts := sync.SyncOptions{
WorktreeRoot: info.WorktreeRoot, WorktreeRoot: vfs.MustNew(info.WorktreeRoot),
LocalRoot: localRoot, LocalRoot: localRoot,
Paths: []string{"."}, Paths: []string{"."},
Include: nil, Include: nil,

View File

@ -1,6 +1,7 @@
package internal package internal
import ( import (
"os"
"os/exec" "os/exec"
"path/filepath" "path/filepath"
"testing" "testing"
@ -16,14 +17,14 @@ import (
const examplesRepoUrl = "https://github.com/databricks/bundle-examples" const examplesRepoUrl = "https://github.com/databricks/bundle-examples"
const examplesRepoProvider = "gitHub" const examplesRepoProvider = "gitHub"
func assertGitInfo(t *testing.T, info git.GitRepositoryInfo, expectedRoot string) { func assertFullGitInfo(t *testing.T, info git.GitRepositoryInfo, expectedRoot string) {
assert.Equal(t, "main", info.CurrentBranch) assert.Equal(t, "main", info.CurrentBranch)
assert.NotEmpty(t, info.LatestCommit) assert.NotEmpty(t, info.LatestCommit)
assert.Equal(t, examplesRepoUrl, info.OriginURL) assert.Equal(t, examplesRepoUrl, info.OriginURL)
assert.Equal(t, expectedRoot, info.WorktreeRoot.Native()) assert.Equal(t, expectedRoot, info.WorktreeRoot)
} }
func TestAccFetchRepositoryInfoAPI(t *testing.T) { func TestAccFetchRepositoryInfoAPI_FromRepo(t *testing.T) {
ctx, wt := acc.WorkspaceTest(t) ctx, wt := acc.WorkspaceTest(t)
me, err := wt.W.CurrentUser.Me(ctx) me, err := wt.W.CurrentUser.Me(ctx)
require.NoError(t, err) require.NoError(t, err)
@ -45,12 +46,62 @@ func TestAccFetchRepositoryInfoAPI(t *testing.T) {
t.Run(inputPath, func(t *testing.T) { t.Run(inputPath, func(t *testing.T) {
info, err := git.FetchRepositoryInfo(ctx, vfs.MustNew(inputPath), wt.W) info, err := git.FetchRepositoryInfo(ctx, vfs.MustNew(inputPath), wt.W)
assert.NoError(t, err) assert.NoError(t, err)
assertGitInfo(t, info, targetPath) assertFullGitInfo(t, info, targetPath)
}) })
} }
} }
func TestAccFetchRepositoryInfoDotGit(t *testing.T) { func TestAccFetchRepositoryInfoAPI_FromNonRepo(t *testing.T) {
ctx, wt := acc.WorkspaceTest(t)
me, err := wt.W.CurrentUser.Me(ctx)
require.NoError(t, err)
rootPath := acc.RandomName("/Workspace/Users/" + me.UserName + "/testing-nonrepo-")
_, stderr := RequireSuccessfulRun(t, "workspace", "mkdirs", rootPath+"/a/b/c")
t.Cleanup(func() {
RequireSuccessfulRun(t, "workspace", "delete", "--recursive", rootPath)
})
assert.Empty(t, stderr.String())
//assert.NotEmpty(t, stdout.String())
ctx = dbr.MockRuntime(ctx, true)
tests := []struct {
input string
msg string
}{
{
input: rootPath + "/a/b/c",
msg: "",
},
{
input: rootPath,
msg: "",
},
{
input: rootPath + "/non-existent",
msg: "doesn't exist",
},
}
for _, test := range tests {
t.Run(test.input+" <==> "+test.msg, func(t *testing.T) {
info, err := git.FetchRepositoryInfo(ctx, vfs.MustNew(test.input), wt.W)
if test.msg == "" {
assert.NoError(t, err)
} else {
assert.Error(t, err)
assert.Contains(t, err.Error(), test.msg)
}
assert.Equal(t, "", info.CurrentBranch)
assert.Equal(t, "", info.LatestCommit)
assert.Equal(t, "", info.OriginURL)
assert.Equal(t, "", info.WorktreeRoot)
})
}
}
func TestAccFetchRepositoryInfoDotGit_FromGitRepo(t *testing.T) {
ctx, wt := acc.WorkspaceTest(t) ctx, wt := acc.WorkspaceTest(t)
repo := cloneRepoLocally(t, examplesRepoUrl) repo := cloneRepoLocally(t, examplesRepoUrl)
@ -62,7 +113,7 @@ func TestAccFetchRepositoryInfoDotGit(t *testing.T) {
t.Run(inputPath, func(t *testing.T) { t.Run(inputPath, func(t *testing.T) {
info, err := git.FetchRepositoryInfo(ctx, vfs.MustNew(inputPath), wt.W) info, err := git.FetchRepositoryInfo(ctx, vfs.MustNew(inputPath), wt.W)
assert.NoError(t, err) assert.NoError(t, err)
assertGitInfo(t, info, repo) assertFullGitInfo(t, info, repo)
}) })
} }
} }
@ -76,3 +127,45 @@ func cloneRepoLocally(t *testing.T, repoUrl string) string {
require.NoError(t, err) require.NoError(t, err)
return localRoot return localRoot
} }
func TestAccFetchRepositoryInfoDotGit_FromNonGitRepo(t *testing.T) {
ctx, wt := acc.WorkspaceTest(t)
tempDir := t.TempDir()
root := filepath.Join(tempDir, "repo")
require.NoError(t, os.MkdirAll(root+"/a/b/c", 0700))
tests := []string{
root + "/a/b/c",
root,
root + "/non-existent",
}
for _, input := range tests {
t.Run(input, func(t *testing.T) {
info, err := git.FetchRepositoryInfo(ctx, vfs.MustNew(input), wt.W)
assert.NoError(t, err)
assert.Equal(t, "", info.CurrentBranch)
assert.Equal(t, "", info.LatestCommit)
assert.Equal(t, "", info.OriginURL)
assert.Equal(t, "", info.WorktreeRoot)
})
}
}
func TestAccFetchRepositoryInfoDotGit_FromBrokenGitRepo(t *testing.T) {
ctx, wt := acc.WorkspaceTest(t)
tempDir := t.TempDir()
root := filepath.Join(tempDir, "repo")
path := root + "/a/b/c"
require.NoError(t, os.MkdirAll(path, 0700))
require.NoError(t, os.WriteFile(root+"/.git", []byte(""), 0000))
info, err := git.FetchRepositoryInfo(ctx, vfs.MustNew(path), wt.W)
assert.NoError(t, err)
assert.Equal(t, root, info.WorktreeRoot)
assert.Equal(t, "", info.CurrentBranch)
assert.Equal(t, "", info.LatestCommit)
assert.Equal(t, "", info.OriginURL)
}

View File

@ -53,6 +53,19 @@ func FromErr(err error) Diagnostics {
} }
} }
// FromErr returns a new warning diagnostic from the specified error, if any.
func WarningFromErr(err error) Diagnostics {
if err == nil {
return nil
}
return []Diagnostic{
{
Severity: Warning,
Summary: err.Error(),
},
}
}
// Warningf creates a new warning diagnostic. // Warningf creates a new warning diagnostic.
func Warningf(format string, args ...any) Diagnostics { func Warningf(format string, args ...any) Diagnostics {
return []Diagnostic{ return []Diagnostic{

View File

@ -19,7 +19,7 @@ type GitRepositoryInfo struct {
OriginURL string OriginURL string
LatestCommit string LatestCommit string
CurrentBranch string CurrentBranch string
WorktreeRoot vfs.Path WorktreeRoot string
} }
type gitInfo struct { type gitInfo struct {
@ -74,14 +74,12 @@ func fetchRepositoryInfoAPI(ctx context.Context, path vfs.Path, w *databricks.Wo
OriginURL: gi.URL, OriginURL: gi.URL,
LatestCommit: gi.HeadCommitID, LatestCommit: gi.HeadCommitID,
CurrentBranch: gi.Branch, CurrentBranch: gi.Branch,
WorktreeRoot: vfs.MustNew(fixedPath), WorktreeRoot: fixedPath,
}, nil }, nil
} }
log.Warnf(ctx, "Failed to load git info from %s", apiEndpoint) log.Warnf(ctx, "Failed to load git info from %s", apiEndpoint)
return GitRepositoryInfo{ return GitRepositoryInfo{}, nil
WorktreeRoot: path,
}, nil
} }
func ensureWorkspacePrefix(p string) string { func ensureWorkspacePrefix(p string) string {
@ -93,32 +91,36 @@ func ensureWorkspacePrefix(p string) string {
func fetchRepositoryInfoDotGit(ctx context.Context, path vfs.Path) (GitRepositoryInfo, error) { func fetchRepositoryInfoDotGit(ctx context.Context, path vfs.Path) (GitRepositoryInfo, error) {
rootDir, err := vfs.FindLeafInTree(path, GitDirectoryName) rootDir, err := vfs.FindLeafInTree(path, GitDirectoryName)
if err != nil { if err != nil || rootDir == nil {
if !errors.Is(err, fs.ErrNotExist) { if errors.Is(err, fs.ErrNotExist) {
return GitRepositoryInfo{}, nil
}
return GitRepositoryInfo{}, err return GitRepositoryInfo{}, err
} }
rootDir = path
result := GitRepositoryInfo{
WorktreeRoot: rootDir.Native(),
} }
repo, err := NewRepository(rootDir) repo, err := NewRepository(rootDir)
if err != nil { if err != nil {
return GitRepositoryInfo{}, err log.Warnf(ctx, "failed to read .git: %s", err)
// return early since operations below won't work
return result, nil
} }
branch, err := repo.CurrentBranch() result.OriginURL = repo.OriginUrl()
result.CurrentBranch, err = repo.CurrentBranch()
if err != nil { if err != nil {
return GitRepositoryInfo{}, nil log.Warnf(ctx, "failed to load current branch: %s", err)
} }
commit, err := repo.LatestCommit() result.LatestCommit, err = repo.LatestCommit()
if err != nil { if err != nil {
return GitRepositoryInfo{}, nil log.Warnf(ctx, "failed to load latest commit: %s", err)
} }
return GitRepositoryInfo{ return result, nil
OriginURL: repo.OriginUrl(),
LatestCommit: commit,
CurrentBranch: branch,
WorktreeRoot: rootDir,
}, nil
} }