Closed tych0 closed 4 years ago
So here's my proposal:
So here's my proposal:
- add a --log-level flag for stacker logs
+1
- change all Println()s, etc. to stackerlog.Info() or stackerlog.Debug(), filtering things with something like tych0@afab53e
+1
- add a --log-deps flags for all log deps (like umoci)
couldn't really parse this, is it "add a log-deps flag to show the logs for any deps that generate logs"?
- add a --progress=true flag, which either prints or omits the progress for things like downloading images or downloading imports over http. default to on
+1
- all of the above goes to stderr
- only the run: section goes to stdout
+0 I don't have a strong opinion. I kind of feel like logs should also go to stdout, but 🤷♀️
couldn't really parse this, is it "add a log-deps flag to show the logs for any deps that generate logs"?
Yeah, exactly. Although I suppose stacker developers are the only people who care about these things, and I've never found them at all useful. So maybe it's not worth the code.
So here's my proposal:
1. add a --log-level flag for stacker logs
+1
2. change all Println()s, etc. to stackerlog.Info() or stackerlog.Debug(), filtering things with something like [tych0@afab53e](https://github.com/tych0/stacker/commit/afab53efdd2242e879952df3926fe95eee5cbb69)
+1. You chose 2 levels here, Info and Debug. Not sure if it was intentional or not, but those line up with this blog that I mostly agree with.
3. add a --log-deps flags for all log deps (like umoci)
Seems fine. I agree that it is nice to turn logging on or off for "dependencies". While this isn't terribly fine grained its a good start, and allows us to get logging turned on and not a flood of debug from all libraries.
4. add a --progress=true flag, which either prints or omits the progress for things like downloading images or downloading imports over http. default to on 5. all of the above goes to stderr
+1
6. only the run: section goes to stdout
I think by default just leave run section stdout on stdout, and stderr on stderr.
If the user wants to separate run section's output/error from logging, then they have to pass '--log-file=
When I went asking about this, ravi suggested context for logging. https://blog.gopheracademy.com/advent-2016/context-logging/
Although Dave Cheny's blog that I admired above suggests Context is not for logging why can't anything be easy.
You chose 2 levels here, Info and Debug. Not sure if it was intentional or not, but those line up with this blog that I mostly agree with.
Yes, I definitely agree with the points about Error and Fatal. There are some warnings in the stacker codebase now, but I think those are probably code smells that should get refactored into errors.
When I went asking about this, ravi suggested context for logging.
I find threading a context through everywhere quite annoying; stacker swallows it in quite a few places. So I agree :)
If the user wants to separate run section's output/error from logging, then they have to pass '--log-file='.
Yeah, sounds good.
I believe all of these are done, so I'm going to close this. Thanks for the input everyone.
Right now, stacker's logging is mostly just Printf()s, which are hard to tune. Let's use some real logging, with levels.
Let's also make sure that unwanted log messages from libraries (like umoci) don't show up.