Skip to content

Commit d88c938

Browse files
authored
Fix labs panic on empty release list and make update check best-effort (#5503)
## Why Found during a full-repo review of the CLI. Two related problems in the `databricks labs` update check that runs before every command of an installed project: 1. The CLI panics with an index-out-of-range error when the release list is empty. This happens for repositories with zero GitHub releases, and also offline: the cache falls back to an empty list when the network is unreachable. 2. The check is fail-closed. It exists only to print an upgrade hint, yet any error aborts the command. The unauthenticated GitHub API allows only 60 requests per hour per IP, so a 403 from rate limiting blocks every `databricks labs <project> ...` invocation until the quota resets. ## Changes Before, an empty release list crashed the CLI and any update-check failure aborted the command; now the upgrade hint is skipped and the command runs normally in both cases. - `cmd/labs/project/project.go`: `checkUpdates` returns early when the loaded release list is empty instead of indexing into it. - `cmd/labs/project/proxy.go`: a failing update check is logged at debug level and the command proceeds, instead of returning the error. ## Test plan - [x] New unit test `TestCheckUpdatesWithEmptyReleaseList`: panics with "index out of range [0] with length 0" before the fix, passes after. - [x] New test `TestRunningCommandWhenUpdateCheckFails`: runs `labs blueprint echo` with the GitHub API returning HTTP 500; fails with "github request failed: 500 Internal Server Error" before the fix, passes after. - [x] `go test ./cmd/labs/...` passes. - [x] `./task fmt-q`, `./task lint-q`, and `./task checks` pass. This pull request and its description were written by Isaac.
1 parent a29c997 commit d88c938

4 files changed

Lines changed: 57 additions & 3 deletions

File tree

cmd/labs/project/command_test.go

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,15 +2,20 @@ package project_test
22

33
import (
44
"context"
5+
"net/http"
6+
"net/http/httptest"
7+
"os"
58
"path/filepath"
69
"testing"
710
"time"
811

12+
"github.com/databricks/cli/cmd/labs/github"
913
"github.com/databricks/cli/internal/testcli"
1014
"github.com/databricks/cli/libs/env"
1115
"github.com/databricks/cli/libs/python"
1216
"github.com/databricks/databricks-sdk-go"
1317
"github.com/stretchr/testify/assert"
18+
"github.com/stretchr/testify/require"
1419
)
1520

1621
type echoOut struct {
@@ -67,3 +72,26 @@ func TestRenderingTable(t *testing.T) {
6772
Third Fourth
6873
`)
6974
}
75+
76+
func TestRunningCommandWhenUpdateCheckFails(t *testing.T) {
77+
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
78+
w.WriteHeader(http.StatusInternalServerError)
79+
}))
80+
defer server.Close()
81+
82+
home := copyTestdata(t, "testdata/installed-in-home")
83+
// Drop the cached releases so the update check has to hit the API.
84+
err := os.Remove(filepath.Join(home, ".databricks", "labs", "blueprint", "cache", "databrickslabs-blueprint-releases.json"))
85+
require.NoError(t, err)
86+
87+
ctx := github.WithApiOverride(t.Context(), server.URL)
88+
ctx = env.WithUserHomeDir(ctx, home)
89+
py, _ := python.DetectExecutable(ctx)
90+
py, _ = filepath.Abs(py)
91+
ctx = env.Set(ctx, "PYTHON_BIN", py)
92+
93+
r := testcli.NewRunner(t, ctx, "labs", "blueprint", "echo")
94+
var out echoOut
95+
r.RunAndParseJSON(&out)
96+
assert.Equal(t, "echo", out.Command)
97+
}

cmd/labs/project/project.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -308,6 +308,10 @@ func (p *Project) checkUpdates(cmd *cobra.Command) error {
308308
if err != nil {
309309
return err
310310
}
311+
// Repositories without releases (and the offline fallback) yield no versions; nothing to advise on.
312+
if len(versions) == 0 {
313+
return nil
314+
}
311315
installed, err := p.InstalledVersion(ctx)
312316
if err != nil {
313317
return err

cmd/labs/project/project_test.go

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,14 @@ package project
22

33
import (
44
"os"
5+
"path/filepath"
56
"strings"
67
"testing"
78

9+
"github.com/databricks/cli/libs/env"
10+
"github.com/spf13/cobra"
811
"github.com/stretchr/testify/assert"
12+
"github.com/stretchr/testify/require"
913
)
1014

1115
func assertEqualPaths(t *testing.T, expected, actual string) {
@@ -19,3 +23,21 @@ func TestLoad(t *testing.T) {
1923
assert.NoError(t, err)
2024
assertEqualPaths(t, "testdata/installed-in-home/.databricks/labs/blueprint/lib", prj.folder)
2125
}
26+
27+
func TestCheckUpdatesWithEmptyReleaseList(t *testing.T) {
28+
ctx := env.WithUserHomeDir(t.Context(), t.TempDir())
29+
rootDir, err := PathInLabs(ctx, "blueprint")
30+
require.NoError(t, err)
31+
prj := &Project{Name: "blueprint", rootDir: rootDir}
32+
require.NoError(t, prj.EnsureFoldersExist())
33+
34+
// The far-future refreshed_at keeps the cache valid so no network call is made.
35+
cache := []byte(`{"refreshed_at": "2033-01-01T00:00:00Z", "data": []}`)
36+
require.NoError(t, os.WriteFile(filepath.Join(prj.CacheDir(), "databrickslabs-blueprint-releases.json"), cache, ownerRW))
37+
version := []byte(`{"version": "v0.3.15", "date": "2023-10-24T15:04:05+01:00"}`)
38+
require.NoError(t, os.WriteFile(filepath.Join(prj.StateDir(), "version.json"), version, ownerRW))
39+
40+
cmd := &cobra.Command{}
41+
cmd.SetContext(ctx)
42+
assert.NoError(t, prj.checkUpdates(cmd))
43+
}

cmd/labs/project/proxy.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -38,9 +38,9 @@ func (cp *proxy) register(parent *cobra.Command) {
3838
}
3939

4040
func (cp *proxy) runE(cmd *cobra.Command, _ []string) error {
41-
err := cp.checkUpdates(cmd)
42-
if err != nil {
43-
return err
41+
// The update check only prints an upgrade hint, so a failure must not block the command.
42+
if err := cp.checkUpdates(cmd); err != nil {
43+
log.Debugf(cmd.Context(), "Failed to check for newer release of %s: %s", cp.Project.Name, err)
4444
}
4545
args, err := cp.commandInput(cmd)
4646
if err != nil {

0 commit comments

Comments
 (0)