You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "William Li (Code Review)" <ge...@cloudera.org> on 2017/04/26 21:50:14 UTC
[kudu-CR] [KUDU-754] add an environment variable for kudu client debugging to stderr
William Li has uploaded a new change for review.
http://gerrit.cloudera.org:8080/6736
Change subject: [KUDU-754] add an environment variable for kudu client debugging to stderr
......................................................................
[KUDU-754] add an environment variable for kudu client debugging to stderr
read environment variable "KUDU_CLIENT_VERBOSE", calls SetVerboseLogLevel to set the specific verbose level. The InitiGoogleLoggingSafeBasic() already sets the debugging to stderr.
Change-Id: Iab5c7c24395c25184489200283dd38da024c07bb
---
M src/kudu/client/client-test.cc
M src/kudu/client/client.cc
M src/kudu/client/client.h
3 files changed, 35 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/36/6736/1
--
To view, visit http://gerrit.cloudera.org:8080/6736
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Iab5c7c24395c25184489200283dd38da024c07bb
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: William Li <wi...@gmail.com>
[kudu-CR] [KUDU-754] add an environment variable for kudu client debugging to stderr
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change.
Change subject: [KUDU-754] add an environment variable for kudu client debugging to stderr
......................................................................
Patch Set 5: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/6736
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Iab5c7c24395c25184489200283dd38da024c07bb
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: William Li <wi...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Cam Mach <ca...@inspur.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: William Li <wi...@gmail.com>
Gerrit-HasComments: No
[kudu-CR] [KUDU-754] add an environment variable for kudu client debugging to stderr
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change.
Change subject: [KUDU-754] add an environment variable for kudu client debugging to stderr
......................................................................
Patch Set 2:
(8 comments)
http://gerrit.cloudera.org:8080/#/c/6736/2/src/kudu/client/client-internal.h
File src/kudu/client/client-internal.h:
PS2, Line 259: e
nit: please add period (.) in the end of the sentence.
PS2, Line 260: Client
nit: probably, it's worth dropping the 'Client' part since it's already in the 'client' namespace.
Also, what do you think about using more or less ubiquitous abbreviations (like in other parts of the source code: search for EnvVar) and name it just
SetVerboseLevelFromEnvVar() ?
The same for the kClientVerboseEnvironmentVariable.
Line 262: // Verbose log Environment Variable Name
ditto
PS2, Line 263: Client
nit: if it's already in the 'client' namespace, may be it's possible to drop that 'Client' part?
http://gerrit.cloudera.org:8080/#/c/6736/2/src/kudu/client/client-test.cc
File src/kudu/client/client-test.cc:
Line 5057: ASSERT_EQ(5, FLAGS_v);
Does it make sense to add a case for negative and non-parsable values to verify that current FLAGS_v value is not touched?
http://gerrit.cloudera.org:8080/#/c/6736/2/src/kudu/client/client.cc
File src/kudu/client/client.cc:
Line 21: #include <memory>
nit: please add #include <cstdlib> since std::getenv() is from that header.
PS2, Line 151: NULL
nit: nullptr
PS2, Line 155: environment variable
nit: does it make sense to output the name of the variable?
--
To view, visit http://gerrit.cloudera.org:8080/6736
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Iab5c7c24395c25184489200283dd38da024c07bb
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: William Li <wi...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: William Li <wi...@gmail.com>
Gerrit-HasComments: Yes
[kudu-CR] [KUDU-754] add an environment variable for kudu client debugging to stderr
Posted by "William Li (Code Review)" <ge...@cloudera.org>.
William Li has posted comments on this change.
Change subject: [KUDU-754] add an environment variable for kudu client debugging to stderr
......................................................................
Patch Set 2:
(12 comments)
new patch has been submitted.
http://gerrit.cloudera.org:8080/#/c/6736/1//COMMIT_MSG
Commit Message:
Line 9: Read environment variable "KUDU_CLIENT_VERBOSE" to get verbose level.
> nit: can you re-wrap and format this into full sentences with appropriate c
Done
http://gerrit.cloudera.org:8080/#/c/6736/1/src/kudu/client/client-test.cc
File src/kudu/client/client-test.cc:
Line 5049:
> nit: whitespace here and below
Done
Line 5050: // Test that, log verbose level can be set through environment varialble
> nit: please wrap (here and a few lines down as well)
Done
Line 5053: FLAGS_v = 0;
> no need for a separate variable here. I think it's clear what you're doing
Done
Line 5059:
> I think it's fine to just use '5' here literal instead of std::atoi() it ab
Done
http://gerrit.cloudera.org:8080/#/c/6736/1/src/kudu/client/client.cc
File src/kudu/client/client.cc:
PS1, Line 148: etClie
> this isn't a constant, so it should just be 'int level'
Done
PS1, Line 149: int32_t level = 0
> const char*?
Done
Line 151: if (env_verbose_level != NULL) {
> style: { goes on the same line as 'if'. Same below.
Done
Line 152: if (safe_strto32(env_verbose_level, &level) && (level >=0)) {
> use safe_strto32() from numbers.h instead, so you can check validity. Maybe
Done
PS1, Line 153: el(level);
> why <= 6? Is there some limit on the vlog level as far as glog is concerned
I was reading the comments from client.h at line 110 paragraph: ".. the highest verbosity level used in Kudu is 6, ....". I leave it open for the upper bound for now. If later you want to pick a big enough number to just set a limitation, we can do that too.
http://gerrit.cloudera.org:8080/#/c/6736/1/src/kudu/client/client.h
File src/kudu/client/client.h:
PS1, Line 125: /// Set signal number to use internally.
: ///
> given this is called by the client during initialization, I don't think it'
Done
Line 129: /// this advanced API can help workaround conflicts.
> This should probably be declared extern in the header, and defined in the .
Done
--
To view, visit http://gerrit.cloudera.org:8080/6736
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Iab5c7c24395c25184489200283dd38da024c07bb
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: William Li <wi...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: William Li <wi...@gmail.com>
Gerrit-HasComments: Yes
[kudu-CR] [KUDU-754] add an environment variable for kudu client debugging to stderr
Posted by "William Li (Code Review)" <ge...@cloudera.org>.
William Li has posted comments on this change.
Change subject: [KUDU-754] add an environment variable for kudu client debugging to stderr
......................................................................
Patch Set 4:
(9 comments)
http://gerrit.cloudera.org:8080/#/c/6736/2/src/kudu/client/client-internal.h
File src/kudu/client/client-internal.h:
PS2, Line 259: e
> nit: please add period (.) in the end of the sentence.
Done
PS2, Line 260: Verbos
> nit: probably, it's worth dropping the 'Client' part since it's already in
Done
Line 262:
> ditto
Done
PS2, Line 263: t
> nit: if it's already in the 'client' namespace, may be it's possible to dro
Done
http://gerrit.cloudera.org:8080/#/c/6736/2/src/kudu/client/client-test.cc
File src/kudu/client/client-test.cc:
Line 5057: // Test the client retries on ServiceUnavailable errors. This scenario uses
> Does it make sense to add a case for negative and non-parsable values to ve
Done
http://gerrit.cloudera.org:8080/#/c/6736/2/src/kudu/client/client.cc
File src/kudu/client/client.cc:
Line 21: #include <cstdlib>
> nit: please add #include <cstdlib> since std::getenv() is from that header
Done
PS2, Line 151: evel
> nit: nullptr
Done
PS2, Line 155:
> nit: does it make sense to output the name of the variable?
Done
http://gerrit.cloudera.org:8080/#/c/6736/3/src/kudu/client/client.cc
File src/kudu/client/client.cc:
Line 25: #include <unordered_map>
> warning: #includes are not sorted properly [llvm-include-order]
Done
--
To view, visit http://gerrit.cloudera.org:8080/6736
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Iab5c7c24395c25184489200283dd38da024c07bb
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: William Li <wi...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: William Li <wi...@gmail.com>
Gerrit-HasComments: Yes
[kudu-CR] [KUDU-754] add an environment variable for kudu client debugging to stderr
Posted by "William Li (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/6736
to look at the new patch set (#5).
Change subject: [KUDU-754] add an environment variable for kudu client debugging to stderr
......................................................................
[KUDU-754] add an environment variable for kudu client debugging to stderr
Read environment variable "KUDU_CLIENT_VERBOSE" to get verbose level.
Calls SetVerboseLogLevel to set the specific verbose level.
Notes: The InitGoogleLoggingSafeBasic() already sets the debugging to stderr.
Change-Id: Iab5c7c24395c25184489200283dd38da024c07bb
---
M src/kudu/client/client-internal.h
M src/kudu/client/client-test.cc
M src/kudu/client/client.cc
3 files changed, 43 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/36/6736/5
--
To view, visit http://gerrit.cloudera.org:8080/6736
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Iab5c7c24395c25184489200283dd38da024c07bb
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: William Li <wi...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Cam Mach <ca...@inspur.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: William Li <wi...@gmail.com>
[kudu-CR] [KUDU-754] add an environment variable for kudu client debugging to stderr
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change.
Change subject: [KUDU-754] add an environment variable for kudu client debugging to stderr
......................................................................
Patch Set 4:
(4 comments)
Looks good, thanks for the contribution! Just passing through with a couple style nits.
http://gerrit.cloudera.org:8080/#/c/6736/4//COMMIT_MSG
Commit Message:
PS4, Line 11: Initi
Nit: Init
http://gerrit.cloudera.org:8080/#/c/6736/4/src/kudu/client/client.cc
File src/kudu/client/client.cc:
Line 60: #include "kudu/gutil/strings/numbers.h"
Nit: should come before substitute.h (alphabetical sorting).
PS4, Line 153: >=0
Nit: separate >= from 0 with a space.
PS4, Line 156: << "."
Nit: no need to terminate LOG messages with a period.
--
To view, visit http://gerrit.cloudera.org:8080/6736
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Iab5c7c24395c25184489200283dd38da024c07bb
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: William Li <wi...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Cam Mach <ca...@inspur.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: William Li <wi...@gmail.com>
Gerrit-HasComments: Yes
[kudu-CR] [KUDU-754] add an environment variable for kudu client debugging to stderr
Posted by "William Li (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/6736
to look at the new patch set (#3).
Change subject: [KUDU-754] add an environment variable for kudu client debugging to stderr
......................................................................
[KUDU-754] add an environment variable for kudu client debugging to stderr
Read environment variable "KUDU_CLIENT_VERBOSE" to get verbose level.
Calls SetVerboseLogLevel to set the specific verbose level.
Notes: The InitiGoogleLoggingSafeBasic() already sets the debugging to stderr.
Change-Id: Iab5c7c24395c25184489200283dd38da024c07bb
---
M src/kudu/client/client-internal.h
M src/kudu/client/client-test.cc
M src/kudu/client/client.cc
3 files changed, 43 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/36/6736/3
--
To view, visit http://gerrit.cloudera.org:8080/6736
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Iab5c7c24395c25184489200283dd38da024c07bb
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: William Li <wi...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: William Li <wi...@gmail.com>
[kudu-CR] [KUDU-754] add an environment variable for kudu client debugging to stderr
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change.
Change subject: [KUDU-754] add an environment variable for kudu client debugging to stderr
......................................................................
Patch Set 5: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/6736
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Iab5c7c24395c25184489200283dd38da024c07bb
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: William Li <wi...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Cam Mach <ca...@inspur.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: William Li <wi...@gmail.com>
Gerrit-HasComments: No
[kudu-CR] [KUDU-754] add an environment variable for kudu client debugging to stderr
Posted by "William Li (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/6736
to look at the new patch set (#2).
Change subject: [KUDU-754] add an environment variable for kudu client debugging to stderr
......................................................................
[KUDU-754] add an environment variable for kudu client debugging to stderr
Read environment variable "KUDU_CLIENT_VERBOSE" to get verbose level.
Calls SetVerboseLogLevel to set the specific verbose level.
Notes: The InitiGoogleLoggingSafeBasic() already sets the debugging to stderr.
Change-Id: Iab5c7c24395c25184489200283dd38da024c07bb
---
M src/kudu/client/client-internal.h
M src/kudu/client/client-test.cc
M src/kudu/client/client.cc
3 files changed, 33 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/36/6736/2
--
To view, visit http://gerrit.cloudera.org:8080/6736
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Iab5c7c24395c25184489200283dd38da024c07bb
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: William Li <wi...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] [KUDU-754] add an environment variable for kudu client debugging to stderr
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change.
Change subject: [KUDU-754] add an environment variable for kudu client debugging to stderr
......................................................................
Patch Set 1:
(12 comments)
http://gerrit.cloudera.org:8080/#/c/6736/1//COMMIT_MSG
Commit Message:
Line 9: read environment variable "KUDU_CLIENT_VERBOSE", calls SetVerboseLogLevel to set the specific verbose level. The InitiGoogleLoggingSafeBasic() already sets the debugging to stderr.
nit: can you re-wrap and format this into full sentences with appropriate capitalization, etc? https://chris.beams.io/posts/git-commit/ has a good guide
http://gerrit.cloudera.org:8080/#/c/6736/1/src/kudu/client/client-test.cc
File src/kudu/client/client-test.cc:
Line 5049:
nit: whitespace here and below
Line 5050: // Test that, if a different verbose level through environment varialble is set, it reflects to the FLAGS_v
nit: please wrap (here and a few lines down as well)
Line 5053: char sample_verboseLevel[] = "5"; // select a specific verbose level different from 0
no need for a separate variable here. I think it's clear what you're doing so the comment can also be removed.
Line 5059: ASSERT_EQ(std::atoi(sample_verboseLevel), FLAGS_v);
I think it's fine to just use '5' here literal instead of std::atoi() it above, since the test is so short it doesn't really aid clarity
http://gerrit.cloudera.org:8080/#/c/6736/1/src/kudu/client/client.cc
File src/kudu/client/client.cc:
PS1, Line 148: kLevel
this isn't a constant, so it should just be 'int level'
PS1, Line 149: char const* temp
const char*?
Line 151: {
style: { goes on the same line as 'if'. Same below.
Line 152: kLevel = std::atoi(temp);
use safe_strto32() from numbers.h instead, so you can check validity. Maybe LOG(WARNING) if the env var is invalid?
PS1, Line 153: kLevel <= 6
why <= 6? Is there some limit on the vlog level as far as glog is concerned?
http://gerrit.cloudera.org:8080/#/c/6736/1/src/kudu/client/client.h
File src/kudu/client/client.h:
PS1, Line 125: /// Set the logging verbose level through environment variable
: void KUDU_EXPORT SetClientVerboseLevelFromEnvironmentVariable();
given this is called by the client during initialization, I don't think it's necessary to export it as a public API. If you put it here just for testing, you could move it to client-internal.h (which the tests are allowed to include but don't get shipped)
Line 129: static const char* kClientVerboseEnvironmentVariable = "KUDU_CLIENT_VERBOSE";
This should probably be declared extern in the header, and defined in the .cc file.
--
To view, visit http://gerrit.cloudera.org:8080/6736
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Iab5c7c24395c25184489200283dd38da024c07bb
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: William Li <wi...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] [KUDU-754] add an environment variable for kudu client debugging to stderr
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has submitted this change and it was merged.
Change subject: [KUDU-754] add an environment variable for kudu client debugging to stderr
......................................................................
[KUDU-754] add an environment variable for kudu client debugging to stderr
Read environment variable "KUDU_CLIENT_VERBOSE" to get verbose level.
Calls SetVerboseLogLevel to set the specific verbose level.
Notes: The InitGoogleLoggingSafeBasic() already sets the debugging to stderr.
Change-Id: Iab5c7c24395c25184489200283dd38da024c07bb
Reviewed-on: http://gerrit.cloudera.org:8080/6736
Tested-by: Kudu Jenkins
Reviewed-by: Adar Dembo <ad...@cloudera.com>
Reviewed-by: Alexey Serbin <as...@cloudera.com>
---
M src/kudu/client/client-internal.h
M src/kudu/client/client-test.cc
M src/kudu/client/client.cc
3 files changed, 43 insertions(+), 0 deletions(-)
Approvals:
Adar Dembo: Looks good to me, approved
Alexey Serbin: Looks good to me, approved
Kudu Jenkins: Verified
--
To view, visit http://gerrit.cloudera.org:8080/6736
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Iab5c7c24395c25184489200283dd38da024c07bb
Gerrit-PatchSet: 6
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: William Li <wi...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Cam Mach <ca...@inspur.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: William Li <wi...@gmail.com>
[kudu-CR] [KUDU-754] add an environment variable for kudu client debugging to stderr
Posted by "William Li (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/6736
to look at the new patch set (#4).
Change subject: [KUDU-754] add an environment variable for kudu client debugging to stderr
......................................................................
[KUDU-754] add an environment variable for kudu client debugging to stderr
Read environment variable "KUDU_CLIENT_VERBOSE" to get verbose level.
Calls SetVerboseLogLevel to set the specific verbose level.
Notes: The InitiGoogleLoggingSafeBasic() already sets the debugging to stderr.
Change-Id: Iab5c7c24395c25184489200283dd38da024c07bb
---
M src/kudu/client/client-internal.h
M src/kudu/client/client-test.cc
M src/kudu/client/client.cc
3 files changed, 43 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/36/6736/4
--
To view, visit http://gerrit.cloudera.org:8080/6736
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Iab5c7c24395c25184489200283dd38da024c07bb
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: William Li <wi...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: William Li <wi...@gmail.com>
[kudu-CR] [KUDU-754] add an environment variable for kudu client debugging to stderr
Posted by "William Li (Code Review)" <ge...@cloudera.org>.
William Li has posted comments on this change.
Change subject: [KUDU-754] add an environment variable for kudu client debugging to stderr
......................................................................
Patch Set 5:
(4 comments)
http://gerrit.cloudera.org:8080/#/c/6736/4//COMMIT_MSG
Commit Message:
PS4, Line 11: InitG
> Nit: Init
Done
http://gerrit.cloudera.org:8080/#/c/6736/4/src/kudu/client/client.cc
File src/kudu/client/client.cc:
Line 60: #include "kudu/gutil/strings/substitute.h"
> Nit: should come before substitute.h (alphabetical sorting).
Done
PS4, Line 153: >=
> Nit: separate >= from 0 with a space.
Done
PS4, Line 156: ;
> Nit: no need to terminate LOG messages with a period.
Done
--
To view, visit http://gerrit.cloudera.org:8080/6736
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Iab5c7c24395c25184489200283dd38da024c07bb
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: William Li <wi...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Cam Mach <ca...@inspur.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: William Li <wi...@gmail.com>
Gerrit-HasComments: Yes