Skip to content

Commit 1fb1ece

Browse files
Fix spurious git-info warning on DBR acceptance tests (#5426)
## Problem Acceptance tests on DBR were failing because we would emit a warning that the repo does not exist. That did not happen normally (the warnings were silenced) causing test failures. This PR fixes that.
1 parent d342956 commit 1fb1ece

3 files changed

Lines changed: 33 additions & 31 deletions

File tree

bundle/config/mutator/load_git_details.go

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,6 @@ package mutator
22

33
import (
44
"context"
5-
"errors"
6-
"os"
75
"path/filepath"
86

97
"github.com/databricks/cli/bundle"
@@ -26,9 +24,7 @@ func (m *loadGitDetails) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagn
2624
var diags diag.Diagnostics
2725
info, err := git.FetchRepositoryInfo(ctx, b.BundleRoot.Native(), b.WorkspaceClient(ctx))
2826
if err != nil {
29-
if !errors.Is(err, os.ErrNotExist) {
30-
diags = append(diags, diag.WarningFromErr(err)...)
31-
}
27+
diags = append(diags, diag.WarningFromErr(err)...)
3228
}
3329

3430
if info.WorktreeRoot == "" {

integration/libs/git/git_fetch_test.go

Lines changed: 7 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -77,37 +77,21 @@ func TestFetchRepositoryInfoAPI_FromNonRepo(t *testing.T) {
7777

7878
ctx = dbr.MockRuntime(ctx, dbr.Environment{IsDbr: true, Version: "15.4"})
7979

80+
// A path outside a Repo has no git info; a non-existent path is treated the
81+
// same way (no repository there), so all cases return empty info with no error.
8082
tests := []struct {
8183
name string
8284
input string
83-
msg string
8485
}{
85-
{
86-
name: "subdir",
87-
input: path.Join(rootPath, "a/b/c"),
88-
msg: "",
89-
},
90-
{
91-
name: "root",
92-
input: rootPath,
93-
msg: "",
94-
},
95-
{
96-
name: "non-existent",
97-
input: path.Join(rootPath, "/non-existent"),
98-
msg: "doesn't exist",
99-
},
86+
{"subdir", path.Join(rootPath, "a/b/c")},
87+
{"root", rootPath},
88+
{"non-existent", path.Join(rootPath, "/non-existent")},
10089
}
10190

10291
for _, test := range tests {
10392
t.Run(test.name, func(t *testing.T) {
10493
info, err := git.FetchRepositoryInfo(ctx, test.input, wt.W)
105-
if test.msg == "" {
106-
assert.NoError(t, err)
107-
} else {
108-
assert.Error(t, err)
109-
assert.ErrorContains(t, err, test.msg)
110-
}
94+
assert.NoError(t, err)
11195
assertEmptyGitInfo(t, info)
11296
})
11397
}
@@ -162,7 +146,7 @@ func TestFetchRepositoryInfoDotGit_FromNonGitRepo(t *testing.T) {
162146
for _, test := range tests {
163147
t.Run(test.name, func(t *testing.T) {
164148
info, err := git.FetchRepositoryInfo(ctx, test.input, wt.W)
165-
assert.ErrorIs(t, err, os.ErrNotExist)
149+
assert.NoError(t, err)
166150
assertEmptyGitInfo(t, info)
167151
})
168152
}

libs/git/info.go

Lines changed: 25 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@ package git
22

33
import (
44
"context"
5+
"errors"
6+
"io/fs"
57
"net/http"
68
"path"
79
"strings"
@@ -12,6 +14,7 @@ import (
1214
"github.com/databricks/cli/libs/log"
1315
"github.com/databricks/cli/libs/vfs"
1416
"github.com/databricks/databricks-sdk-go"
17+
"github.com/databricks/databricks-sdk-go/apierr"
1518
"github.com/databricks/databricks-sdk-go/client"
1619
)
1720

@@ -37,18 +40,29 @@ type response struct {
3740
}
3841

3942
// Fetch repository information either by quering .git or by fetching it from API (for dabs-in-workspace case).
40-
// - In case we could not find git repository, all string fields of RepositoryInfo will be "" and err will be nil.
43+
// - In case we could not find git repository (including when the path does not exist), all string fields of RepositoryInfo will be "" and err will be nil.
4144
// - If there were any errors when trying to determine git root (e.g. API call returned an error or there were permission issues
4245
// reading the file system), all strings fields of RepositoryInfo will be "" and err will be non-nil.
4346
// - If we could determine git worktree root but there were errors when reading metadata (origin, branch, commit), those errors
4447
// will be logged as warnings, RepositoryInfo is guaranteed to have non-empty WorktreeRoot and other fields on best effort basis.
4548
// - In successful case, all fields are set to proper git repository metadata.
4649
func FetchRepositoryInfo(ctx context.Context, path string, w *databricks.WorkspaceClient) (RepositoryInfo, error) {
50+
var info RepositoryInfo
51+
var err error
4752
if strings.HasPrefix(path, "/Workspace/") && dbr.RunsOnRuntime(ctx) {
48-
return fetchRepositoryInfoAPI(ctx, path, w)
53+
info, err = fetchRepositoryInfoAPI(ctx, path, w)
4954
} else {
50-
return fetchRepositoryInfoDotGit(ctx, path)
55+
info, err = fetchRepositoryInfoDotGit(ctx, path)
5156
}
57+
58+
// A path that does not exist just means there is no repository there, which
59+
// is not an error. Both backends report this as fs.ErrNotExist (the API
60+
// backend translates a workspace 404 to it), so it is normalized to a nil
61+
// error in a single place rather than special-cased by every caller.
62+
if errors.Is(err, fs.ErrNotExist) {
63+
return info, nil
64+
}
65+
return info, err
5266
}
5367

5468
func fetchRepositoryInfoAPI(ctx context.Context, path string, w *databricks.WorkspaceClient) (RepositoryInfo, error) {
@@ -75,6 +89,14 @@ func fetchRepositoryInfoAPI(ctx context.Context, path string, w *databricks.Work
7589
&response,
7690
)
7791
if err != nil {
92+
// The workspace API returns 404 when the path is not a workspace object
93+
// (for example, an ephemeral directory that is not part of a Repo).
94+
// Normalize it to fs.ErrNotExist, the same signal fetchRepositoryInfoDotGit
95+
// produces for a missing local path, so FetchRepositoryInfo can treat
96+
// "no path" as "no repository" uniformly.
97+
if apierr.IsMissing(err) {
98+
return result, fs.ErrNotExist
99+
}
78100
return result, err
79101
}
80102

0 commit comments

Comments
 (0)