Fixed prune with a confirmation, a force, and removed the spinner #145
Conversation
commands/prune.go
Outdated
| } | ||
| cmd.out.Info("Unused Docker images, containers, volumes, and networks cleaned up.") | ||
| } else { | ||
| cmd.out.Info("Cleanup aborted.") |
There was a problem hiding this comment.
Should this be cmd.out.Warn()? (This is particularly useful if we run a spinner to differentiate no-action from successful action.)
commands/prune.go
Outdated
|
|
||
| if util.AskYesNo("Are you sure you want to remove all unused containers, networks, images, caches, and volumes?") { | ||
| /* #nosec */ | ||
| if exitCode := util.PassthruCommand(exec.Command("docker", "system", "prune", "--all", "--volumes", "--force")); exitCode != 0 { |
There was a problem hiding this comment.
Should we start a spinner inside here in case the command might take awhile to run?
There was a problem hiding this comment.
If we kept the spinner I think it would still go above this conditional, just inside the AskYesNo block. I'm in favor of it since it can definitely take a while to run and there is no real indication that things are happening. If we leave it off then perhaps just mention in the prompt that this may take a while to execute.
There was a problem hiding this comment.
Well, the actual prune command has a lot of output as it is cleaning up things, so the spinner will likely get in the way. I'll test it out to see.
I'm thinking no spinner, but I'll see what it looks like.
There was a problem hiding this comment.
I just did a test and I didn't see any output (so if some is being generated then the spinner is gobbling it). Interestingly, I didn't see the success message until I hit "Enter" while the spinner was running so I thought things might have actually gotten stuck. Interested to see if you get the same experience (and if so then perhaps the note that it'll will take a bit is a better course).
|
|
|
It worked for me though we still seem to have some weird conditions between when the spinner gets overwritten versus when it is augmented. See attached screenshot:
I don't know that those would be a big enough blocker to stop proceeding but it would be nice to figure out what is going on there (I think @grayside dug a good bit and this may be the best we can do). I'll also note that I had to update to go 1.10 to build it as nfpm now depends on a Format element in archive/tar that 1.9 doesn't seem to recognize. |
|
@grayside Do you have any thoughts here? I'm torn if the intermittent appending vs newlines (see "Deleted Containers/Images" vs "Deleted Volumes") . Do we care about these inconsistencies? Is it worth it for the spinner? or should we disable the spinner and just drop a quick message like "This may take a while...."? |
|
How much wait time is there when nothing is being output to the screen? If it takes awhile but doesn't leave the user waiting to see progress, I think we should skip the spinner. I find it jarring to mix the spinner with regular logs in the output for a single command execution, but prune is not a command we are ever likely to mix with something else, and if we do we can address it then. |
…lag for consistent run execution
No description provided.