Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove dependency on global state for the root command #606

Merged
merged 3 commits into from
Jul 26, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions cmd/bundle/run.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ var runCmd = &cobra.Command{
return err
}
if output != nil {
switch root.OutputType() {
switch root.OutputType(cmd) {
case flags.OutputText:
resultString, err := output.String()
if err != nil {
Expand All @@ -61,7 +61,7 @@ var runCmd = &cobra.Command{
}
cmd.OutOrStdout().Write(b)
default:
return fmt.Errorf("unknown output type %s", root.OutputType())
return fmt.Errorf("unknown output type %s", root.OutputType(cmd))
}
}
return nil
Expand Down
6 changes: 3 additions & 3 deletions cmd/root/auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,9 @@ var workspaceClient int
var accountClient int
var currentUser int

func init() {
RootCmd.PersistentFlags().StringP("profile", "p", "", "~/.databrickscfg profile")
RootCmd.RegisterFlagCompletionFunc("profile", databrickscfg.ProfileCompletion)
func initProfileFlag(cmd *cobra.Command) {
cmd.PersistentFlags().StringP("profile", "p", "", "~/.databrickscfg profile")
cmd.RegisterFlagCompletionFunc("profile", databrickscfg.ProfileCompletion)
}

func MustAccountClient(cmd *cobra.Command, args []string) error {
Expand Down
6 changes: 3 additions & 3 deletions cmd/root/bundle.go
Original file line number Diff line number Diff line change
Expand Up @@ -118,8 +118,8 @@ func environmentCompletion(cmd *cobra.Command, args []string, toComplete string)
return maps.Keys(b.Config.Environments), cobra.ShellCompDirectiveDefault
}

func init() {
func initEnvironmentFlag(cmd *cobra.Command) {
// To operate in the context of a bundle, all commands must take an "environment" parameter.
RootCmd.PersistentFlags().StringP("environment", "e", "", "bundle environment to use (if applicable)")
RootCmd.RegisterFlagCompletionFunc("environment", environmentCompletion)
cmd.PersistentFlags().StringP("environment", "e", "", "bundle environment to use (if applicable)")
cmd.RegisterFlagCompletionFunc("environment", environmentCompletion)
}
51 changes: 31 additions & 20 deletions cmd/root/bundle_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (

"github.com/databricks/cli/bundle"
"github.com/databricks/cli/bundle/config"
"github.com/spf13/cobra"
"github.com/stretchr/testify/assert"
)

Expand All @@ -27,15 +28,18 @@ func setupDatabricksCfg(t *testing.T) {
t.Setenv(homeEnvVar, tempHomeDir)
}

func setup(t *testing.T, host string) *bundle.Bundle {
setupDatabricksCfg(t)

func emptyCommand(t *testing.T) *cobra.Command {
ctx := context.Background()
RootCmd.SetContext(ctx)
_, err := initializeLogger(ctx)
assert.NoError(t, err)
cmd := &cobra.Command{}
cmd.SetContext(ctx)
initProfileFlag(cmd)
return cmd
}

err = configureBundle(RootCmd, []string{"validate"}, func() (*bundle.Bundle, error) {
func setup(t *testing.T, cmd *cobra.Command, host string) *bundle.Bundle {
setupDatabricksCfg(t)

err := configureBundle(cmd, []string{"validate"}, func() (*bundle.Bundle, error) {
return &bundle.Bundle{
Config: config.Root{
Bundle: config.Bundle{
Expand All @@ -48,46 +52,50 @@ func setup(t *testing.T, host string) *bundle.Bundle {
}, nil
})
assert.NoError(t, err)

return bundle.Get(RootCmd.Context())
return bundle.Get(cmd.Context())
}

func TestBundleConfigureDefault(t *testing.T) {
b := setup(t, "https://x.com")
cmd := emptyCommand(t)
b := setup(t, cmd, "https://x.com")
assert.NotPanics(t, func() {
b.WorkspaceClient()
})
}

func TestBundleConfigureWithMultipleMatches(t *testing.T) {
b := setup(t, "https://a.com")
cmd := emptyCommand(t)
b := setup(t, cmd, "https://a.com")
assert.Panics(t, func() {
b.WorkspaceClient()
})
}

func TestBundleConfigureWithNonExistentProfileFlag(t *testing.T) {
RootCmd.Flag("profile").Value.Set("NOEXIST")
cmd := emptyCommand(t)
cmd.Flag("profile").Value.Set("NOEXIST")

b := setup(t, "https://x.com")
b := setup(t, cmd, "https://x.com")
assert.PanicsWithError(t, "no matching config profiles found", func() {
b.WorkspaceClient()
})
}

func TestBundleConfigureWithMismatchedProfile(t *testing.T) {
RootCmd.Flag("profile").Value.Set("PROFILE-1")
cmd := emptyCommand(t)
cmd.Flag("profile").Value.Set("PROFILE-1")

b := setup(t, "https://x.com")
b := setup(t, cmd, "https://x.com")
assert.PanicsWithError(t, "config host mismatch: profile uses host https://a.com, but CLI configured to use https://x.com", func() {
b.WorkspaceClient()
})
}

func TestBundleConfigureWithCorrectProfile(t *testing.T) {
RootCmd.Flag("profile").Value.Set("PROFILE-1")
cmd := emptyCommand(t)
cmd.Flag("profile").Value.Set("PROFILE-1")

b := setup(t, "https://a.com")
b := setup(t, cmd, "https://a.com")
assert.NotPanics(t, func() {
b.WorkspaceClient()
})
Expand All @@ -99,7 +107,8 @@ func TestBundleConfigureWithMismatchedProfileEnvVariable(t *testing.T) {
t.Setenv("DATABRICKS_CONFIG_PROFILE", "")
})

b := setup(t, "https://x.com")
cmd := emptyCommand(t)
b := setup(t, cmd, "https://x.com")
assert.PanicsWithError(t, "config host mismatch: profile uses host https://a.com, but CLI configured to use https://x.com", func() {
b.WorkspaceClient()
})
Expand All @@ -110,9 +119,11 @@ func TestBundleConfigureWithProfileFlagAndEnvVariable(t *testing.T) {
t.Cleanup(func() {
t.Setenv("DATABRICKS_CONFIG_PROFILE", "")
})
RootCmd.Flag("profile").Value.Set("PROFILE-1")

b := setup(t, "https://a.com")
cmd := emptyCommand(t)
cmd.Flag("profile").Value.Set("PROFILE-1")

b := setup(t, cmd, "https://a.com")
assert.NotPanics(t, func() {
b.WorkspaceClient()
})
Expand Down
29 changes: 20 additions & 9 deletions cmd/root/io.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,32 +10,43 @@ import (

const envOutputFormat = "DATABRICKS_OUTPUT_FORMAT"

var outputType flags.Output = flags.OutputText
type outputFlag struct {
output flags.Output
}

func initOutputFlag(cmd *cobra.Command) *outputFlag {
f := outputFlag{
output: flags.OutputText,
}

func init() {
// Configure defaults from environment, if applicable.
// If the provided value is invalid it is ignored.
if v, ok := os.LookupEnv(envOutputFormat); ok {
outputType.Set(v)
f.output.Set(v)
}

RootCmd.PersistentFlags().VarP(&outputType, "output", "o", "output type: text or json")
cmd.PersistentFlags().VarP(&f.output, "output", "o", "output type: text or json")
return &f
}

func OutputType() flags.Output {
return outputType
func OutputType(cmd *cobra.Command) flags.Output {
f, ok := cmd.Flag("output").Value.(*flags.Output)
if !ok {
panic("output flag not defined")
}

return *f
}

func initializeIO(cmd *cobra.Command) error {
func (f *outputFlag) initializeIO(cmd *cobra.Command) error {
var template string
if cmd.Annotations != nil {
// rely on zeroval being an empty string
template = cmd.Annotations["template"]
}

cmdIO := cmdio.NewIO(outputType, cmd.InOrStdin(), cmd.OutOrStdout(), cmd.ErrOrStderr(), template)
cmdIO := cmdio.NewIO(f.output, cmd.InOrStdin(), cmd.OutOrStdout(), cmd.ErrOrStderr(), template)
ctx := cmdio.InContext(cmd.Context(), cmdIO)
cmd.SetContext(ctx)

return nil
}
54 changes: 32 additions & 22 deletions cmd/root/logger.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"github.com/databricks/cli/libs/flags"
"github.com/databricks/cli/libs/log"
"github.com/fatih/color"
"github.com/spf13/cobra"
"golang.org/x/exp/slog"
)

Expand Down Expand Up @@ -66,12 +67,18 @@ func (l *friendlyHandler) Handle(ctx context.Context, rec slog.Record) error {
return err
}

func makeLogHandler(opts slog.HandlerOptions) (slog.Handler, error) {
switch logOutput {
type logFlags struct {
file flags.LogFileFlag
level flags.LogLevelFlag
output flags.Output
}

func (f *logFlags) makeLogHandler(opts slog.HandlerOptions) (slog.Handler, error) {
switch f.output {
case flags.OutputJSON:
return opts.NewJSONHandler(logFile.Writer()), nil
return opts.NewJSONHandler(f.file.Writer()), nil
case flags.OutputText:
w := logFile.Writer()
w := f.file.Writer()
if cmdio.IsTTY(w) {
return &friendlyHandler{
Handler: opts.NewTextHandler(w),
Expand All @@ -81,26 +88,26 @@ func makeLogHandler(opts slog.HandlerOptions) (slog.Handler, error) {
return opts.NewTextHandler(w), nil

default:
return nil, fmt.Errorf("invalid log output mode: %s", logOutput)
return nil, fmt.Errorf("invalid log output mode: %s", f.output)
}
}

func initializeLogger(ctx context.Context) (context.Context, error) {
func (f *logFlags) initializeContext(ctx context.Context) (context.Context, error) {
opts := slog.HandlerOptions{}
opts.Level = logLevel.Level()
opts.Level = f.level.Level()
opts.AddSource = true
opts.ReplaceAttr = log.ReplaceAttrFunctions{
log.ReplaceLevelAttr,
log.ReplaceSourceAttr,
}.ReplaceAttr

// Open the underlying log file if the user configured an actual file to log to.
err := logFile.Open()
err := f.file.Open()
if err != nil {
return nil, err
}

handler, err := makeLogHandler(opts)
handler, err := f.makeLogHandler(opts)
if err != nil {
return nil, err
}
Expand All @@ -109,27 +116,30 @@ func initializeLogger(ctx context.Context) (context.Context, error) {
return log.NewContext(ctx, slog.Default()), nil
}

var logFile = flags.NewLogFileFlag()
var logLevel = flags.NewLogLevelFlag()
var logOutput = flags.OutputText
func initLogFlags(cmd *cobra.Command) *logFlags {
f := logFlags{
file: flags.NewLogFileFlag(),
level: flags.NewLogLevelFlag(),
output: flags.OutputText,
}

func init() {
// Configure defaults from environment, if applicable.
// If the provided value is invalid it is ignored.
if v, ok := os.LookupEnv(envLogFile); ok {
logFile.Set(v)
f.file.Set(v)
}
if v, ok := os.LookupEnv(envLogLevel); ok {
logLevel.Set(v)
f.level.Set(v)
}
if v, ok := os.LookupEnv(envLogFormat); ok {
logOutput.Set(v)
f.output.Set(v)
}

RootCmd.PersistentFlags().Var(&logFile, "log-file", "file to write logs to")
RootCmd.PersistentFlags().Var(&logLevel, "log-level", "log level")
RootCmd.PersistentFlags().Var(&logOutput, "log-format", "log output format (text or json)")
RootCmd.RegisterFlagCompletionFunc("log-file", logFile.Complete)
RootCmd.RegisterFlagCompletionFunc("log-level", logLevel.Complete)
RootCmd.RegisterFlagCompletionFunc("log-format", logOutput.Complete)
cmd.PersistentFlags().Var(&f.file, "log-file", "file to write logs to")
cmd.PersistentFlags().Var(&f.level, "log-level", "log level")
cmd.PersistentFlags().Var(&f.output, "log-format", "log output format (text or json)")
cmd.RegisterFlagCompletionFunc("log-file", f.file.Complete)
cmd.RegisterFlagCompletionFunc("log-level", f.level.Complete)
cmd.RegisterFlagCompletionFunc("log-format", f.output.Complete)
return &f
}
37 changes: 25 additions & 12 deletions cmd/root/progress_logger.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,42 +7,55 @@ import (

"github.com/databricks/cli/libs/cmdio"
"github.com/databricks/cli/libs/flags"
"github.com/spf13/cobra"
"golang.org/x/term"
)

const envProgressFormat = "DATABRICKS_CLI_PROGRESS_FORMAT"

func resolveModeDefault(format flags.ProgressLogFormat) flags.ProgressLogFormat {
if (logLevel.String() == "disabled" || logFile.String() != "stderr") &&
type progressLoggerFlag struct {
flags.ProgressLogFormat

log *logFlags
}

func (f *progressLoggerFlag) resolveModeDefault(format flags.ProgressLogFormat) flags.ProgressLogFormat {
if (f.log.level.String() == "disabled" || f.log.file.String() != "stderr") &&
term.IsTerminal(int(os.Stderr.Fd())) {
return flags.ModeInplace
}
return flags.ModeAppend
}

func initializeProgressLogger(ctx context.Context) (context.Context, error) {
if logLevel.String() != "disabled" && logFile.String() == "stderr" &&
progressFormat == flags.ModeInplace {
func (f *progressLoggerFlag) initializeContext(ctx context.Context) (context.Context, error) {
if f.log.level.String() != "disabled" && f.log.file.String() == "stderr" &&
f.ProgressLogFormat == flags.ModeInplace {
return nil, fmt.Errorf("inplace progress logging cannot be used when log-file is stderr")
}

format := progressFormat
format := f.ProgressLogFormat
if format == flags.ModeDefault {
format = resolveModeDefault(format)
format = f.resolveModeDefault(format)
}

progressLogger := cmdio.NewLogger(format)
return cmdio.NewContext(ctx, progressLogger), nil
}

var progressFormat = flags.NewProgressLogFormat()
func initProgressLoggerFlag(cmd *cobra.Command, logFlags *logFlags) *progressLoggerFlag {
f := progressLoggerFlag{
ProgressLogFormat: flags.NewProgressLogFormat(),

log: logFlags,
}

func init() {
// Configure defaults from environment, if applicable.
// If the provided value is invalid it is ignored.
if v, ok := os.LookupEnv(envProgressFormat); ok {
progressFormat.Set(v)
f.Set(v)
}
RootCmd.PersistentFlags().Var(&progressFormat, "progress-format", "format for progress logs (append, inplace, json)")
RootCmd.RegisterFlagCompletionFunc("progress-format", progressFormat.Complete)

cmd.PersistentFlags().Var(&f.ProgressLogFormat, "progress-format", "format for progress logs (append, inplace, json)")
cmd.RegisterFlagCompletionFunc("progress-format", f.ProgressLogFormat.Complete)
return &f
}
Loading