azriel91 / peace

Zero Stress Automation
https://peace.mk
Apache License 2.0
110 stars 1 forks source link

Allow progress bars to be toggled on/off per command #167

Closed azriel91 closed 9 months ago

azriel91 commented 9 months ago

Since the interruptibility change, all *Cmds display progress bars on the CLI.

The progress bars are noise for commands that complete near immediately. Compare:

Command with progress bars ```md ❯ ./envman status Using profile ⦗demo_1⦘ -- type `development` ⏳ 1. app_download ▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱ (el: 0s, eta: 0s) ⏳ 2. app_extract ▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱ (el: 0s, eta: 0s) ⏳ 3. iam_policy ▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱ (el: 0s, eta: 0s) ⏳ 4. iam_role ▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱ (el: 0s, eta: 0s) ⏳ 5. instance_profile ▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱ (el: 0s, eta: 0s) ⏳ 6. s3_bucket ▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱ (el: 0s, eta: 0s) ⏳ 7. s3_object ▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱▱ (el: 0s, eta: 0s) # Current States (Stored) 1. `app_download` : `azriel91/web_app/0.1.1/web_app.tar` non-existent, ETag not yet determined 2. `app_extract` : 0 files 3. `iam_policy` : does not exist 4. `iam_role` : does not exist 5. `instance_profile`: does not exist 6. `s3_bucket` : does not exist 7. `s3_object` : does not exist ```
Command without progress bars ```md ❯ ./envman status Using profile ⦗demo_1⦘ -- type `development` # Current States (Stored) 1. `app_download` : `azriel91/web_app/0.1.1/web_app.tar` non-existent, ETag not yet determined 2. `app_extract` : 0 files 3. `iam_policy` : does not exist 4. `iam_role` : does not exist 5. `instance_profile`: does not exist 6. `s3_bucket` : does not exist 7. `s3_object` : does not exist ```

Current State

The tricky part for envman is the code structure of:

Currently defined in that order, the user facing command definitions:

  1. Take in the CliOutput as a generic O: OutputWrite type parameter.
  2. Specify whether to print the current profile using profile_print: bool.

Goal State

Ideally, whichever entry point we have, it is the user facing *Cmds that define whether or not to render progress.

This could be achieved in the following ways:

  1. Restructuring the code such that the OutputWrite is instantiated within the user facing *Cmd, taking in whether Progress is rendered -- may be through a closure, so the *Cmd does not need to know whether it is a CliOutput or any other OutputWrite.
  2. Change Output to take in whether progress is enabled or disabled, either set on OutputWrite itself, or through CmdCtx.
  3. Change the user facing *Cmds that don't need progress, to pass through NoOpOutput.

Considerations

Other considerations:

  1. How do we turn off progress rendering for a web endpoint?

    From an API ergonomics perspective, this should be internal to the user facing *Cmd, which means the second and third options are favoured.

  2. Do we want to take in the same OutputWrite, then turn it off?

    Could be easier for a CmdCtx type alias with the OutputWrite masked within the alias -- in case the tool always uses one kind of OutputWrite.

  3. How about multiple OutputWrites? e.g. CLI output + logging, or web output + logging.

    At this point we might not even want O as a type parameter, and instead hold a Box<dyn OutputWrite> that delegates to a list of OutputWrites.

    We'll defer to #166 to solve this.

Solution Selection

We'll go with option 2.

Initially I chose option 3, to pass in a NoOpOutput for the near-immediate returning *Cmds.

However, we need the OutputWrite to not be NoOp for outcome. So we need to either go with option 2, or create a NoProgressOutputWrite which suppresses progress output. Between these two, the original option 2 is technically cleaner.

We should be able to turn progress off and on even after CmdCtx has been constructed, so it shouldn't (just?) be a CmdCtxBuilder method.

In the long term, having multiple outputs may be supported by calling cmd_ctx_builder.add_output(..) with as many OutputWrite implementations that are desired.

azriel91 commented 9 months ago

On further investigation, CmdExecutionBuilder already provides a with_progress_render_enabled flag, which we disabled within peace's States*ReadCmds and DiffCmd.