Sailesh Mukil has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8270 )

Change subject: IMPALA-5053: [SECURITY] Make KRPC work with Kerberos
......................................................................


Patch Set 3:

(11 comments)

http://gerrit.cloudera.org:8080/#/c/8270/2/be/src/rpc/rpc-mgr-test.cc
File be/src/rpc/rpc-mgr-test.cc:

http://gerrit.cloudera.org:8080/#/c/8270/2/be/src/rpc/rpc-mgr-test.cc@124
PS2, Line 124:
> nit: long line
Done


http://gerrit.cloudera.org:8080/#/c/8270/2/be/src/rpc/rpc-mgr.cc
File be/src/rpc/rpc-mgr.cc:

http://gerrit.cloudera.org:8080/#/c/8270/2/be/src/rpc/rpc-mgr.cc@66
PS2, Line 66:
> nit: long line.
Done


http://gerrit.cloudera.org:8080/#/c/8270/2/be/src/testutil/mini-kdc-wrapper.h
File be/src/testutil/mini-kdc-wrapper.h:

http://gerrit.cloudera.org:8080/#/c/8270/2/be/src/testutil/mini-kdc-wrapper.h@18
PS2, Line 18: WRAPPE
> WRAPPER ?
Done


http://gerrit.cloudera.org:8080/#/c/8270/2/be/src/testutil/mini-kdc-wrapper.h@40
PS2, Line 40: mode
> mode
Done


http://gerrit.cloudera.org:8080/#/c/8270/2/be/src/testutil/mini-kdc-wrapper.h@65
PS2, Line 65:   const std::string spn_;
> const ? Same below.
Done


http://gerrit.cloudera.org:8080/#/c/8270/2/be/src/testutil/mini-kdc-wrapper.h@66
PS2, Line 66:
> nit: white space.
Done


http://gerrit.cloudera.org:8080/#/c/8270/2/be/src/testutil/mini-kdc-wrapper.h@75
PS2, Line 75:
> nit: white space.
Done


http://gerrit.cloudera.org:8080/#/c/8270/2/be/src/testutil/mini-kdc-wrapper.h@82
PS2, Line 82: // Starts the KDC and configures it
> One line comment.
Done


http://gerrit.cloudera.org:8080/#/c/8270/2/be/src/util/auth-util.h
File be/src/util/auth-util.h:

http://gerrit.cloudera.org:8080/#/c/8270/2/be/src/util/auth-util.h@47
PS2, Line 47: internal kerberos principal.
> Mind adding comments explaining what the difference is between internal and
Done


http://gerrit.cloudera.org:8080/#/c/8270/2/be/src/util/auth-util.h@50
PS2, Line 50: string.
> Same comment as GetInternalKerberosPrincipal().
Done


http://gerrit.cloudera.org:8080/#/c/8270/2/be/src/util/auth-util.h@54
PS2, Line 54:
> Please also document the return value. For example, what if 'principal' doe
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8cec5cca5fdb4b1d46bab19e86cb1a8a3ad718fd
Gerrit-Change-Number: 8270
Gerrit-PatchSet: 3
Gerrit-Owner: Sailesh Mukil <sail...@cloudera.com>
Gerrit-Reviewer: Michael Ho <k...@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sail...@cloudera.com>
Gerrit-Comment-Date: Fri, 10 Nov 2017 00:08:42 +0000
Gerrit-HasComments: Yes

Reply via email to