Enable silent log-level for hiding INFO messages #1308

afbjorklund · 2023-01-20T17:10:45Z

For a normal run, with downloads already cached, this will only output the message (if any)

jandubois · 2023-01-20T19:59:16Z

Thanks, LGTM, but can we discuss if "silent" shouldn't be the default, and --info would be an option to switch the info-level output back on?

@AkihiroSuda WDYT?

cmd/limactl/main.go

afbjorklund · 2023-01-21T15:30:17Z

minikube saves all logging output to a file, unless you give an explicit option: --alsologtostderr

For the regular progress output, it goes all in on emojis (unless you disable them). It divides the crowd.

jandubois · 2023-01-23T04:01:03Z

minikube saves all logging output to a file, unless you give an explicit option: --alsologtostderr

I always found this a bit unconventional; but I can see the advantage for something that runs a little longer (like limactl start). It allows you to get the full log after the command failed without having to run it again.

For the regular progress output, it goes all in on emojis (unless you disable them). It divides the crowd.

Personally I like the minikube emoji, but I understand that some people feel differently. I do find it challenging to pick meaningful emoji for some status messages though.

afbjorklund · 2023-02-11T08:47:01Z

As noted elsewhere, it also needs a fix for the URL that it is downloading (by default only showing progress bar)

pkg/fileutils/download.go:      logrus.WithField("digest", f.Digest).Infof("Attempting to download %s from %q", description, f.Location)
pkg/fileutils/download.go-      res, err := downloader.Download(dest, f.Location,
pkg/fileutils/download.go-              downloader.WithCache(),
pkg/fileutils/download.go-              downloader.WithExpectedDigest(f.Digest),
pkg/fileutils/download.go-      )
--
pkg/start/start.go:             logrus.WithField("digest", f.Digest).Infof("Attempting to download the nerdctl archive from %q", f.Location)
pkg/start/start.go-             res, err := downloader.Download("", f.Location, downloader.WithCache(), downloader.WithExpectedDigest(f.Digest))

The suggestion is to just print the URL on a separate line, maybe then log as debug (so that is not too redundant).

https://github.com/containerd/nerdctl/releases/download/v1.1.0/nerdctl-full-1.1.0-linux-amd64.tar.gz
735.38 KiB / 224.27 MiB [>__________________________________] 0.32% 506.65 KiB/s

INFO[0002] Attempting to download the nerdctl archive from "https://github.com/containerd/nerdctl/releases/download/v1.1.0/nerdctl-full-1.1.0-linux-amd64.tar.gz" digest="sha256:5440c7b3af63df2ad2c98e185e06a27b4a21eea334b05408e84f8502251d9459"

Show a user-readable description of the download #1407

Changed it to not show the full url by default, only the base name (and description)

https://github.com/containerd/nerdctl/releases/download/v1.1.0/nerdctl-full-1.1.0-linux-amd64.tar.gz

Downloading the nerdctl archive (nerdctl-full-1.1.0-linux-amd64.tar.gz)

afbjorklund · 2023-03-07T19:50:13Z

Added --log-level

jandubois · 2023-04-06T01:42:38Z

cmd/limactl/main.go

+		silent, _ := cmd.Flags().GetBool("silent")
+		if silent {
+			logrus.SetLevel(logrus.WarnLevel)
+		}


I agree with @jbergstroem that silent should mean "no output", so the log level should be "Error" and not "Warn".

But if this option is just an alias for --log-level error, then I would rather not have it at all. If we have it as a distinct option, then it also needs to suppress of progress bars, and the message output.

If it was just an alias, then I would rather have separate --debug, --info, --warn, and --error flags.

It was just an alias, like the --debug. I guess that --quiet would have been better.

But I only needed a way to hide the log spam, a log file would also have worked...

I guess that --quiet would have been better.

To me the issue is not (just) the name, but also that it is redundant. And it isn't even clear what it does: is --quiet the same as --log-level warn or --log-level error?

Or does it something else, and should be combined with --log-level because it really quiets down stuff that is not a log entry, like the download progress bars, or the message from lima.yaml?

It was just an alias, like the --debug.

I would have objected to adding --debug if we already had --log-level, but now we want to keep it for backwards compatibility.

As I mentioned before, we should have either --trace, --debug, --info, --warn, and --error options, or --log-level, but not both. But even if we wanted to have both, why would the shortcut be called --quiet or --silent and not --warn or --error to make it clear what they do?

Just for the record, I prefer to have --log-level over individual options, if for no other reason than to keep the --help output tidy.

Having redundant options has a cost, especially to new users: they wonder what the differences are, and then have to look at the documentation. It also clutters up the --help output:

--debug debug mode --log-level string Set the logging level [trace, debug, info, warn, error, fatal, panic] --silent silent mode

We now have 3 options just to set a log level, when 1 would be sufficient.

So unless --silent/--quiet provides additional functionality, I'm opposed to adding it.

Maybe this is a separate issue, but I would not expect this output:

$ limactl start --silent --tty=false template://docker > lima.log 31.66 MiB / 647.50 MiB (4.89%) ? p/s 68.80 MiB / 647.50 MiB (10.62%) 7.43 MiB/s 101.78 MiB / 647.50 MiB (15.72%) 7.37 MiB/s 139.12 MiB / 647.50 MiB (21.49%) 7.38 MiB/s 173.44 MiB / 647.50 MiB (26.79%) 7.35 MiB/s 209.23 MiB / 647.50 MiB (32.31%) 7.34 MiB/s 246.92 MiB / 647.50 MiB (38.13%) 7.35 MiB/s 282.62 MiB / 647.50 MiB (43.65%) 7.33 MiB/s 317.83 MiB / 647.50 MiB (49.09%) 7.32 MiB/s 355.36 MiB / 647.50 MiB (54.88%) 7.33 MiB/s 390.67 MiB / 647.50 MiB (60.34%) 7.31 MiB/s 426.27 MiB / 647.50 MiB (65.83%) 7.30 MiB/s 464.20 MiB / 647.50 MiB (71.69%) 7.32 MiB/s 499.06 MiB / 647.50 MiB (77.08%) 7.29 MiB/s 533.50 MiB / 647.50 MiB (82.39%) 7.27 MiB/s 570.41 MiB / 647.50 MiB (88.09%) 7.28 MiB/s 605.97 MiB / 647.50 MiB (93.59%) 7.27 MiB/s 643.31 MiB / 647.50 MiB (99.35%) 7.28 MiB/s 647.50 MiB / 647.50 MiB (100.00%) 7.56 MiB/s

Or without redirecting STDOUT:

$ limactl start --silent --tty=false template://docker 647.50 MiB / 647.50 MiB [-----------------------------------] 100.00% 7.16 MiB/s To run `docker` on the host (assumes docker-cli is installed), run the following commands: ------ docker context create lima-docker --docker "host=unix:///Users/jan/.lima/docker/sock/docker.sock" docker context use lima-docker docker run hello-world ------

Now there is only a single (new) option, hopefully it should be less surprising ?

--log-level string Set the logging level [trace, debug, info, warn, error]

afbjorklund · 2023-04-06T08:34:36Z

Now --quiet

Signed-off-by: Anders F Björklund <[email protected]>

afbjorklund · 2023-04-07T07:34:07Z

I removed the short forms, so now one needs to use the long way of hiding INFO messages:

--log-level WARN

The old debug flag remains, it could (potentially) do more things than just change log level ?

--log-level DEBUG

jandubois

Thanks, LGTM

afbjorklund force-pushed the silent branch from 458b7fc to dac7c7a Compare January 20, 2023 17:11

AkihiroSuda reviewed Jan 21, 2023

View reviewed changes

cmd/limactl/main.go Outdated Show resolved Hide resolved

afbjorklund mentioned this pull request Jan 21, 2023

Add progress option for showing progress only #1312

Closed

jandubois mentioned this pull request Feb 15, 2023

DNS logging should be opt-in #1367

Closed

afbjorklund force-pushed the silent branch from dac7c7a to 2ffaecf Compare March 7, 2023 19:49

AkihiroSuda requested a review from jandubois April 5, 2023 23:39

jandubois requested changes Apr 6, 2023

View reviewed changes

afbjorklund changed the title ~~Enable silent mode for hiding INFO messages~~ Enable quiet mode for hiding INFO messages Apr 6, 2023

Add flag for setting log-level explicitly

8fa03ca

Signed-off-by: Anders F Björklund <[email protected]>

afbjorklund force-pushed the silent branch from 4f194b5 to 8fa03ca Compare April 7, 2023 07:31

afbjorklund changed the title ~~Enable quiet mode for hiding INFO messages~~ Enable silent log-level for hiding INFO messages Apr 7, 2023

AkihiroSuda approved these changes Apr 7, 2023

View reviewed changes

AkihiroSuda added this to the v0.15.1 milestone Apr 7, 2023

AkihiroSuda requested a review from jandubois April 7, 2023 10:11

jandubois approved these changes Apr 10, 2023

View reviewed changes

jandubois merged commit a080148 into lima-vm:master Apr 10, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Enable silent log-level for hiding INFO messages #1308

Enable silent log-level for hiding INFO messages #1308

Uh oh!

afbjorklund commented Jan 20, 2023 •

edited

Loading

Uh oh!

jandubois commented Jan 20, 2023

Uh oh!

Uh oh!

afbjorklund commented Jan 21, 2023

Uh oh!

jandubois commented Jan 23, 2023

Uh oh!

afbjorklund commented Feb 11, 2023 •

edited

Loading

Uh oh!

afbjorklund commented Mar 7, 2023

Uh oh!

jandubois Apr 6, 2023

Uh oh!

afbjorklund Apr 6, 2023 •

edited

Loading

Uh oh!

jandubois Apr 6, 2023

Uh oh!

afbjorklund Apr 7, 2023

Uh oh!

afbjorklund commented Apr 6, 2023

Uh oh!

afbjorklund commented Apr 7, 2023

Uh oh!

jandubois left a comment

Uh oh!

Uh oh!

Enable silent log-level for hiding INFO messages #1308

Enable silent log-level for hiding INFO messages #1308

Uh oh!

Conversation

afbjorklund commented Jan 20, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jandubois commented Jan 20, 2023

Uh oh!

Uh oh!

afbjorklund commented Jan 21, 2023

Uh oh!

jandubois commented Jan 23, 2023

Uh oh!

afbjorklund commented Feb 11, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

afbjorklund commented Mar 7, 2023

Uh oh!

jandubois Apr 6, 2023

Choose a reason for hiding this comment

Uh oh!

afbjorklund Apr 6, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jandubois Apr 6, 2023

Choose a reason for hiding this comment

Uh oh!

afbjorklund Apr 7, 2023

Choose a reason for hiding this comment

Uh oh!

afbjorklund commented Apr 6, 2023

Uh oh!

afbjorklund commented Apr 7, 2023

Uh oh!

jandubois left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

afbjorklund commented Jan 20, 2023 •

edited

Loading

afbjorklund commented Feb 11, 2023 •

edited

Loading

afbjorklund Apr 6, 2023 •

edited

Loading