Remove unused JSON progress logging mode (#3812)

## Changes

The progress logger now only supports `append` mode.

## Why

JSON mode was added in #276 (March 2023). The intent was to make
progress events machine readable but this didn't materialize (we didn't
end up using it in the VS Code extension).

This functionality would print events to stderr if the user specified
`--progress-format json`, or set the an equivalent environment variable.
Because the flag is hidden, and there are no online references to the
functionality, I believe it is safe to remove. If users take a
dependency on JSON output, it should be enabled via the existing
`--output json` flag and be written to stdout.

After this change is merged, the remaining functionality can be moved
into the core `cmdio` types, and the "progress logger" can be removed.
Once there is a single type for all I/O, we have a better shot at
improving it.

Related change: #3811.

## Tests

Tests pass.

Author: pietern

NEXT_CHANGELOG.md

@@ -7,6 +7,7 @@
 ### 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
 

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

@@ -44,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, 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

@@ -43,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, json)")
+	flags.Var(&f.ProgressLogFormat, "progress-format", "format for progress logs (append)")
 	flags.MarkHidden("progress-format")
 	cmd.RegisterFlagCompletionFunc("progress-format", f.Complete)
 	return &f

libs/cmdio/logger.go

@@ -3,8 +3,6 @@ package cmdio
 import (
 	"bufio"
 	"context"
-	"encoding/json"
-	"errors"
 	"fmt"
 	"io"
 	"os"
@@ -17,7 +15,7 @@ import (
 
 // This is the interface for all io interactions with a user
 type Logger struct {
-	// Mode for the logger. One of (append, json).
+	// Mode for the logger. One of (append).
 	Mode flags.ProgressLogFormat
 
 	// Input stream (eg. stdin). Answers to questions prompted using the Ask() method
@@ -121,10 +119,6 @@ func splitAtLastNewLine(s string) (string, string) {
 }
 
 func (l *Logger) AskSelect(question string, choices []string) (string, error) {
-	if l.Mode == flags.ModeJson {
-		return "", errors.New("question prompts are not supported in json mode")
-	}
-
 	// Promptui does not support multiline prompts. So we split the question.
 	first, last := splitAtLastNewLine(question)
 	_, err := l.Writer.Write([]byte(first))
@@ -150,10 +144,6 @@ func (l *Logger) AskSelect(question string, choices []string) (string, error) {
 }
 
 func (l *Logger) Ask(question, defaultVal string) (string, error) {
-	if l.Mode == flags.ModeJson {
-		return "", errors.New("question prompts are not supported in json mode")
-	}
-
 	// Add default value to question prompt.
 	if defaultVal != "" {
 		question += fmt.Sprintf(` [%s]`, defaultVal)
@@ -180,34 +170,9 @@ func (l *Logger) Ask(question, defaultVal string) (string, error) {
 	return ans, nil
 }
 
-func (l *Logger) writeJson(event Event) {
-	b, err := json.MarshalIndent(event, "", "  ")
-	if err != nil {
-		// we panic because there we cannot catch this in jobs.RunNowAndWait
-		panic(err)
-	}
-	_, _ = l.Writer.Write(b)
-	_, _ = l.Writer.Write([]byte("\n"))
-}
-
-func (l *Logger) writeAppend(event Event) {
-	_, _ = l.Writer.Write([]byte(event.String()))
-	_, _ = l.Writer.Write([]byte("\n"))
-}
-
 func (l *Logger) Log(event Event) {
 	l.mutex.Lock()
 	defer l.mutex.Unlock()
-	switch l.Mode {
-	case flags.ModeJson:
-		l.writeJson(event)
-
-	case flags.ModeAppend:
-		l.writeAppend(event)
-
-	default:
-		// we panic because errors are not captured in some log sides like
-		// jobs.RunNowAndWait
-		panic("unknown progress logger mode: " + l.Mode.String())
-	}
+	_, _ = l.Writer.Write([]byte(event.String()))
+	_, _ = l.Writer.Write([]byte("\n"))
 }

libs/cmdio/logger_test.go

@@ -1,27 +1,11 @@
 package cmdio
 
 import (
-	"context"
 	"testing"
 
-	"github.com/databricks/cli/libs/flags"
 	"github.com/stretchr/testify/assert"
 )
 
-func TestAskFailedInJsonMode(t *testing.T) {
-	l := NewLogger(flags.ModeJson)
-	_, err := l.Ask("What is your spirit animal?", "")
-	assert.ErrorContains(t, err, "question prompts are not supported in json mode")
-}
-
-func TestAskChoiceFailsInJsonMode(t *testing.T) {
-	l := NewLogger(flags.ModeJson)
-	ctx := NewContext(context.Background(), l)
-
-	_, err := AskSelect(ctx, "what is a question?", []string{"b", "c", "a"})
-	assert.EqualError(t, err, "question prompts are not supported in json mode")
-}
-
 func TestSplitAtLastNewLine(t *testing.T) {
 	first, last := splitAtLastNewLine("hello\nworld")
 	assert.Equal(t, "hello\n", first)

libs/flags/progress_format.go

@@ -11,7 +11,6 @@ type ProgressLogFormat string
 
 var (
 	ModeAppend  = ProgressLogFormat("append")
-	ModeJson    = ProgressLogFormat("json")
 	ModeDefault = ProgressLogFormat("default")
 )
 
@@ -28,19 +27,13 @@ func (p *ProgressLogFormat) Set(s string) error {
 	switch lower {
 	case ModeAppend.String():
 		*p = ProgressLogFormat(ModeAppend.String())
-	case ModeJson.String():
-		*p = ProgressLogFormat(ModeJson.String())
 	case ModeDefault.String():
 		// We include ModeDefault here for symmetry reasons so this flag value
 		// can be unset after test runs. We should not point this value in error
 		// messages though since it's internal only
-		*p = ProgressLogFormat(ModeJson.String())
+		*p = ProgressLogFormat(ModeAppend.String())
 	default:
-		valid := []string{
-			ModeAppend.String(),
-			ModeJson.String(),
-		}
-		return fmt.Errorf("accepted arguments are [%s]", strings.Join(valid, ", "))
+		return fmt.Errorf("accepted arguments are [%s]", ModeAppend.String())
 	}
 	return nil
 }
@@ -53,6 +46,5 @@ func (p *ProgressLogFormat) Type() string {
 func (p *ProgressLogFormat) Complete(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) {
 	return []string{
 		"append",
-		"json",
 	}, cobra.ShellCompDirectiveNoFileComp
 }

libs/flags/progress_format_test.go

@@ -16,20 +16,7 @@ func TestProgressFormatSet(t *testing.T) {
 
 	// invalid arg
 	err := p.Set("foo")
-	assert.ErrorContains(t, err, "accepted arguments are [append, json]")
-
-	// set json
-	err = p.Set("json")
-	assert.NoError(t, err)
-	assert.Equal(t, "json", p.String())
-
-	err = p.Set("JSON")
-	assert.NoError(t, err)
-	assert.Equal(t, "json", p.String())
-
-	err = p.Set("Json")
-	assert.NoError(t, err)
-	assert.Equal(t, "json", p.String())
+	assert.ErrorContains(t, err, "accepted arguments are [append]")
 
 	// set append
 	err = p.Set("append")