Adar Dembo 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) I'm a little concerned that HMS (and Sentry) integration will make these tests more flaky. Could you loop them on dist-test to gauge the situation? 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: enable Nit: to enable 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 second parameter as "enabling Kerberos" rather than "enabling Sentry" since a lot of our tests parameterize in that way. 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 this? I wonder if we should start consolidating logic like this into the ExternalMiniCluster itself since so many tests need it. http://gerrit.cloudera.org:8080/#/c/12877/1/src/kudu/integration-tests/master-stress-test.cc File src/kudu/integration-tests/master-stress-test.cc: http://gerrit.cloudera.org:8080/#/c/12877/1/src/kudu/integration-tests/master-stress-test.cc@117 PS1, Line 117: // Parameterized based on HmsMode and and whether or not enable Sentry integration. Same comments, etc. 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@85 PS1, Line 85: // Parameterized based on HmsMode and and whether or not enable Sentry integration. Same comments here as in alter_table-randomized-test. -- 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: Thu, 28 Mar 2019 20:49:45 +0000 Gerrit-HasComments: Yes
