Skip to content

Commit ed84fe9

Browse files
asnarepietern
andauthored
Fix a potential panic during databricks labs install --offline=true (#3955)
## Changes This PR fixes a panic during `databricks labs install --offline=true` if the release cache can't be loaded. (Preliminary support for offline installs was introduced by #2049.) ## Why This is easy to hit during offline installation if the archive hasn't been unpacked into the location expected by the code. When this happens the code should not panic but rather propagate the error. ## Tests Tested manually, and via some additional tests that cover loading the release cache in offline mode: - A test for the happy path (already covered indirectly) - A new test for the error handling (that would have revealed the panic). Co-authored-by: Pieter Noordhuis <[email protected]>
1 parent b580590 commit ed84fe9

File tree

2 files changed

+29
-1
lines changed

2 files changed

+29
-1
lines changed

cmd/labs/github/releases.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,10 @@ func (r *ReleaseCache) Load(ctx context.Context) (Versions, error) {
3737
})
3838
}
3939
cached, err := r.cache.LoadCache()
40-
return cached.Data, err
40+
if err != nil {
41+
return nil, err
42+
}
43+
return cached.Data, nil
4144
}
4245

4346
// getVersions is considered to be a private API, as we want the usage go through a cache

cmd/labs/github/releases_test.go

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,12 @@ import (
44
"context"
55
"net/http"
66
"net/http/httptest"
7+
"os"
8+
"path/filepath"
79
"testing"
810

911
"github.com/stretchr/testify/assert"
12+
"github.com/stretchr/testify/require"
1013
)
1114

1215
func TestLoadsReleasesForCLI(t *testing.T) {
@@ -35,3 +38,25 @@ func TestLoadsReleasesForCLI(t *testing.T) {
3538
_, err = r.Load(ctx)
3639
assert.NoError(t, err)
3740
}
41+
42+
func TestLoadsReleasesWhenOffline(t *testing.T) {
43+
cacheDir := t.TempDir()
44+
cacheFile := filepath.Join(cacheDir, "databricks-cli-releases.json")
45+
cache := `{"data":[{"tag_name":"v1.2.3"},{"tag_name":"v1.2.2"}]}`
46+
err := os.WriteFile(cacheFile, []byte(cache), 0o644)
47+
require.NoError(t, err)
48+
49+
ctx := context.Background()
50+
r := NewReleaseCache("databricks", "cli", cacheDir, true)
51+
all, err := r.Load(ctx)
52+
assert.NoError(t, err)
53+
assert.Len(t, all, 2)
54+
}
55+
56+
func TestLoadingErrorWhenOffline(t *testing.T) {
57+
ctx := context.Background()
58+
r := NewReleaseCache("databricks", "cli", t.TempDir(), true)
59+
all, err := r.Load(ctx)
60+
assert.Error(t, err)
61+
assert.Nil(t, all)
62+
}

0 commit comments

Comments
 (0)