Add new logging structure. Fix cockpit. #565
Conversation
|
@cdrage - check out the proposed implementation and see what you think. I have one unit test failing so I'll need to track that down. |
|
@cdrage. I think I picked up all of the cockpit logging changes from your PR. Do you mind going through and making sure? |
| logger.info("Action/Mode Selected is: %s" % args.action) | ||
|
|
||
| # Finally, parse args and give error if necessary | ||
| logger.debug("Final parsed cmdline: {}".format(' '.join(cmdline))) | ||
| args = self.parser.parse_args(cmdline) |
There was a problem hiding this comment.
this needs to be moved up before setup_logging, passing -v doesn't work since we haven't parsed args yet
There was a problem hiding this comment.
hmm. so you are saying the logger.debug() statement needs to be moved before setup_logging?
There was a problem hiding this comment.
I think I see what you are saying now. You are saying that self.parser.parse_args() needs to be moved above the setup_logging() call. I don't think this is true because the call to self.parser.parse_known_args() should have captured --verbose from the cmdline. I'll run some tests to see if it changes things.
There was a problem hiding this comment.
Moving it down did make the test pass. Now just need to figure out what part of my logic is flawed in the first part.
There was a problem hiding this comment.
@dustymabe Yeah, I'm not able to receive any debug information at all by passing -v or --verbose
|
Test is failing due to my comment about no verbose output ^^ |
| # If connected to a tty, then default to color, else, no color | ||
| if not outputtype: | ||
| if sys.stdout.isatty(): | ||
| outputtype = 'stdoutcolor' |
There was a problem hiding this comment.
I wonder if we should use something shorter for the cli that would make sense for a variable name as well. would --logtype on the cli work? then here we could use logtype as the var name.
|
@dustymabe I do have to agree with you, this looks a lot better than what I had traditional proposed. Other than the names (use cockpit_logging? and my previous comment) it looks good. I'll go ahead and test it a bit more too. |
dustymabe
force-pushed
the
dusty-logging-refactor
branch
from
f909aab to
f83c085
Compare
| # If no logtype was set then let's have a sane default | ||
| # If connected to a tty, then default to color, else, no color | ||
| if not logtype: | ||
| if sys.stdout.isatty(): |
Adds colorized output as well as a new command line option for choosing between different logging choices. Also adds support for a 'cockpit' logging mode, which just prints out select logging messages for the cockpit UI. Fixes projectatomic#134
Add in messages that the cockpit UI is concerned with. Also remove old print* functions from utils as they are no longer needed. Fixes projectatomic#432
dustymabe
force-pushed
the
dusty-logging-refactor
branch
from
f83c085 to
86e87be
Compare
|
LGTM 👍 |
Add new logging structure. Fix cockpit.
No description provided.