----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34082/#review126515 -----------------------------------------------------------
sentry-policy/sentry-policy-common/src/main/java/org/apache/sentry/policy/common/PolicyEngine.java (line 55) <https://reviews.apache.org/r/34082/#comment189526> 1. UG = UserGroup One option here is still use the same method name for overload with different parameters. or use the full name: getAllUserGroupPrivileges? 2. Do we needs some comments here? sentry-policy/sentry-policy-common/src/main/java/org/apache/sentry/policy/common/PolicyEngine.java (line 71) <https://reviews.apache.org/r/34082/#comment189527> The similar comments as above (the method name and comments) sentry-policy/sentry-policy-common/src/main/java/org/apache/sentry/policy/common/PolicyEngine.java (line 75) <https://reviews.apache.org/r/34082/#comment189530> The name convention for "support" type of thing usually goes like: isPrivilegeForUserSupported sentry-policy/sentry-policy-db/src/main/java/org/apache/sentry/policy/db/SimpleDBPolicyEngine.java (line 95) <https://reviews.apache.org/r/34082/#comment189529> Needs to add debug log for also users. sentry-policy/sentry-policy-indexer/src/main/java/org/apache/sentry/policy/indexer/SimpleIndexerPolicyEngine.java (line 73) <https://reviews.apache.org/r/34082/#comment189531> Does this mean SimpleIndexerPolicyEngine doens't support user? As the implementation calls the providerBackend to get privileges, the questions is why it cannot support user or it is simply no need for it to support the user? sentry-policy/sentry-policy-indexer/src/main/java/org/apache/sentry/policy/indexer/SimpleIndexerPolicyEngine.java (line 101) <https://reviews.apache.org/r/34082/#comment189532> The same question goes how the make the code clean as to whether to support or not user. sentry-policy/sentry-policy-search/src/main/java/org/apache/sentry/policy/search/SimpleSearchPolicyEngine.java (line 88) <https://reviews.apache.org/r/34082/#comment189533> The same question above as to user support. sentry-policy/sentry-policy-sqoop/src/main/java/org/apache/sentry/policy/sqoop/SimpleSqoopPolicyEngine.java (line 72) <https://reviews.apache.org/r/34082/#comment189534> The same question above as to user support. - Jerry Chen On May 12, 2015, 7:01 a.m., Colin Ma wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/34082/ > ----------------------------------------------------------- > > (Updated May 12, 2015, 7:01 a.m.) > > > Review request for sentry, Dapeng Sun, shen guoquan, and Prasad Mujumdar. > > > Repository: sentry > > > Description > ------- > > Update policy engine for grant user to role > > > Diffs > ----- > > > sentry-policy/sentry-policy-common/src/main/java/org/apache/sentry/policy/common/PolicyEngine.java > 38a5b65 > > sentry-policy/sentry-policy-db/src/main/java/org/apache/sentry/policy/db/SimpleDBPolicyEngine.java > a03794e > > sentry-policy/sentry-policy-indexer/src/main/java/org/apache/sentry/policy/indexer/SimpleIndexerPolicyEngine.java > 2f4bc1d > > sentry-policy/sentry-policy-search/src/main/java/org/apache/sentry/policy/search/SimpleSearchPolicyEngine.java > bc518fb > > sentry-policy/sentry-policy-sqoop/src/main/java/org/apache/sentry/policy/sqoop/SimpleSqoopPolicyEngine.java > e8615a0 > > Diff: https://reviews.apache.org/r/34082/diff/ > > > Testing > ------- > > > Thanks, > > Colin Ma > >
