> On May 17, 2018, 10:22 p.m., Sergio Pena wrote: > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyStoreProcessor.java > > Lines 1054 (patched) > > <https://reviews.apache.org/r/67174/diff/2/?file=2024612#file2024612line1054> > > > > Don't we need to provide the implementation for this in this patch?
In order to do so, the datastructure for Thrift should be changed to contain the privileges for both role and user. There is another jira for that. Without that change, the returned result still not used and returned to client. > On May 17, 2018, 10:22 p.m., Sergio Pena wrote: > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/QueryParamBuilder.java > > Lines 303 (patched) > > <https://reviews.apache.org/r/67174/diff/2/?file=2024613#file2024613line303> > > > > Agree, This method might not be needed if the callers just make the > > correct call instead of using this generic method. The checking entity types hapen in multiple places. Based on your preference, when the same code blocks are called multiple times, we should have a function to abstract that, so the code is cleaner. > On May 17, 2018, 10:22 p.m., Sergio Pena wrote: > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java > > Lines 172-173 (original), 176-178 (patched) > > <https://reviews.apache.org/r/67174/diff/2/?file=2024614#file2024614line177> > > > > Have we decided what the decomposition of the ALL when revoking > > individual actions be? I think we should not fix this issue yet. I thing > > the discussion is that we won't be able to decompose the ALL to avoid > > incompatibilities. This code change is not to change behavior, but to fix the bugs in existing behavior. If we decide to change the behavior, and not decompose the ALL, we need to have another jira, and not change the behavior in this jira. - Na ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67174/#review203371 ----------------------------------------------------------- On May 18, 2018, 3:27 p.m., Na Li wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/67174/ > ----------------------------------------------------------- > > (Updated May 18, 2018, 3:27 p.m.) > > > Review request for sentry, Alexander Kolbasov, Arjun Mishra, kalyan kumar > kalvagadda, and Sergio Pena. > > > Bugs: sentry-2156 > https://issues.apache.org/jira/browse/sentry-2156 > > > Repository: sentry > > > Description > ------- > > Add functions related to grant/revoke privileges to/from user > > Fix the bugs related to grant/revoke partial privileges. They are caused by > adding fine grained privileges (CRATE, DROP, ALTER) > add test cases for grant/revoke privileges to/from user > > > Diffs > ----- > > > sentry-core/sentry-core-common/src/main/java/org/apache/sentry/service/common/ServiceConstants.java > 71e9585 > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyStoreProcessor.java > 816cfe1 > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/QueryParamBuilder.java > 8a77fc1 > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java > cafe2b5 > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java > 0322cc3 > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDatabaseProvider.java > 3e31852 > > > Diff: https://reviews.apache.org/r/67174/diff/4/ > > > Testing > ------- > > all test cases in TestSentryStore passed > > > Thanks, > > Na Li > >
