Adar Dembo has posted comments on this change.

Change subject: [tools]: Keep the verbosity of CLI at WARNING and above
......................................................................


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/4447/1/src/kudu/tools/tool_main.cc
File src/kudu/tools/tool_main.cc:

PS1, Line 215:   // Set the verbosity of the commands to WARNING(2) and above.
             :   // If the user had explicitly specified verbosity, then user's
             :   // verbosity level is honored. Since '--v' depends on 
minloglevel
             :   // specifying either of them on CLI will override this setting.
             :   if 
(google::GetCommandLineFlagInfoOrDie("minloglevel").is_default &&
             :       google::GetCommandLineFlagInfoOrDie("v").is_default) {
             :     google::SetCommandLineOption("minloglevel",
             :                                  
SimpleItoa(google::GLOG_WARNING).c_str());
             :   }
Couple of comments:
1. Can you move this into ParseCommandLineFlags, since it's  doing some parsing 
of its own?
2. I still think that the default logging behavior should be to log absolutely 
nothing. Maybe FATAL or higher so that _crashes_ are logged, but that's about 
it. This is because during regular operations (i.e. outside of crashes), every 
action returns a detailed Status which gets logged already. That Status should 
be enough to diagnose simple errors. If not, the user can rerun the action with 
--v or --minloglevel set to something else.
3. Ideally we'd show --v and --minloglevel on every action's help, but adding 
it to every action manually is a pain in the butt. This is where "inheriting" 
parameters from modes in the mode chain would be a nice feature to have. You 
don't need to implement that, though; I'm just thinking out loud.


-- 
To view, visit http://gerrit.cloudera.org:8080/4447
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I882a340d4c1d205e4e998c888f487b7185000e3c
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dinesh Bhat <din...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <a...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <aser...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <dral...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes

Reply via email to