From a2bde432e71c41e6ffce3ab5c7de99aedc56e5d0 Mon Sep 17 00:00:00 2001 From: Keepers Date: Mon, 24 Oct 2022 10:15:59 -0600 Subject: [PATCH] add progress bar toggle to cli flags (#1264) ## Description Adds a flag that allows users to toggle off the progress bar spinners. Primarily useful when running in a lower log level such as debug. ## Type of change - [x] :sunflower: Feature ## Issue(s) * #1112 ## Test Plan - [x] :muscle: Manual --- src/cli/cli.go | 5 +- src/internal/observe/observe.go | 73 ++++++++++++++++++++++++++-- src/internal/observe/observe_test.go | 12 ++--- src/pkg/logger/logger.go | 68 +++++++++++++------------- 4 files changed, 112 insertions(+), 46 deletions(-) diff --git a/src/cli/cli.go b/src/cli/cli.go index 31cecea22..b161be6e8 100644 --- a/src/cli/cli.go +++ b/src/cli/cli.go @@ -66,6 +66,7 @@ func BuildCommandTree(cmd *cobra.Command) { cmd.PersistentPostRunE = config.InitFunc() config.AddConfigFlags(cmd) logger.AddLogLevelFlag(cmd) + observe.AddProgressBarFlags(cmd) print.AddOutputFlag(cmd) options.AddGlobalOperationFlags(cmd) @@ -87,11 +88,11 @@ func BuildCommandTree(cmd *cobra.Command) { func Handle() { ctx := config.Seed(context.Background()) ctx = print.SetRootCmd(ctx, corsoCmd) - observe.SeedWriter(ctx, print.StderrWriter(ctx)) + observe.SeedWriter(ctx, print.StderrWriter(ctx), observe.PreloadFlags()) BuildCommandTree(corsoCmd) - ctx, log := logger.Seed(ctx) + ctx, log := logger.Seed(ctx, logger.PreloadLogLevel()) defer func() { _ = log.Sync() // flush all logs in the buffer }() diff --git a/src/internal/observe/observe.go b/src/internal/observe/observe.go index 58754b439..0b2d1656a 100644 --- a/src/internal/observe/observe.go +++ b/src/internal/observe/observe.go @@ -4,28 +4,79 @@ import ( "context" "fmt" "io" + "os" "sync" + "github.com/spf13/cobra" + "github.com/spf13/pflag" "github.com/vbauerster/mpb/v8" "github.com/vbauerster/mpb/v8/decor" ) -const progressBarWidth = 32 +const ( + noProgressBarsFN = "no-progress-bars" + progressBarWidth = 32 +) var ( wg sync.WaitGroup con context.Context writer io.Writer progress *mpb.Progress + cfg *config ) func init() { + cfg = &config{} + makeSpinFrames(progressBarWidth) } +// adds the persistent boolean flag --no-progress-bars to the provided command. +// This is a hack for help displays. Due to seeding the context, we also +// need to parse the configuration before we execute the command. +func AddProgressBarFlags(parent *cobra.Command) { + fs := parent.PersistentFlags() + fs.Bool(noProgressBarsFN, false, "turn off the progress bar displays") +} + +// Due to races between the lazy evaluation of flags in cobra and the need to init observer +// behavior in a ctx, these options get pre-processed manually here using pflags. The canonical +// AddProgressBarFlag() ensures the flags are displayed as part of the help/usage output. +func PreloadFlags() bool { + fs := pflag.NewFlagSet("seed-observer", pflag.ContinueOnError) + fs.ParseErrorsWhitelist.UnknownFlags = true + fs.Bool(noProgressBarsFN, false, "turn off the progress bar displays") + // prevents overriding the corso/cobra help processor + fs.BoolP("help", "h", false, "") + + // parse the os args list to find the log level flag + if err := fs.Parse(os.Args[1:]); err != nil { + return false + } + + // retrieve the user's preferred display + // automatically defaults to "info" + shouldHide, err := fs.GetBool(noProgressBarsFN) + if err != nil { + return false + } + + return shouldHide +} + +// --------------------------------------------------------------------------- +// configuration +// --------------------------------------------------------------------------- + +// config handles observer configuration +type config struct { + doNotDisplay bool +} + // SeedWriter adds default writer to the observe package. // Uses a noop writer until seeded. -func SeedWriter(ctx context.Context, w io.Writer) { +func SeedWriter(ctx context.Context, w io.Writer, hide bool) { writer = w con = ctx @@ -33,6 +84,10 @@ func SeedWriter(ctx context.Context, w io.Writer) { con = context.Background() } + cfg = &config{ + doNotDisplay: hide, + } + progress = mpb.NewWithContext( con, mpb.WithWidth(progressBarWidth), @@ -48,14 +103,18 @@ func Complete() { progress.Wait() } - SeedWriter(con, writer) + SeedWriter(con, writer, cfg.doNotDisplay) } +// --------------------------------------------------------------------------- +// Progress for Known Quantities +// --------------------------------------------------------------------------- + // ItemProgress tracks the display of an item by counting the bytes // read through the provided readcloser, up until the byte count matches // the totalBytes. func ItemProgress(rc io.ReadCloser, iname string, totalBytes int64) (io.ReadCloser, func()) { - if writer == nil || rc == nil || totalBytes == 0 { + if cfg.doNotDisplay || writer == nil || rc == nil || totalBytes == 0 { return rc, func() {} } @@ -77,6 +136,10 @@ func ItemProgress(rc io.ReadCloser, iname string, totalBytes int64) (io.ReadClos return bar.ProxyReader(rc), waitAndCloseBar(bar) } +// --------------------------------------------------------------------------- +// Progress for Unknown Quantities +// --------------------------------------------------------------------------- + var spinFrames []string // The bar width is set to a static 32 characters. The default spinner is only @@ -109,7 +172,7 @@ func makeSpinFrames(barWidth int) { // incrementing the count of items handled. Each write to the provided channel // counts as a single increment. The caller is expected to close the channel. func CollectionProgress(user, category, dirName string) (chan<- struct{}, func()) { - if writer == nil || len(user) == 0 || len(dirName) == 0 { + if cfg.doNotDisplay || writer == nil || len(user) == 0 || len(dirName) == 0 { ch := make(chan struct{}) go func(ci <-chan struct{}) { diff --git a/src/internal/observe/observe_test.go b/src/internal/observe/observe_test.go index 16b559e4a..912131ada 100644 --- a/src/internal/observe/observe_test.go +++ b/src/internal/observe/observe_test.go @@ -32,13 +32,13 @@ func (suite *ObserveProgressUnitSuite) TestItemProgress() { t := suite.T() recorder := strings.Builder{} - observe.SeedWriter(ctx, &recorder) + observe.SeedWriter(ctx, &recorder, false) defer func() { // don't cross-contaminate other tests. observe.Complete() //nolint:forbidigo - observe.SeedWriter(context.Background(), nil) + observe.SeedWriter(context.Background(), nil, false) }() from := make([]byte, 100) @@ -85,13 +85,13 @@ func (suite *ObserveProgressUnitSuite) TestCollectionProgress_unblockOnCtxCancel t := suite.T() recorder := strings.Builder{} - observe.SeedWriter(ctx, &recorder) + observe.SeedWriter(ctx, &recorder, false) defer func() { // don't cross-contaminate other tests. observe.Complete() //nolint:forbidigo - observe.SeedWriter(context.Background(), nil) + observe.SeedWriter(context.Background(), nil, false) }() progCh, closer := observe.CollectionProgress("test", "testcat", "testertons") @@ -120,13 +120,13 @@ func (suite *ObserveProgressUnitSuite) TestCollectionProgress_unblockOnChannelCl t := suite.T() recorder := strings.Builder{} - observe.SeedWriter(ctx, &recorder) + observe.SeedWriter(ctx, &recorder, false) defer func() { // don't cross-contaminate other tests. observe.Complete() //nolint:forbidigo - observe.SeedWriter(context.Background(), nil) + observe.SeedWriter(context.Background(), nil, false) }() progCh, closer := observe.CollectionProgress("test", "testcat", "testertons") diff --git a/src/pkg/logger/logger.go b/src/pkg/logger/logger.go index ef1ef14ba..03c6eb93e 100644 --- a/src/pkg/logger/logger.go +++ b/src/pkg/logger/logger.go @@ -8,8 +8,6 @@ import ( "github.com/spf13/pflag" "go.uber.org/zap" "go.uber.org/zap/zapcore" - - "github.com/alcionai/corso/src/cli/print" ) var ( @@ -29,13 +27,40 @@ const ( Production ) +const logLevelFN = "log-level" + // adds the persistent flag --log-level to the provided command. // defaults to "info". -// This is a hack for help displays. Due to seeding the context, we +// This is a hack for help displays. Due to seeding the context, we also // need to parse the log level before we execute the command. func AddLogLevelFlag(parent *cobra.Command) { fs := parent.PersistentFlags() - fs.StringVar(&llFlag, "log-level", "info", "set the log level to debug|info|warn|error") + fs.StringVar(&llFlag, logLevelFN, "info", "set the log level to debug|info|warn|error") +} + +// Due to races between the lazy evaluation of flags in cobra and the need to init logging +// behavior in a ctx, log-level gets pre-processed manually here using pflags. The canonical +// AddLogLevelFlag() ensures the flag is displayed as part of the help/usage output. +func PreloadLogLevel() string { + fs := pflag.NewFlagSet("seed-logger", pflag.ContinueOnError) + fs.ParseErrorsWhitelist.UnknownFlags = true + fs.String(logLevelFN, "info", "set the log level to debug|info|warn|error") + // prevents overriding the corso/cobra help processor + fs.BoolP("help", "h", false, "") + + // parse the os args list to find the log level flag + if err := fs.Parse(os.Args[1:]); err != nil { + return "info" + } + + // retrieve the user's preferred log level + // automatically defaults to "info" + levelString, err := fs.GetString(logLevelFN) + if err != nil { + return "info" + } + + return levelString } func genLogger(level logLevel) (*zapcore.Core, *zap.SugaredLogger) { @@ -119,38 +144,15 @@ const ctxKey loggingKey = "corsoLogger" // It also parses the command line for flag values prior to executing // cobra. This early parsing is necessary since logging depends on // a seeded context prior to cobra evaluating flags. -func Seed(ctx context.Context) (ctxOut context.Context, zsl *zap.SugaredLogger) { - level := Info - - // this func handles composing the return values whether or not an error occurs - defer func() { - zsl = singleton(level) - ctxOut = context.WithValue(ctx, ctxKey, zsl) - }() - - fs := pflag.NewFlagSet("seed-logger", pflag.ContinueOnError) - fs.ParseErrorsWhitelist.UnknownFlags = true - fs.String("log-level", "info", "set the log level to debug|info|warn|error") - // prevents overriding the corso/cobra help processor - fs.BoolP("help", "h", false, "") - - // parse the os args list to find the log level flag - if err := fs.Parse(os.Args[1:]); err != nil { - print.Err(ctx, err.Error()) - return +func Seed(ctx context.Context, lvl string) (context.Context, *zap.SugaredLogger) { + if len(lvl) == 0 { + lvl = "info" } - // retrieve the user's preferred log level - // automatically defaults to "info" - levelString, err := fs.GetString("log-level") - if err != nil { - print.Err(ctx, err.Error()) - return - } + zsl := singleton(levelOf(lvl)) + ctxOut := context.WithValue(ctx, ctxKey, zsl) - level = levelOf(levelString) - - return // return values handled in defer + return ctxOut, zsl } // SeedLevel embeds a logger into the context with the given log-level.