Skip to content

Commit 8c0a915

Browse files
committed
Fix review findings: config resolution, timeouts, output routing, tests
Co-authored-by: Isaac
1 parent ca0b554 commit 8c0a915

3 files changed

Lines changed: 250 additions & 52 deletions

File tree

cmd/doctor/checks.go

Lines changed: 38 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,15 @@
11
package doctor
22

33
import (
4+
"errors"
45
"fmt"
6+
"io"
57
"net/http"
8+
"time"
69

710
"github.com/databricks/cli/internal/build"
811
"github.com/databricks/cli/libs/databrickscfg/profile"
12+
"github.com/databricks/cli/libs/env"
913
"github.com/databricks/databricks-sdk-go"
1014
"github.com/databricks/databricks-sdk-go/config"
1115
"github.com/spf13/cobra"
@@ -14,7 +18,10 @@ import (
1418
const (
1519
statusPass = "pass"
1620
statusFail = "fail"
21+
statusWarn = "warn"
1722
statusInfo = "info"
23+
24+
networkTimeout = 10 * time.Second
1825
)
1926

2027
// runChecks runs all diagnostic checks and returns the results.
@@ -67,6 +74,14 @@ func checkConfigFile(cmd *cobra.Command) CheckResult {
6774

6875
profiles, err := profiler.LoadProfiles(ctx, profile.MatchAllProfiles)
6976
if err != nil {
77+
// Config file absence is not a hard failure since auth can work via env vars.
78+
if errors.Is(err, profile.ErrNoConfiguration) {
79+
return CheckResult{
80+
Name: "Config File",
81+
Status: statusWarn,
82+
Message: "No config file found (auth can still work via environment variables)",
83+
}
84+
}
7085
return CheckResult{
7186
Name: "Config File",
7287
Status: statusFail,
@@ -83,18 +98,34 @@ func checkConfigFile(cmd *cobra.Command) CheckResult {
8398
}
8499

85100
func checkCurrentProfile(cmd *cobra.Command) CheckResult {
101+
ctx := cmd.Context()
102+
86103
profileFlag := cmd.Flag("profile")
87-
profileName := "default"
88104
if profileFlag != nil && profileFlag.Changed {
89-
profileName = profileFlag.Value.String()
105+
return CheckResult{
106+
Name: "Current Profile",
107+
Status: statusInfo,
108+
Message: profileFlag.Value.String(),
109+
}
90110
}
111+
112+
if envProfile := env.Get(ctx, "DATABRICKS_CONFIG_PROFILE"); envProfile != "" {
113+
return CheckResult{
114+
Name: "Current Profile",
115+
Status: statusInfo,
116+
Message: envProfile + " (from DATABRICKS_CONFIG_PROFILE)",
117+
}
118+
}
119+
91120
return CheckResult{
92121
Name: "Current Profile",
93122
Status: statusInfo,
94-
Message: profileName,
123+
Message: "none (using environment or defaults)",
95124
}
96125
}
97126

127+
// checkAuth uses the SDK's standard config resolution to authenticate.
128+
// This respects --profile, --host, environment variables, and config file.
98129
func checkAuth(cmd *cobra.Command) (CheckResult, *databricks.WorkspaceClient) {
99130
ctx := cmd.Context()
100131
cfg := &config.Config{}
@@ -198,7 +229,8 @@ func checkNetworkWithHost(cmd *cobra.Command, host string) CheckResult {
198229
}
199230
}
200231

201-
resp, err := http.DefaultClient.Do(req)
232+
client := &http.Client{Timeout: networkTimeout}
233+
resp, err := client.Do(req)
202234
if err != nil {
203235
return CheckResult{
204236
Name: "Network",
@@ -207,7 +239,8 @@ func checkNetworkWithHost(cmd *cobra.Command, host string) CheckResult {
207239
Detail: err.Error(),
208240
}
209241
}
210-
resp.Body.Close()
242+
defer resp.Body.Close()
243+
io.Copy(io.Discard, resp.Body)
211244

212245
return CheckResult{
213246
Name: "Network",

cmd/doctor/doctor.go

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,11 @@
11
package doctor
22

33
import (
4-
"context"
54
"encoding/json"
65
"fmt"
6+
"io"
77

88
"github.com/databricks/cli/cmd/root"
9-
"github.com/databricks/cli/libs/cmdio"
109
"github.com/databricks/cli/libs/flags"
1110
"github.com/fatih/color"
1211
"github.com/spf13/cobra"
@@ -23,13 +22,13 @@ type CheckResult struct {
2322
// New returns the doctor command.
2423
func New() *cobra.Command {
2524
cmd := &cobra.Command{
26-
Use: "doctor",
27-
Args: root.NoArgs,
28-
Short: "Validate your Databricks CLI setup",
25+
Use: "doctor",
26+
Args: root.NoArgs,
27+
Short: "Validate your Databricks CLI setup",
28+
GroupID: "development",
2929
}
3030

3131
cmd.RunE = func(cmd *cobra.Command, args []string) error {
32-
ctx := cmd.Context()
3332
results := runChecks(cmd)
3433

3534
switch root.OutputType(cmd) {
@@ -38,10 +37,11 @@ func New() *cobra.Command {
3837
if err != nil {
3938
return err
4039
}
40+
buf = append(buf, '\n')
4141
_, err = cmd.OutOrStdout().Write(buf)
4242
return err
4343
case flags.OutputText:
44-
renderResults(ctx, results)
44+
renderResults(cmd.OutOrStdout(), results)
4545
return nil
4646
default:
4747
return fmt.Errorf("unknown output type %s", root.OutputType(cmd))
@@ -51,7 +51,7 @@ func New() *cobra.Command {
5151
return cmd
5252
}
5353

54-
func renderResults(ctx context.Context, results []CheckResult) {
54+
func renderResults(w io.Writer, results []CheckResult) {
5555
green := color.New(color.FgGreen).SprintFunc()
5656
red := color.New(color.FgRed).SprintFunc()
5757
yellow := color.New(color.FgYellow).SprintFunc()
@@ -61,19 +61,19 @@ func renderResults(ctx context.Context, results []CheckResult) {
6161
for _, r := range results {
6262
var icon string
6363
switch r.Status {
64-
case "pass":
64+
case statusPass:
6565
icon = green("[ok]")
66-
case "fail":
66+
case statusFail:
6767
icon = red("[FAIL]")
68-
case "warn":
68+
case statusWarn:
6969
icon = yellow("[warn]")
70-
case "info":
70+
case statusInfo:
7171
icon = cyan("[info]")
7272
}
7373
msg := fmt.Sprintf("%s %s: %s", icon, bold(r.Name), r.Message)
7474
if r.Detail != "" {
7575
msg += fmt.Sprintf(" (%s)", r.Detail)
7676
}
77-
cmdio.LogString(ctx, msg)
77+
fmt.Fprintln(w, msg)
7878
}
7979
}

0 commit comments

Comments
 (0)