Skip to content

Commit a29c997

Browse files
authored
Surface psql list errors instead of reporting an empty workspace (#5515)
## Why Found during a full-repo review of the CLI. `databricks psql` lists both provisioned database instances and autoscaling projects to populate its interactive selector, but it discarded the errors from both list calls. When both calls fail (for example, an auth failure), the user gets "no Lakebase databases found in workspace" instead of the real error, which sends them debugging the wrong problem. ## Changes Before, list-API failures were silently ignored and surfaced as an empty-workspace message; now, when both list calls fail, the underlying errors are returned to the user. `listAllDatabases` returns an error joined from both call errors when both fail. A single failing call is still tolerated, since a workspace may have only one of the two products enabled. The interactive selection path wraps and returns the error, and shell completion reports `ShellCompDirectiveError` instead of silently completing nothing. ## Test plan - [x] Added a table-driven unit test for `listAllDatabases` covering both calls failing (error returned, `errors.Is` matches both), each single call failing (tolerated), and both succeeding - [x] `go test ./cmd/psql/` passes - [x] `go test ./acceptance -run 'TestAccept/cmd/psql'` passes - [x] `./task fmt-q`, `./task lint-q`, and `./task checks` pass This pull request and its description were written by Isaac.
1 parent abcea9e commit a29c997

2 files changed

Lines changed: 93 additions & 5 deletions

File tree

cmd/psql/psql.go

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -181,7 +181,10 @@ For more information, see: https://docs.databricks.com/aws/en/oltp/
181181
ctx := cmd.Context()
182182
w := cmdctx.WorkspaceClient(ctx)
183183

184-
instances, projects := listAllDatabases(ctx, w)
184+
instances, projects, err := listAllDatabases(ctx, w)
185+
if err != nil {
186+
return nil, cobra.ShellCompDirectiveError
187+
}
185188

186189
var names []string
187190
for _, inst := range instances {
@@ -237,8 +240,10 @@ func parseResourcePath(input string) (project, branch, endpoint string, err erro
237240
}
238241

239242
// listAllDatabases fetches all database instances and projects in parallel.
240-
// Errors are silently ignored; callers should check for empty results.
241-
func listAllDatabases(ctx context.Context, w *databricks.WorkspaceClient) ([]database.DatabaseInstance, []postgres.Project) {
243+
// A single failing call is tolerated because a workspace may have only one of
244+
// the two products enabled; an error is returned only when both calls fail,
245+
// so that e.g. an auth failure is not reported as an empty workspace.
246+
func listAllDatabases(ctx context.Context, w *databricks.WorkspaceClient) ([]database.DatabaseInstance, []postgres.Project, error) {
242247
type result[T any] struct {
243248
value []T
244249
err error
@@ -260,6 +265,10 @@ func listAllDatabases(ctx context.Context, w *databricks.WorkspaceClient) ([]dat
260265
instResult := <-instancesCh
261266
projResult := <-projectsCh
262267

268+
if instResult.err != nil && projResult.err != nil {
269+
return nil, nil, errors.Join(instResult.err, projResult.err)
270+
}
271+
263272
var instances []database.DatabaseInstance
264273
var projects []postgres.Project
265274
if instResult.err == nil {
@@ -269,7 +278,7 @@ func listAllDatabases(ctx context.Context, w *databricks.WorkspaceClient) ([]dat
269278
projects = projResult.value
270279
}
271280

272-
return instances, projects
281+
return instances, projects, nil
273282
}
274283

275284
// showSelectionAndConnect shows a combined dropdown of Lakebase databases.
@@ -278,8 +287,11 @@ func showSelectionAndConnect(ctx context.Context, retryConfig libpsql.RetryConfi
278287

279288
sp := cmdio.NewSpinner(ctx)
280289
sp.Update("Loading Lakebase databases...")
281-
instances, projects := listAllDatabases(ctx, w)
290+
instances, projects, err := listAllDatabases(ctx, w)
282291
sp.Close()
292+
if err != nil {
293+
return fmt.Errorf("failed to list Lakebase databases: %w", err)
294+
}
283295

284296
if len(instances) == 0 && len(projects) == 0 {
285297
return errors.New("no Lakebase databases found in workspace")

cmd/psql/psql_test.go

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,14 @@
11
package psql
22

33
import (
4+
"errors"
45
"testing"
56

7+
"github.com/databricks/databricks-sdk-go/experimental/mocks"
8+
"github.com/databricks/databricks-sdk-go/service/database"
9+
"github.com/databricks/databricks-sdk-go/service/postgres"
610
"github.com/stretchr/testify/assert"
11+
"github.com/stretchr/testify/mock"
712
"github.com/stretchr/testify/require"
813
)
914

@@ -81,3 +86,74 @@ func TestParseResourcePath(t *testing.T) {
8186
})
8287
}
8388
}
89+
90+
func TestListAllDatabases(t *testing.T) {
91+
instErr := errors.New("instances list failed")
92+
projErr := errors.New("projects list failed")
93+
instances := []database.DatabaseInstance{{Name: "my-instance"}}
94+
projects := []postgres.Project{{Name: "projects/my-project"}}
95+
96+
tests := []struct {
97+
name string
98+
instErr error
99+
projErr error
100+
wantInstances []database.DatabaseInstance
101+
wantProjects []postgres.Project
102+
wantErr bool
103+
}{
104+
{
105+
name: "both succeed",
106+
wantInstances: instances,
107+
wantProjects: projects,
108+
},
109+
{
110+
name: "instances call fails",
111+
instErr: instErr,
112+
wantProjects: projects,
113+
},
114+
{
115+
name: "projects call fails",
116+
projErr: projErr,
117+
wantInstances: instances,
118+
},
119+
{
120+
name: "both calls fail",
121+
instErr: instErr,
122+
projErr: projErr,
123+
wantErr: true,
124+
},
125+
}
126+
127+
for _, tc := range tests {
128+
t.Run(tc.name, func(t *testing.T) {
129+
m := mocks.NewMockWorkspaceClient(t)
130+
131+
var instReturn []database.DatabaseInstance
132+
if tc.instErr == nil {
133+
instReturn = instances
134+
}
135+
m.GetMockDatabaseAPI().EXPECT().
136+
ListDatabaseInstancesAll(mock.Anything, database.ListDatabaseInstancesRequest{}).
137+
Return(instReturn, tc.instErr)
138+
139+
var projReturn []postgres.Project
140+
if tc.projErr == nil {
141+
projReturn = projects
142+
}
143+
m.GetMockPostgresAPI().EXPECT().
144+
ListProjectsAll(mock.Anything, postgres.ListProjectsRequest{}).
145+
Return(projReturn, tc.projErr)
146+
147+
gotInstances, gotProjects, err := listAllDatabases(t.Context(), m.WorkspaceClient)
148+
if tc.wantErr {
149+
require.Error(t, err)
150+
assert.ErrorIs(t, err, tc.instErr)
151+
assert.ErrorIs(t, err, tc.projErr)
152+
return
153+
}
154+
require.NoError(t, err)
155+
assert.Equal(t, tc.wantInstances, gotInstances)
156+
assert.Equal(t, tc.wantProjects, gotProjects)
157+
})
158+
}
159+
}

0 commit comments

Comments
 (0)