> On May 17, 2018, 4:06 p.m., kalyan kumar kalvagadda wrote:
> > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
> > Lines 169-173 (original), 171-178 (patched)
> > <https://reviews.apache.org/r/67174/diff/2/?file=2024614#file2024614line172>
> >
> > This can be in seperate patch.
>
> Na Li wrote:
> There are more places involved
>
> Na Li wrote:
> When I rewrite the code to combine both role and user for privileges, the
> existing bugs are fixed.
>
> 1) If we just have another patch for PARTIAL_REVOKE_ACTIONS and test
> cases, the patch does not really reflect the real code change
> 2) Or, I need to purposely change the rewritten working codee to make it
> contains the original bug. It takes more time to do so and does not make
> sense for me to have one patch contains rewritten code but keeps bugs I know,
> and another patch to fix those bugs.
I remove the code that will decompose all to include {drop, create, alter}. So
partial revoke behaves the same as before.
> On May 17, 2018, 4:06 p.m., kalyan kumar kalvagadda wrote:
> > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
> > Lines 805 (patched)
> > <https://reviews.apache.org/r/67174/diff/2/?file=2024614#file2024614line806>
> >
> > Can you make this API interface simple? Currently there are two inputs
> > from this method to the caller.
> > 1. Return value
> > 2. privilegesToRemove
>
> Na Li wrote:
> what's your suggestion on returning two outuputs?
As I talked with you, there are following possible scenarios to cover
1) The processing should not continue. For example, the user has "ALL"
privilege on the object. granting any individual privilege is no-op
2) The processing should continue. privilegesToRemove is empty. For example,
user has no privileges
3) The processing should continue. privilegesToRemove is not empty. For
example, user has "insert" and then granted "ALL"
We cannot cover all these scenario using privilegesToRemove only.
- Na
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67174/#review203329
-----------------------------------------------------------
On May 21, 2018, 10:48 p.m., Na Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67174/
> -----------------------------------------------------------
>
> (Updated May 21, 2018, 10:48 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
> 56c506b
>
> 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/6/
>
>
> Testing
> -------
>
> all test cases in TestSentryStore passed
>
>
> Thanks,
>
> Na Li
>
>
