Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/12967 )
Change subject: [SentryPrivilegesFetcher] deduplicate RPCs to Sentry ...................................................................... Patch Set 4: Code-Review+2 (1 comment) http://gerrit.cloudera.org:8080/#/c/12967/4/src/kudu/master/sentry_privileges_fetcher.cc File src/kudu/master/sentry_privileges_fetcher.cc: http://gerrit.cloudera.org:8080/#/c/12967/4/src/kudu/master/sentry_privileges_fetcher.cc@355 PS4, Line 355: if (VLOG_IS_ON(1)) { : std::ostringstream os; : privilege_resp.printTo(os); : if (action != SentryAction::ALL && action != SentryAction::OWNER && : privilege_resp.grantOption == TSentryGrantOption::ENABLED) { : VLOG(1) << "ignoring ENABLED grant option for unexpected action: " : << static_cast<int16_t>(action); : } Whoops, just noticed this upon looking at the 3->4 interdiff, this is always filling an ostringstream and not doing anything with it. Should probably be: if (VLOG_IS_ON(1) && action != SentryAction::ALL && action != SentryAction::OWNER && privilege_resp.grantOption == TSentryGrantOption::ENABLED) { VLOG(1) << "ignoring enabled grant option for action: " << ActionToString(action); } or just get rid of it; having a grant option for a non-ALL/OWNER action isn't interesting to know about. Feel free to do this separately though, since it's not part of this patch. -- To view, visit http://gerrit.cloudera.org:8080/12967 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2714ab032df6029240ad9e7f4fb03ff73c6b79ef Gerrit-Change-Number: 12967 Gerrit-PatchSet: 4 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: Thu, 11 Apr 2019 01:11:56 +0000 Gerrit-HasComments: Yes
