Migrate progress logger functions to use cmdio directly

NEXT_CHANGELOG.md

@@ -6,6 +6,9 @@
 
 ### CLI
 
+* Remove `inplace` mode for the `--progress-format` flag. ([#3811](https://github.com/databricks/cli/pull/3811))
+* Remove `json` mode for the `--progress-format` flag. ([#3812](https://github.com/databricks/cli/pull/3812))
+
 ### 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/job.go

@@ -74,9 +74,7 @@ func (r *jobRunner) logFailedTasks(ctx context.Context, runId int64) {
 				log.Errorf(ctx, "task %s failed. Unable to fetch error trace: %s", red(task.TaskKey), err)
 				continue
 			}
-			if progressLogger, ok := cmdio.FromContext(ctx); ok {
-				progressLogger.Log(progress.NewTaskErrorEvent(task.TaskKey, taskInfo.Error, taskInfo.ErrorTrace))
-			}
+			cmdio.Log(ctx, progress.NewTaskErrorEvent(task.TaskKey, taskInfo.Error, taskInfo.ErrorTrace))
 			log.Errorf(ctx, "Task %s failed!\nError:\n%s\nTrace:\n%s",
 				red(task.TaskKey), taskInfo.Error, taskInfo.ErrorTrace)
 		} else {
@@ -89,9 +87,8 @@ func (r *jobRunner) logFailedTasks(ctx context.Context, runId int64) {
 // jobRunMonitor tracks state for a single job run and provides callbacks
 // for monitoring progress.
 type jobRunMonitor struct {
-	ctx            context.Context
-	prevState      *jobs.RunState
-	progressLogger *cmdio.Logger
+	ctx       context.Context
+	prevState *jobs.RunState
 }
 
 // onProgress is the single callback that handles all state tracking and logging.
@@ -104,7 +101,7 @@ func (m *jobRunMonitor) onProgress(info *jobs.Run) {
 	// First time we see this run.
 	if m.prevState == nil {
 		log.Infof(m.ctx, "Run available at %s", info.RunPageUrl)
-		m.progressLogger.Log(progress.NewJobRunUrlEvent(info.RunPageUrl))
+		cmdio.Log(m.ctx, progress.NewJobRunUrlEvent(info.RunPageUrl))
 	}
 
 	// No state change: do not log.
@@ -125,7 +122,7 @@ func (m *jobRunMonitor) onProgress(info *jobs.Run) {
 		RunName:   info.RunName,
 		State:     *info.State,
 	}
-	m.progressLogger.Log(event)
+	cmdio.Log(m.ctx, event)
 	log.Info(m.ctx, event.String())
 }
 
@@ -151,15 +148,8 @@ func (r *jobRunner) Run(ctx context.Context, opts *Options) (output.RunOutput, e
 
 	w := r.bundle.WorkspaceClient()
 
-	// callback to log progress events. Called on every poll request
-	progressLogger, ok := cmdio.FromContext(ctx)
-	if !ok {
-		return nil, errors.New("no progress logger found")
-	}
-
 	monitor := &jobRunMonitor{
-		ctx:            ctx,
-		progressLogger: progressLogger,
+		ctx: ctx,
 	}
 
 	waiter, err := w.Jobs.RunNow(ctx, *req)
@@ -171,7 +161,7 @@ func (r *jobRunner) Run(ctx context.Context, opts *Options) (output.RunOutput, e
 		details, err := w.Jobs.GetRun(ctx, jobs.GetRunRequest{
 			RunId: waiter.RunId,
 		})
-		progressLogger.Log(progress.NewJobRunUrlEvent(details.RunPageUrl))
+		cmdio.Log(ctx, progress.NewJobRunUrlEvent(details.RunPageUrl))
 		return nil, err
 	}
 

bundle/run/job_test.go

@@ -9,7 +9,6 @@ import (
 	"github.com/databricks/cli/bundle/config"
 	"github.com/databricks/cli/bundle/config/resources"
 	"github.com/databricks/cli/libs/cmdio"
-	"github.com/databricks/cli/libs/flags"
 	"github.com/databricks/databricks-sdk-go/experimental/mocks"
 	"github.com/databricks/databricks-sdk-go/service/jobs"
 	"github.com/stretchr/testify/mock"
@@ -160,7 +159,6 @@ func TestJobRunnerRestart(t *testing.T) {
 		b.SetWorkpaceClient(m.WorkspaceClient)
 
 		ctx := cmdio.MockDiscard(context.Background())
-		ctx = cmdio.NewContext(ctx, cmdio.NewLogger(flags.ModeAppend))
 
 		jobApi := m.GetMockJobsAPI()
 		jobApi.EXPECT().ListRunsAll(mock.Anything, jobs.ListRunsRequest{
@@ -231,7 +229,6 @@ func TestJobRunnerRestartForContinuousUnpausedJobs(t *testing.T) {
 	b.SetWorkpaceClient(m.WorkspaceClient)
 
 	ctx := cmdio.MockDiscard(context.Background())
-	ctx = cmdio.NewContext(ctx, cmdio.NewLogger(flags.ModeAppend))
 
 	jobApi := m.GetMockJobsAPI()
 

bundle/run/pipeline.go

@@ -106,13 +106,9 @@ func (r *pipelineRunner) Run(ctx context.Context, opts *Options) (output.RunOutp
 
 	// setup progress logger and tracker to query events
 	updateTracker := progress.NewUpdateTracker(pipelineID, updateID, w)
-	progressLogger, ok := cmdio.FromContext(ctx)
-	if !ok {
-		return nil, errors.New("no progress logger found")
-	}
 
 	// Log the pipeline update URL as soon as it is available.
-	progressLogger.Log(progress.NewPipelineUpdateUrlEvent(w.Config.Host, updateID, pipelineID))
+	cmdio.Log(ctx, progress.NewPipelineUpdateUrlEvent(w.Config.Host, updateID, pipelineID))
 
 	if opts.NoWait {
 		return &output.PipelineOutput{
@@ -129,7 +125,7 @@ func (r *pipelineRunner) Run(ctx context.Context, opts *Options) (output.RunOutp
 			return nil, err
 		}
 		for _, event := range events {
-			progressLogger.Log(&event)
+			cmdio.Log(ctx, &event)
 			log.Info(ctx, event.String())
 		}
 

bundle/run/pipeline_test.go

@@ -9,7 +9,6 @@ import (
 	"github.com/databricks/cli/bundle/config"
 	"github.com/databricks/cli/bundle/config/resources"
 	"github.com/databricks/cli/libs/cmdio"
-	"github.com/databricks/cli/libs/flags"
 	sdk_config "github.com/databricks/databricks-sdk-go/config"
 	"github.com/databricks/databricks-sdk-go/experimental/mocks"
 	"github.com/databricks/databricks-sdk-go/service/pipelines"
@@ -76,7 +75,6 @@ func TestPipelineRunnerRestart(t *testing.T) {
 	b.SetWorkpaceClient(m.WorkspaceClient)
 
 	ctx := cmdio.MockDiscard(context.Background())
-	ctx = cmdio.NewContext(ctx, cmdio.NewLogger(flags.ModeAppend))
 
 	mockWait := &pipelines.WaitGetPipelineIdle[struct{}]{
 		Poll: func(time.Duration, func(*pipelines.GetPipelineResponse)) (*pipelines.GetPipelineResponse, error) {

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

bundle/statemgmt/state_push_test.go

@@ -9,6 +9,7 @@ import (
 	"github.com/databricks/cli/bundle"
 	"github.com/databricks/cli/bundle/config"
 	mockfiler "github.com/databricks/cli/internal/mocks/libs/filer"
+	"github.com/databricks/cli/libs/cmdio"
 	"github.com/databricks/cli/libs/filer"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/mock"
@@ -51,7 +52,7 @@ func TestStatePush(t *testing.T) {
 		identityFiler(mock),
 	}
 
-	ctx := context.Background()
+	ctx := cmdio.MockDiscard(context.Background())
 	b := statePushTestBundle(t)
 
 	// Write a stale local state file.

cmd/bundle/destroy.go

@@ -12,9 +12,7 @@ import (
 	"github.com/databricks/cli/bundle/phases"
 	"github.com/databricks/cli/cmd/bundle/utils"
 	"github.com/databricks/cli/cmd/root"
-	"github.com/databricks/cli/libs/cmdio"
 	"github.com/databricks/cli/libs/diag"
-	"github.com/databricks/cli/libs/flags"
 	"github.com/databricks/cli/libs/logdiag"
 	"github.com/spf13/cobra"
 	"golang.org/x/term"
@@ -68,15 +66,6 @@ Typical use cases:
 			return errors.New("please specify --auto-approve to skip interactive confirmation checks for non tty consoles")
 		}
 
-		// Check auto-approve is selected for json logging
-		logger, ok := cmdio.FromContext(ctx)
-		if !ok {
-			return errors.New("progress logger not found")
-		}
-		if logger.Mode == flags.ModeJson && !autoApprove {
-			return errors.New("please specify --auto-approve since selected logging format is json")
-		}
-
 		phases.Initialize(ctx, b)
 		if logdiag.HasError(ctx) {
 			return root.ErrAlreadyPrinted

cmd/pipelines/destroy.go

@@ -12,9 +12,7 @@ import (
 	"github.com/databricks/cli/bundle/phases"
 	"github.com/databricks/cli/cmd/bundle/utils"
 	"github.com/databricks/cli/cmd/root"
-	"github.com/databricks/cli/libs/cmdio"
 	"github.com/databricks/cli/libs/diag"
-	"github.com/databricks/cli/libs/flags"
 	"github.com/databricks/cli/libs/logdiag"
 	"github.com/spf13/cobra"
 	"golang.org/x/term"
@@ -56,15 +54,6 @@ func destroyCommand() *cobra.Command {
 			return errors.New("please specify --auto-approve to skip interactive confirmation checks for non tty consoles")
 		}
 
-		// Check auto-approve is selected for json logging
-		logger, ok := cmdio.FromContext(ctx)
-		if !ok {
-			return errors.New("progress logger not found")
-		}
-		if logger.Mode == flags.ModeJson && !autoApprove {
-			return errors.New("please specify --auto-approve since selected logging format is json")
-		}
-
 		phases.Initialize(ctx, b)
 		if logdiag.HasError(ctx) {
 			return root.ErrAlreadyPrinted

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)")
 	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)")
 	flags.MarkHidden("progress-format")
 	cmd.RegisterFlagCompletionFunc("progress-format", f.Complete)
 	return &f