Todd Lipcon has posted comments on this change.

Change subject: rpc: improve error messages and logging for bad authentication
......................................................................


Patch Set 2:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/4764/2/src/kudu/rpc/sasl_client.cc
File src/kudu/rpc/sasl_client.cc:

PS2, Line 382: NotAuthorized
> Do we want to distinguish between 'not authorized' and 'not authenticated'?
Unfortunately adding new codes to Status is difficult because that would add 
new enums to the wire protocol. So, we more or less have to keep the ones we've 
got.


http://gerrit.cloudera.org:8080/#/c/4764/2/src/kudu/rpc/sasl_common.cc
File src/kudu/rpc/sasl_common.cc:

PS2, Line 214:   regex_t re;
             :   CHECK_EQ(0, regcomp(&re,
             :                       "generic failure: GSSAPI Error: "
             :                       "Unspecified GSS failure. +"
             :                       "Minor code may provide more information +"
             :                       "\\((.+)\\)", REG_EXTENDED));
             :   auto c = MakeScopedCleanup([&]() { regfree(&re); });
> Is it possible to make it only once and store the result for future invocat
will give it a try and see if it doesn't make things more messy. I figured this 
is a rare enough path that it wasn't a perf concern, but if it doesn't add too 
many LOC I'll do it


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I3a5156b09fa4f8c7591f4e399ce8cc450c089e88
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <[email protected]>
Gerrit-Reviewer: Alexey Serbin <[email protected]>
Gerrit-Reviewer: Dan Burkert <[email protected]>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <[email protected]>
Gerrit-HasComments: Yes

Reply via email to