Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/12833 )
Change subject: WIP [master] introduced SentryPrivilegesFetcher ...................................................................... Patch Set 10: (1 comment) http://gerrit.cloudera.org:8080/#/c/12833/9/src/kudu/master/sentry_authz_provider-test.cc File src/kudu/master/sentry_authz_provider-test.cc: http://gerrit.cloudera.org:8080/#/c/12833/9/src/kudu/master/sentry_authz_provider-test.cc@426 PS9, Line 426: SentryPrivilegesBranch privileges; : ASSERT_OK(sentry_authz_provider_->fetcher_.GetSentryPrivileges( : > Yeah I had the same feedback in Andrew's patch. In my case, I didn't commen I agree with limiting the test matrix, OTOH if we're limiting it, we might as well limit it to the half of the matrix that we expect to be used (ie using Kerberos). Yes, this test is testing the SentryAuthzProvider's logic, but some of that logic is its interaction with Sentry, and so testing its connection with Sentry in a realistic way is probably worth doing. -- To view, visit http://gerrit.cloudera.org:8080/12833 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Idaefacd50736f1f152dae34e76778e17b2e84cbe Gerrit-Change-Number: 12833 Gerrit-PatchSet: 10 Gerrit-Owner: Alexey Serbin <aser...@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, 09 Apr 2019 18:11:57 +0000 Gerrit-HasComments: Yes
