Andrew Wong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12877 )

Change subject: [sentry] enable sentry integration for master stress test
......................................................................


Patch Set 1:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/12877/1/src/kudu/integration-tests/alter_table-randomized-test.cc
File src/kudu/integration-tests/alter_table-randomized-test.cc:

http://gerrit.cloudera.org:8080/#/c/12877/1/src/kudu/integration-tests/alter_table-randomized-test.cc@106
PS1, Line 106: and
nit: remove


http://gerrit.cloudera.org:8080/#/c/12877/1/src/kudu/integration-tests/alter_table-randomized-test.cc@108
PS1, Line 108:                              public 
::testing::WithParamInterface<pair<HmsMode, bool>> {
> Not that it changes anything, but might be more obvious to refer to the sec
We should probably avoid conflating the two. A disabled HMS integration 
wouldn't work with an enabled Sentry integration, but I don't think that's the 
case for Kerberos.

Maybe instead have a SentryMode enum?


http://gerrit.cloudera.org:8080/#/c/12877/1/src/kudu/integration-tests/alter_table-randomized-test.cc@126
PS1, Line 126:     if (enable_sentry) {
> Is there another test fixture we could leverage to avoid duplicating all of
+1 seems this is copied in three tests; probably worth sticking it somewhere 
central.


http://gerrit.cloudera.org:8080/#/c/12877/1/src/kudu/integration-tests/alter_table-randomized-test.cc@178
PS1, Line 178:      { HmsMode::NONE, false },
             :
             :       { HmsMode::ENABLE_METASTORE_INTEGRATION, false },
             :
             :
nit: extra lines


http://gerrit.cloudera.org:8080/#/c/12877/1/src/kudu/integration-tests/master_failover-itest.cc
File src/kudu/integration-tests/master_failover-itest.cc:

http://gerrit.cloudera.org:8080/#/c/12877/1/src/kudu/integration-tests/master_failover-itest.cc@196
PS1, Line 196:     vector<pair<HmsMode, bool>> {
             :       { HmsMode::NONE, false },
             :
             :       { HmsMode::ENABLE_METASTORE_INTEGRATION, false },
             :
             :       { HmsMode::ENABL
nit: extra lines



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic48aa1bfd0947c645bb81137bb34e6cdfc088cf4
Gerrit-Change-Number: 12877
Gerrit-PatchSet: 1
Gerrit-Owner: Hao Hao <[email protected]>
Gerrit-Reviewer: Adar Dembo <[email protected]>
Gerrit-Reviewer: Alexey Serbin <[email protected]>
Gerrit-Reviewer: Andrew Wong <[email protected]>
Gerrit-Reviewer: Hao Hao <[email protected]>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Fri, 29 Mar 2019 04:29:08 +0000
Gerrit-HasComments: Yes

Reply via email to