Attila Bukor has posted comments on this change. ( http://gerrit.cloudera.org:8080/15483 )
Change subject: KUDU-3079 Add MiniRanger ...................................................................... Patch Set 28: (10 comments) http://gerrit.cloudera.org:8080/#/c/15483/26/src/kudu/ranger/mini_ranger.h File src/kudu/ranger/mini_ranger.h: http://gerrit.cloudera.org:8080/#/c/15483/26/src/kudu/ranger/mini_ranger.h@53 PS26, Line 53: on a resource > Is this the policy for a single resource? The vectors of databases, tables, I think so, but couldn't really find any docs that were clear about how this works. We'll likely need to test it. http://gerrit.cloudera.org:8080/#/c/15483/26/src/kudu/ranger/mini_ranger.h@54 PS26, Line 54: user > nit: users Done http://gerrit.cloudera.org:8080/#/c/15483/26/src/kudu/ranger/mini_ranger.h@76 PS26, Line 76: curl_.set_auth(CurlAuthType::BASIC, "admin", "admin"); : } > nit: remove a few spaces how should this be formatted? http://gerrit.cloudera.org:8080/#/c/15483/26/src/kudu/ranger/mini_ranger.h@126 PS26, Line 126: > Will this be evaluated too? nope, removed http://gerrit.cloudera.org:8080/#/c/15483/26/src/kudu/ranger/mini_ranger.cc File src/kudu/ranger/mini_ranger.cc: http://gerrit.cloudera.org:8080/#/c/15483/26/src/kudu/ranger/mini_ranger.cc@238 PS26, Line 238: > nit: Failed Done http://gerrit.cloudera.org:8080/#/c/15483/26/src/kudu/ranger/mini_ranger.cc@235 PS26, Line 235: "Failed to create Kudu service"); : return Status::OK(); : } : : Status MiniRanger::AddPolicy(AuthorizationPolicy policy) { : EasyJson policy_json; : policy_js > Could probably just Done http://gerrit.cloudera.org:8080/#/c/15483/26/src/kudu/ranger/mini_ranger.cc@303 PS26, Line 303: er > nit: keep the casing consistent? PostToRanger? Done http://gerrit.cloudera.org:8080/#/c/15483/26/src/kudu/ranger/mini_ranger.cc@303 PS26, Line 303: > nit: const string& shouldn't we use pass-by-value and move instead of const-ref? http://gerrit.cloudera.org:8080/#/c/15483/26/src/kudu/ranger/mini_ranger_configs.h File src/kudu/ranger/mini_ranger_configs.h: http://gerrit.cloudera.org:8080/#/c/15483/26/src/kudu/ranger/mini_ranger_configs.h@64 PS26, Line 64: RPC > nit: RPCs Done http://gerrit.cloudera.org:8080/#/c/15483/22/src/kudu/tools/tool.proto File src/kudu/tools/tool.proto: http://gerrit.cloudera.org:8080/#/c/15483/22/src/kudu/tools/tool.proto@45 PS22, Line 45: // Whether or not Ranger should be enabled : optional bool enable_ranger = 9; : > Ah I see. I think virtually all of our usage of Sentry happened through uni Done -- To view, visit http://gerrit.cloudera.org:8080/15483 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I15ab1eb8abe71c074c26b286073442882e101bc6 Gerrit-Change-Number: 15483 Gerrit-PatchSet: 28 Gerrit-Owner: Attila Bukor <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Andrew Wong <[email protected]> Gerrit-Reviewer: Attila Bukor <[email protected]> Gerrit-Reviewer: Grant Henke <[email protected]> Gerrit-Reviewer: Hao Hao <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Mon, 23 Mar 2020 10:10:38 +0000 Gerrit-HasComments: Yes
