Remove unused in-place progress logging mode

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

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.

bundle/run/progress/job.go

@@ -34,7 +34,3 @@ func (event *JobProgressEvent) String() string {
 
 	return result.String()
 }
-
-func (event *JobProgressEvent) IsInplaceSupported() bool {
-	return true
-}

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
-}

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

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
-}

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

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

cmd/root/progress_logger_test.go

@@ -19,47 +19,18 @@ type progressLoggerTest struct {
 
 func initializeProgressLoggerTest(t *testing.T) (
 	*progressLoggerTest,
-	*flags.LogLevelFlag,
-	*flags.LogFileFlag,
 	*flags.ProgressLogFormat,
 ) {
 	plt := &progressLoggerTest{
 		Command: &cobra.Command{},
 	}
 	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)

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
-}

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
 }

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)
 

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
-}

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
 }