Skip to content

Commit

Permalink
Properly read Git metadata when running inside workspace (#1945)
Browse files Browse the repository at this point in the history
## Changes

Since there is no .git directory in Workspace file system, we need to
make an API call to api/2.0/workspace/get-status?return_git_info=true to
fetch git the root of the repo, current branch, commit and origin.

Added new function FetchRepositoryInfo that either looks up and parses
.git or calls remote API depending on env.

Refactor Repository/View/FileSet to accept repository root rather than
calculate it. This helps because:
- Repository is currently created in multiple places and finding the
repository root is becoming relatively expensive (API call needed).
- Repository/FileSet/View do not have access to current Bundle which is
where WorkplaceClient is stored.

## Tests

- Tested manually by running "bundle validate --json" inside web
terminal within Databricks env.
- Added integration tests for the new API.

---------

Co-authored-by: Andrew Nester <[email protected]>
Co-authored-by: Pieter Noordhuis <[email protected]>
  • Loading branch information
3 people authored Dec 5, 2024
1 parent 0a36681 commit 0ad790e
Show file tree
Hide file tree
Showing 17 changed files with 471 additions and 89 deletions.
4 changes: 4 additions & 0 deletions bundle/bundle.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,10 @@ type Bundle struct {
// Exclusively use this field for filesystem operations.
SyncRoot vfs.Path

// Path to the root of git worktree containing the bundle.
// https://git-scm.com/docs/git-worktree
WorktreeRoot vfs.Path

// Config contains the bundle configuration.
// It is loaded from the bundle configuration files and mutators may update it.
Config config.Root
Expand Down
4 changes: 4 additions & 0 deletions bundle/bundle_read_only.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,10 @@ func (r ReadOnlyBundle) SyncRoot() vfs.Path {
return r.b.SyncRoot
}

func (r ReadOnlyBundle) WorktreeRoot() vfs.Path {
return r.b.WorktreeRoot
}

func (r ReadOnlyBundle) WorkspaceClient() *databricks.WorkspaceClient {
return r.b.WorkspaceClient()
}
Expand Down
49 changes: 22 additions & 27 deletions bundle/config/mutator/load_git_details.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import (
"github.com/databricks/cli/bundle"
"github.com/databricks/cli/libs/diag"
"github.com/databricks/cli/libs/git"
"github.com/databricks/cli/libs/log"
"github.com/databricks/cli/libs/vfs"
)

type loadGitDetails struct{}
Expand All @@ -21,45 +21,40 @@ func (m *loadGitDetails) Name() string {
}

func (m *loadGitDetails) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics {
// Load relevant git repository
repo, err := git.NewRepository(b.BundleRoot)
var diags diag.Diagnostics
info, err := git.FetchRepositoryInfo(ctx, b.BundleRoot.Native(), b.WorkspaceClient())
if err != nil {
return diag.FromErr(err)
diags = append(diags, diag.WarningFromErr(err)...)
}

// Read branch name of current checkout
branch, err := repo.CurrentBranch()
if err == nil {
b.Config.Bundle.Git.ActualBranch = branch
if b.Config.Bundle.Git.Branch == "" {
// Only load branch if there's no user defined value
b.Config.Bundle.Git.Inferred = true
b.Config.Bundle.Git.Branch = branch
}
if info.WorktreeRoot == "" {
b.WorktreeRoot = b.BundleRoot
} else {
log.Warnf(ctx, "failed to load current branch: %s", err)
b.WorktreeRoot = vfs.MustNew(info.WorktreeRoot)
}

b.Config.Bundle.Git.ActualBranch = info.CurrentBranch
if b.Config.Bundle.Git.Branch == "" {
// Only load branch if there's no user defined value
b.Config.Bundle.Git.Inferred = true
b.Config.Bundle.Git.Branch = info.CurrentBranch
}

// load commit hash if undefined
if b.Config.Bundle.Git.Commit == "" {
commit, err := repo.LatestCommit()
if err != nil {
log.Warnf(ctx, "failed to load latest commit: %s", err)
} else {
b.Config.Bundle.Git.Commit = commit
}
b.Config.Bundle.Git.Commit = info.LatestCommit
}

// load origin url if undefined
if b.Config.Bundle.Git.OriginURL == "" {
remoteUrl := repo.OriginUrl()
b.Config.Bundle.Git.OriginURL = remoteUrl
b.Config.Bundle.Git.OriginURL = info.OriginURL
}

// repo.Root() returns the absolute path of the repo
relBundlePath, err := filepath.Rel(repo.Root(), b.BundleRoot.Native())
relBundlePath, err := filepath.Rel(b.WorktreeRoot.Native(), b.BundleRoot.Native())
if err != nil {
return diag.FromErr(err)
diags = append(diags, diag.FromErr(err)...)
} else {
b.Config.Bundle.Git.BundleRootPath = filepath.ToSlash(relBundlePath)
}
b.Config.Bundle.Git.BundleRootPath = filepath.ToSlash(relBundlePath)
return nil
return diags
}
1 change: 1 addition & 0 deletions bundle/config/validate/files_to_sync_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ func setupBundleForFilesToSyncTest(t *testing.T) *bundle.Bundle {
BundleRoot: vfs.MustNew(dir),
SyncRootPath: dir,
SyncRoot: vfs.MustNew(dir),
WorktreeRoot: vfs.MustNew(dir),
Config: config.Root{
Bundle: config.Bundle{
Target: "default",
Expand Down
9 changes: 5 additions & 4 deletions bundle/deploy/files/sync.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,11 @@ func GetSyncOptions(ctx context.Context, rb bundle.ReadOnlyBundle) (*sync.SyncOp
}

opts := &sync.SyncOptions{
LocalRoot: rb.SyncRoot(),
Paths: rb.Config().Sync.Paths,
Include: includes,
Exclude: rb.Config().Sync.Exclude,
WorktreeRoot: rb.WorktreeRoot(),
LocalRoot: rb.SyncRoot(),
Paths: rb.Config().Sync.Paths,
Include: includes,
Exclude: rb.Config().Sync.Exclude,

RemotePath: rb.Config().Workspace.FilePath,
Host: rb.WorkspaceClient().Config.Host,
Expand Down
32 changes: 27 additions & 5 deletions cmd/sync/sync.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ import (
"github.com/databricks/cli/bundle/deploy/files"
"github.com/databricks/cli/cmd/root"
"github.com/databricks/cli/libs/flags"
"github.com/databricks/cli/libs/git"
"github.com/databricks/cli/libs/log"
"github.com/databricks/cli/libs/sync"
"github.com/databricks/cli/libs/vfs"
"github.com/spf13/cobra"
Expand All @@ -37,6 +39,7 @@ func (f *syncFlags) syncOptionsFromBundle(cmd *cobra.Command, args []string, b *

opts.Full = f.full
opts.PollInterval = f.interval
opts.WorktreeRoot = b.WorktreeRoot
return opts, nil
}

Expand All @@ -60,11 +63,30 @@ func (f *syncFlags) syncOptionsFromArgs(cmd *cobra.Command, args []string) (*syn
}
}

ctx := cmd.Context()
client := root.WorkspaceClient(ctx)

localRoot := vfs.MustNew(args[0])
info, err := git.FetchRepositoryInfo(ctx, localRoot.Native(), client)

if err != nil {
log.Warnf(ctx, "Failed to read git info: %s", err)
}

var worktreeRoot vfs.Path

if info.WorktreeRoot == "" {
worktreeRoot = localRoot
} else {
worktreeRoot = vfs.MustNew(info.WorktreeRoot)
}

opts := sync.SyncOptions{
LocalRoot: vfs.MustNew(args[0]),
Paths: []string{"."},
Include: nil,
Exclude: nil,
WorktreeRoot: worktreeRoot,
LocalRoot: localRoot,
Paths: []string{"."},
Include: nil,
Exclude: nil,

RemotePath: args[1],
Full: f.full,
Expand All @@ -75,7 +97,7 @@ func (f *syncFlags) syncOptionsFromArgs(cmd *cobra.Command, args []string) (*syn
// The sync code will automatically create this directory if it doesn't
// exist and add it to the `.gitignore` file in the root.
SnapshotBasePath: filepath.Join(args[0], ".databricks"),
WorkspaceClient: root.WorkspaceClient(cmd.Context()),
WorkspaceClient: client,

OutputHandler: outputHandler,
}
Expand Down
172 changes: 172 additions & 0 deletions internal/git_fetch_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,172 @@
package internal

import (
"os"
"os/exec"
"path"
"path/filepath"
"testing"

"github.com/databricks/cli/internal/acc"
"github.com/databricks/cli/libs/dbr"
"github.com/databricks/cli/libs/git"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

const examplesRepoUrl = "https://github.com/databricks/bundle-examples"
const examplesRepoProvider = "gitHub"

func assertFullGitInfo(t *testing.T, expectedRoot string, info git.RepositoryInfo) {
assert.Equal(t, "main", info.CurrentBranch)
assert.NotEmpty(t, info.LatestCommit)
assert.Equal(t, examplesRepoUrl, info.OriginURL)
assert.Equal(t, expectedRoot, info.WorktreeRoot)
}

func assertEmptyGitInfo(t *testing.T, info git.RepositoryInfo) {
assertSparseGitInfo(t, "", info)
}

func assertSparseGitInfo(t *testing.T, expectedRoot string, info git.RepositoryInfo) {
assert.Equal(t, "", info.CurrentBranch)
assert.Equal(t, "", info.LatestCommit)
assert.Equal(t, "", info.OriginURL)
assert.Equal(t, expectedRoot, info.WorktreeRoot)
}

func TestAccFetchRepositoryInfoAPI_FromRepo(t *testing.T) {
ctx, wt := acc.WorkspaceTest(t)
me, err := wt.W.CurrentUser.Me(ctx)
require.NoError(t, err)

targetPath := acc.RandomName(path.Join("/Workspace/Users", me.UserName, "/testing-clone-bundle-examples-"))
stdout, stderr := RequireSuccessfulRun(t, "repos", "create", examplesRepoUrl, examplesRepoProvider, "--path", targetPath)
t.Cleanup(func() {
RequireSuccessfulRun(t, "repos", "delete", targetPath)
})

assert.Empty(t, stderr.String())
assert.NotEmpty(t, stdout.String())
ctx = dbr.MockRuntime(ctx, true)

for _, inputPath := range []string{
path.Join(targetPath, "knowledge_base/dashboard_nyc_taxi"),
targetPath,
} {
t.Run(inputPath, func(t *testing.T) {
info, err := git.FetchRepositoryInfo(ctx, inputPath, wt.W)
assert.NoError(t, err)
assertFullGitInfo(t, targetPath, info)
})
}
}

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(path.Join("/Workspace/Users", me.UserName, "testing-nonrepo-"))
_, stderr := RequireSuccessfulRun(t, "workspace", "mkdirs", path.Join(rootPath, "a/b/c"))
t.Cleanup(func() {
RequireSuccessfulRun(t, "workspace", "delete", "--recursive", rootPath)
})

assert.Empty(t, stderr.String())
ctx = dbr.MockRuntime(ctx, true)

tests := []struct {
input string
msg string
}{
{
input: path.Join(rootPath, "a/b/c"),
msg: "",
},
{
input: rootPath,
msg: "",
},
{
input: path.Join(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, test.input, wt.W)
if test.msg == "" {
assert.NoError(t, err)
} else {
assert.Error(t, err)
assert.Contains(t, err.Error(), test.msg)
}
assertEmptyGitInfo(t, info)
})
}
}

func TestAccFetchRepositoryInfoDotGit_FromGitRepo(t *testing.T) {
ctx, wt := acc.WorkspaceTest(t)

repo := cloneRepoLocally(t, examplesRepoUrl)

for _, inputPath := range []string{
filepath.Join(repo, "knowledge_base/dashboard_nyc_taxi"),
repo,
} {
t.Run(inputPath, func(t *testing.T) {
info, err := git.FetchRepositoryInfo(ctx, inputPath, wt.W)
assert.NoError(t, err)
assertFullGitInfo(t, repo, info)
})
}
}

func cloneRepoLocally(t *testing.T, repoUrl string) string {
tempDir := t.TempDir()
localRoot := filepath.Join(tempDir, "repo")

cmd := exec.Command("git", "clone", "--depth=1", examplesRepoUrl, localRoot)
err := cmd.Run()
require.NoError(t, err)
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(filepath.Join(root, "a/b/c"), 0700))

tests := []string{
filepath.Join(root, "a/b/c"),
root,
filepath.Join(root, "/non-existent"),
}

for _, input := range tests {
t.Run(input, func(t *testing.T) {
info, err := git.FetchRepositoryInfo(ctx, input, wt.W)
assert.NoError(t, err)
assertEmptyGitInfo(t, info)
})
}
}

func TestAccFetchRepositoryInfoDotGit_FromBrokenGitRepo(t *testing.T) {
ctx, wt := acc.WorkspaceTest(t)

tempDir := t.TempDir()
root := filepath.Join(tempDir, "repo")
path := filepath.Join(root, "a/b/c")
require.NoError(t, os.MkdirAll(path, 0700))
require.NoError(t, os.WriteFile(filepath.Join(root, ".git"), []byte(""), 0000))

info, err := git.FetchRepositoryInfo(ctx, path, wt.W)
assert.NoError(t, err)
assertSparseGitInfo(t, root, info)
}
13 changes: 13 additions & 0 deletions libs/diag/diagnostic.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
func Warningf(format string, args ...any) Diagnostics {
return []Diagnostic{
Expand Down
Loading

0 comments on commit 0ad790e

Please sign in to comment.