Alexey Serbin 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@108 PS1, Line 108: public ::testing::WithParamInterface<pair<HmsMode, bool>> { > We should probably avoid conflating the two. A disabled HMS integration wou +1 for SentryMode or at least have boolean constants named bool SENTRY_DISABLED = false; bool SENTRY_ENABLED = true; http://gerrit.cloudera.org:8080/#/c/12877/1/src/kudu/integration-tests/alter_table-randomized-test.cc@140 PS1, Line 140: GetServerPrivilege Would Dababase privilege be enough for these new scenarios? http://gerrit.cloudera.org:8080/#/c/12877/1/src/kudu/integration-tests/alter_table-randomized-test.cc@176 PS1, Line 176: ::testing::ValuesIn nit: will it be able to infer the type if having just ... ::testing::Values( { HmsMode::NONE, false }, { HmsMode::ENABLE_METASTORE_INTEGRATION, false }, { HmsMode::ENABLE_METASTORE_INTEGRATION, true }, ) ... ? 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@118 PS1, Line 118: MasterStressTest Does it make sense to restart Sentry time to time during this test? That would be something orthogonal to restarting masters and since we are expecting NetworkError status (right?) everything should run smooth? http://gerrit.cloudera.org:8080/#/c/12877/1/src/kudu/integration-tests/master-stress-test.cc@522 PS1, Line 522: ::testing::ValuesIn( : vector<pair<HmsMode, bool>> { : { HmsMode::NONE, false }, : : { HmsMode::ENABLE_METASTORE_INTEGRATION, false }, : : { HmsMode::ENABLE_METASTORE_INTEGRATION, true }, : } nit: will it compile with some the reduced option below ::testing::Values( { HmsMode::NONE, false }, { HmsMode::ENABLE_METASTORE_INTEGRATION, false }, { HmsMode::ENABLE_METASTORE_INTEGRATION, true } ) ? -- 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 <hao....@cloudera.com> Gerrit-Reviewer: Adar Dembo <a...@cloudera.com> Gerrit-Reviewer: Alexey Serbin <aser...@cloudera.com> Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com> Gerrit-Reviewer: Hao Hao <hao....@cloudera.com> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Tue, 02 Apr 2019 04:49:08 +0000 Gerrit-HasComments: Yes
