diff --git a/NEXT_CHANGELOG.md b/NEXT_CHANGELOG.md index 4f895598c2..8a1908225e 100644 --- a/NEXT_CHANGELOG.md +++ b/NEXT_CHANGELOG.md @@ -6,6 +6,8 @@ ### CLI +* Remove `inplace` mode for the `--progress-format` flag. ([#3811](https://github.com/databricks/cli/pull/3811)) + ### Dependency updates ### Bundles diff --git a/bundle/deployplan/action.go b/bundle/deployplan/action.go index d27488f4bd..6cd4d079b4 100644 --- a/bundle/deployplan/action.go +++ b/bundle/deployplan/action.go @@ -19,11 +19,6 @@ func (a Action) String() string { return fmt.Sprintf(" %s %s", a.ActionType.StringShort(), key) } -// Implements cmdio.Event for cmdio.Log -func (a Action) IsInplaceSupported() bool { - return false -} - type ActionType int // Actions are ordered in increasing severity. diff --git a/bundle/run/progress/job.go b/bundle/run/progress/job.go index 309ba0d79d..6deee451ad 100644 --- a/bundle/run/progress/job.go +++ b/bundle/run/progress/job.go @@ -34,7 +34,3 @@ func (event *JobProgressEvent) String() string { return result.String() } - -func (event *JobProgressEvent) IsInplaceSupported() bool { - return true -} diff --git a/bundle/run/progress/job_events.go b/bundle/run/progress/job_events.go index 26a15a3015..80e4938a71 100644 --- a/bundle/run/progress/job_events.go +++ b/bundle/run/progress/job_events.go @@ -27,10 +27,6 @@ func (event *TaskErrorEvent) String() string { return result.String() } -func (event *TaskErrorEvent) IsInplaceSupported() bool { - return false -} - type JobRunUrlEvent struct { Type string `json:"type"` Url string `json:"url"` @@ -46,7 +42,3 @@ func NewJobRunUrlEvent(url string) *JobRunUrlEvent { func (event *JobRunUrlEvent) String() string { return fmt.Sprintf("Run URL: %s\n", event.Url) } - -func (event *JobRunUrlEvent) IsInplaceSupported() bool { - return false -} diff --git a/bundle/run/progress/pipeline.go b/bundle/run/progress/pipeline.go index 8adad2cf97..a98f074f78 100644 --- a/bundle/run/progress/pipeline.go +++ b/bundle/run/progress/pipeline.go @@ -39,11 +39,6 @@ func (event *ProgressEvent) String() string { return result.String() } -func (event *ProgressEvent) IsInplaceSupported() bool { - return false -} - -// TODO: Add inplace logging to pipelines. https://github.com/databricks/cli/issues/280 type UpdateTracker struct { UpdateId string PipelineId string diff --git a/bundle/run/progress/pipeline_events.go b/bundle/run/progress/pipeline_events.go index 173b6ecf76..22b56fac5f 100644 --- a/bundle/run/progress/pipeline_events.go +++ b/bundle/run/progress/pipeline_events.go @@ -21,7 +21,3 @@ func NewPipelineUpdateUrlEvent(host, updateId, pipelineId string) *PipelineUpdat func (event *PipelineUpdateUrlEvent) String() string { return fmt.Sprintf("Update URL: %s\n", event.Url) } - -func (event *PipelineUpdateUrlEvent) IsInplaceSupported() bool { - return false -} diff --git a/cmd/pipelines/root/progress_logger.go b/cmd/pipelines/root/progress_logger.go index f914626e1c..74ae01b71d 100644 --- a/cmd/pipelines/root/progress_logger.go +++ b/cmd/pipelines/root/progress_logger.go @@ -3,30 +3,17 @@ package root import ( "context" - "errors" - "os" "github.com/databricks/cli/libs/cmdio" "github.com/databricks/cli/libs/env" "github.com/databricks/cli/libs/flags" "github.com/spf13/cobra" - "golang.org/x/term" ) const envProgressFormat = "DATABRICKS_CLI_PROGRESS_FORMAT" 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 (f *progressLoggerFlag) initializeContext(ctx context.Context) (context.Context, error) { @@ -36,14 +23,9 @@ func (f *progressLoggerFlag) initializeContext(ctx context.Context) (context.Con return ctx, nil } - if f.log.level.String() != "disabled" && f.log.file.String() == "stderr" && - f.ProgressLogFormat == flags.ModeInplace { - return nil, errors.New("inplace progress logging cannot be used when log-file is stderr") - } - format := f.ProgressLogFormat if format == flags.ModeDefault { - format = f.resolveModeDefault(format) + format = flags.ModeAppend } progressLogger := cmdio.NewLogger(format) @@ -53,8 +35,6 @@ func (f *progressLoggerFlag) initializeContext(ctx context.Context) (context.Con func initProgressLoggerFlag(cmd *cobra.Command, logFlags *logFlags) *progressLoggerFlag { f := progressLoggerFlag{ ProgressLogFormat: flags.NewProgressLogFormat(), - - log: logFlags, } // Configure defaults from environment, if applicable. @@ -64,7 +44,7 @@ func initProgressLoggerFlag(cmd *cobra.Command, logFlags *logFlags) *progressLog } flags := cmd.PersistentFlags() - flags.Var(&f.ProgressLogFormat, "progress-format", "format for progress logs (append, inplace, json)") + flags.Var(&f.ProgressLogFormat, "progress-format", "format for progress logs (append, json)") flags.MarkHidden("progress-format") cmd.RegisterFlagCompletionFunc("progress-format", f.Complete) return &f diff --git a/cmd/root/progress_logger.go b/cmd/root/progress_logger.go index 0e3efa6afb..f7185b92fd 100644 --- a/cmd/root/progress_logger.go +++ b/cmd/root/progress_logger.go @@ -2,30 +2,17 @@ package root import ( "context" - "errors" - "os" "github.com/databricks/cli/libs/cmdio" "github.com/databricks/cli/libs/env" "github.com/databricks/cli/libs/flags" "github.com/spf13/cobra" - "golang.org/x/term" ) const envProgressFormat = "DATABRICKS_CLI_PROGRESS_FORMAT" 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 (f *progressLoggerFlag) initializeContext(ctx context.Context) (context.Context, error) { @@ -35,14 +22,9 @@ func (f *progressLoggerFlag) initializeContext(ctx context.Context) (context.Con return ctx, nil } - if f.log.level.String() != "disabled" && f.log.file.String() == "stderr" && - f.ProgressLogFormat == flags.ModeInplace { - return nil, errors.New("inplace progress logging cannot be used when log-file is stderr") - } - format := f.ProgressLogFormat if format == flags.ModeDefault { - format = f.resolveModeDefault(format) + format = flags.ModeAppend } progressLogger := cmdio.NewLogger(format) @@ -52,8 +34,6 @@ func (f *progressLoggerFlag) initializeContext(ctx context.Context) (context.Con func initProgressLoggerFlag(cmd *cobra.Command, logFlags *logFlags) *progressLoggerFlag { f := progressLoggerFlag{ ProgressLogFormat: flags.NewProgressLogFormat(), - - log: logFlags, } // Configure defaults from environment, if applicable. @@ -63,7 +43,7 @@ func initProgressLoggerFlag(cmd *cobra.Command, logFlags *logFlags) *progressLog } flags := cmd.PersistentFlags() - flags.Var(&f.ProgressLogFormat, "progress-format", "format for progress logs (append, inplace, json)") + flags.Var(&f.ProgressLogFormat, "progress-format", "format for progress logs (append, json)") flags.MarkHidden("progress-format") cmd.RegisterFlagCompletionFunc("progress-format", f.Complete) return &f diff --git a/cmd/root/progress_logger_test.go b/cmd/root/progress_logger_test.go index ed39ca59f0..2095976a37 100644 --- a/cmd/root/progress_logger_test.go +++ b/cmd/root/progress_logger_test.go @@ -19,8 +19,6 @@ type progressLoggerTest struct { func initializeProgressLoggerTest(t *testing.T) ( *progressLoggerTest, - *flags.LogLevelFlag, - *flags.LogFileFlag, *flags.ProgressLogFormat, ) { plt := &progressLoggerTest{ @@ -28,38 +26,11 @@ func initializeProgressLoggerTest(t *testing.T) ( } plt.logFlags = initLogFlags(plt.Command) plt.progressLoggerFlag = initProgressLoggerFlag(plt.Command, plt.logFlags) - return plt, &plt.level, &plt.file, &plt.ProgressLogFormat -} - -func TestInitializeErrorOnIncompatibleConfig(t *testing.T) { - plt, logLevel, logFile, progressFormat := initializeProgressLoggerTest(t) - require.NoError(t, logLevel.Set("info")) - require.NoError(t, logFile.Set("stderr")) - require.NoError(t, progressFormat.Set("inplace")) - _, err := plt.progressLoggerFlag.initializeContext(context.Background()) - assert.ErrorContains(t, err, "inplace progress logging cannot be used when log-file is stderr") -} - -func TestNoErrorOnDisabledLogLevel(t *testing.T) { - plt, logLevel, logFile, progressFormat := initializeProgressLoggerTest(t) - require.NoError(t, logLevel.Set("disabled")) - require.NoError(t, logFile.Set("stderr")) - require.NoError(t, progressFormat.Set("inplace")) - _, err := plt.progressLoggerFlag.initializeContext(context.Background()) - assert.NoError(t, err) -} - -func TestNoErrorOnNonStderrLogFile(t *testing.T) { - plt, logLevel, logFile, progressFormat := initializeProgressLoggerTest(t) - require.NoError(t, logLevel.Set("info")) - require.NoError(t, logFile.Set("stdout")) - require.NoError(t, progressFormat.Set("inplace")) - _, err := plt.progressLoggerFlag.initializeContext(context.Background()) - assert.NoError(t, err) + return plt, &plt.ProgressLogFormat } func TestDefaultLoggerModeResolution(t *testing.T) { - plt, _, _, progressFormat := initializeProgressLoggerTest(t) + plt, progressFormat := initializeProgressLoggerTest(t) require.Equal(t, *progressFormat, flags.ModeDefault) ctx, err := plt.progressLoggerFlag.initializeContext(context.Background()) require.NoError(t, err) diff --git a/libs/cmdio/error_event.go b/libs/cmdio/error_event.go index 62897995bc..6a6dd8e795 100644 --- a/libs/cmdio/error_event.go +++ b/libs/cmdio/error_event.go @@ -7,7 +7,3 @@ type ErrorEvent struct { func (event *ErrorEvent) String() string { return "Error: " + event.Error } - -func (event *ErrorEvent) IsInplaceSupported() bool { - return false -} diff --git a/libs/cmdio/event.go b/libs/cmdio/event.go index 6976ab3d88..c3c09acbdc 100644 --- a/libs/cmdio/event.go +++ b/libs/cmdio/event.go @@ -3,7 +3,4 @@ package cmdio type Event interface { // convert event into human readable string String() string - - // true if event supports inplace logging, return false otherwise - IsInplaceSupported() bool } diff --git a/libs/cmdio/logger.go b/libs/cmdio/logger.go index 7d369a8a08..873f3be2c0 100644 --- a/libs/cmdio/logger.go +++ b/libs/cmdio/logger.go @@ -17,7 +17,7 @@ import ( // This is the interface for all io interactions with a user type Logger struct { - // Mode for the logger. One of (append, inplace, json). + // Mode for the logger. One of (append, json). Mode flags.ProgressLogFormat // Input stream (eg. stdin). Answers to questions prompted using the Ask() method @@ -27,28 +27,22 @@ type Logger struct { // Output stream where the logger writes to Writer io.Writer - // If true, indicates no events have been printed by the logger yet. Used - // by inplace logging for formatting - isFirstEvent bool - mutex sync.Mutex } func NewLogger(mode flags.ProgressLogFormat) *Logger { return &Logger{ - Mode: mode, - Writer: os.Stderr, - Reader: *bufio.NewReader(os.Stdin), - isFirstEvent: true, + Mode: mode, + Writer: os.Stderr, + Reader: *bufio.NewReader(os.Stdin), } } func Default() *Logger { return &Logger{ - Mode: flags.ModeAppend, - Writer: os.Stderr, - Reader: *bufio.NewReader(os.Stdin), - isFirstEvent: true, + Mode: flags.ModeAppend, + Writer: os.Stderr, + Reader: *bufio.NewReader(os.Stdin), } } @@ -201,34 +195,10 @@ func (l *Logger) writeAppend(event Event) { _, _ = l.Writer.Write([]byte("\n")) } -func (l *Logger) writeInplace(event Event) { - if l.isFirstEvent { - // save cursor location - _, _ = l.Writer.Write([]byte("\033[s")) - } - - // move cursor to saved location - _, _ = l.Writer.Write([]byte("\033[u")) - - // clear from cursor to end of screen - _, _ = l.Writer.Write([]byte("\033[0J")) - - _, _ = l.Writer.Write([]byte(event.String())) - _, _ = l.Writer.Write([]byte("\n")) - l.isFirstEvent = false -} - func (l *Logger) Log(event Event) { l.mutex.Lock() defer l.mutex.Unlock() switch l.Mode { - case flags.ModeInplace: - if event.IsInplaceSupported() { - l.writeInplace(event) - } else { - l.writeAppend(event) - } - case flags.ModeJson: l.writeJson(event) diff --git a/libs/cmdio/message_event.go b/libs/cmdio/message_event.go index 55dc136cd4..19272ce071 100644 --- a/libs/cmdio/message_event.go +++ b/libs/cmdio/message_event.go @@ -7,7 +7,3 @@ type MessageEvent struct { func (event *MessageEvent) String() string { return event.Message } - -func (event *MessageEvent) IsInplaceSupported() bool { - return false -} diff --git a/libs/flags/progress_format.go b/libs/flags/progress_format.go index a0c0a8d30b..bf6e393809 100644 --- a/libs/flags/progress_format.go +++ b/libs/flags/progress_format.go @@ -11,7 +11,6 @@ type ProgressLogFormat string var ( ModeAppend = ProgressLogFormat("append") - ModeInplace = ProgressLogFormat("inplace") ModeJson = ProgressLogFormat("json") ModeDefault = ProgressLogFormat("default") ) @@ -29,8 +28,6 @@ func (p *ProgressLogFormat) Set(s string) error { switch lower { case ModeAppend.String(): *p = ProgressLogFormat(ModeAppend.String()) - case ModeInplace.String(): - *p = ProgressLogFormat(ModeInplace.String()) case ModeJson.String(): *p = ProgressLogFormat(ModeJson.String()) case ModeDefault.String(): @@ -41,7 +38,6 @@ func (p *ProgressLogFormat) Set(s string) error { default: valid := []string{ ModeAppend.String(), - ModeInplace.String(), ModeJson.String(), } return fmt.Errorf("accepted arguments are [%s]", strings.Join(valid, ", ")) @@ -57,7 +53,6 @@ func (p *ProgressLogFormat) Type() string { func (p *ProgressLogFormat) Complete(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) { return []string{ "append", - "inplace", "json", }, cobra.ShellCompDirectiveNoFileComp } diff --git a/libs/flags/progress_format_test.go b/libs/flags/progress_format_test.go index 55d6da65b9..46014960e4 100644 --- a/libs/flags/progress_format_test.go +++ b/libs/flags/progress_format_test.go @@ -16,7 +16,7 @@ func TestProgressFormatSet(t *testing.T) { // invalid arg err := p.Set("foo") - assert.ErrorContains(t, err, "accepted arguments are [append, inplace, json]") + assert.ErrorContains(t, err, "accepted arguments are [append, json]") // set json err = p.Set("json") @@ -43,17 +43,4 @@ func TestProgressFormatSet(t *testing.T) { err = p.Set("APPEND") assert.NoError(t, err) assert.Equal(t, "append", p.String()) - - // set inplace - err = p.Set("inplace") - assert.NoError(t, err) - assert.Equal(t, "inplace", p.String()) - - err = p.Set("Inplace") - assert.NoError(t, err) - assert.Equal(t, "inplace", p.String()) - - err = p.Set("INPLACE") - assert.NoError(t, err) - assert.Equal(t, "inplace", p.String()) }