Xiaomeng Zhang has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/14433 )

Change subject: IMPALA-7504/KUDU-2979 ParseKerberosPrincipal() should use 
krb5_parse_name() instead
......................................................................


Patch Set 6:

(7 comments)

http://gerrit.cloudera.org:8080/#/c/14433/5//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/14433/5//COMMIT_MSG@10
PS5, Line 10: using custom code.
> instead of using custom code.
Done


http://gerrit.cloudera.org:8080/#/c/14433/5//COMMIT_MSG@12
PS5, Line 12: When kerberos is initialized in Impala's copy of Kudu code, it 
stores a
> The reader of commit messages is developers like you and me.
Done


http://gerrit.cloudera.org:8080/#/c/14433/5//COMMIT_MSG@18
PS5, Line 18: Test done:
> to verify parsing a principal nae containing a special character.
Done


http://gerrit.cloudera.org:8080/#/c/14433/5//COMMIT_MSG@19
PS5, Line 19: Add two authentication-test, one to verify parsing a principal 
containing
> Is it possible to have an automated test for this?
Yes, added a new test BadPrincipalFormat


http://gerrit.cloudera.org:8080/#/c/14433/5//COMMIT_MSG@21
PS5, Line 21: format principal, new error code is 2 instead of original 112
> Do you want to mention running end-to-end tests?
What kind of end-to-end test? The one included in impala-private-parameterized?


http://gerrit.cloudera.org:8080/#/c/14433/5/be/src/kudu/security/init.h
File be/src/kudu/security/init.h:

http://gerrit.cloudera.org:8080/#/c/14433/5/be/src/kudu/security/init.h@39
PS5, Line 39: // Parses the given Kerberos principal into service name, 
hostname, and realm.
> Maybe: "Parse a kerberos principal name and extract the ervice_name, hostna
Done


http://gerrit.cloudera.org:8080/#/c/14433/5/be/src/util/auth-util.cc
File be/src/util/auth-util.cc:

http://gerrit.cloudera.org:8080/#/c/14433/5/be/src/util/auth-util.cc@92
PS5, Line 92:       hostname, realm), strings::Substitute("bad principal format 
$0", principal));
> It is more idiomatic and marginally more efficient to use:
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0e64ebdc10f102dbdc5b87f6fe3f2a0310b1be24
Gerrit-Change-Number: 14433
Gerrit-PatchSet: 6
Gerrit-Owner: Xiaomeng Zhang <[email protected]>
Gerrit-Reviewer: Andrew Sherman <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Michael Ho <[email protected]>
Gerrit-Reviewer: Xiaomeng Zhang <[email protected]>
Gerrit-Comment-Date: Wed, 23 Oct 2019 01:13:17 +0000
Gerrit-HasComments: Yes

Reply via email to