You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sentry.apache.org by Na Li via Review Board <no...@reviews.apache.org> on 2018/06/04 02:57:58 UTC
Re: Review Request 67402: SENTRY-2244: Alter sentry role or user at
granting privilege can avoid extra query to database
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67402/
-----------------------------------------------------------
(Updated June 4, 2018, 2:57 a.m.)
Review request for sentry, kalyan kumar kalvagadda and Sergio Pena.
Summary (updated)
-----------------
SENTRY-2244: Alter sentry role or user at granting privilege can avoid extra query to database
Bugs: sentry-2244
https://issues.apache.org/jira/browse/sentry-2244
Repository: sentry
Description
-------
When granting privilege to a user or role, sentry does not need to get individual privilege with specific action from DB again after all its associated privileges are fetched from DB. We just just find the corresponding privilege from list to avoid extra query to DB
Diffs
-----
sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java 5932335
Diff: https://reviews.apache.org/r/67402/diff/1/
Testing
-------
test cases in TestSentryStore succeeded
Thanks,
Na Li
Re: Review Request 67402: SENTRY-2244: Alter sentry role or user at
granting privilege can avoid extra query to database
Posted by Na Li via Review Board <no...@reviews.apache.org>.
> On June 6, 2018, 2:28 p.m., kalyan kumar kalvagadda wrote:
> > sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
> > Line 817 (original), 843 (patched)
> > <https://reviews.apache.org/r/67402/diff/2/?file=2034813#file2034813line843>
> >
> > How is findMatchPrivileges different from mRole.getPrivileges().contains()? why don't we call
> > mRole.getPrivileges().contains(convertToMSentryPrivilege(tNotAll))?
mRole.getPrivileges().contains() returns boolean value: yes or no. it does not return MSentryPrivilege. That is why I cannot use mRole.getPrivileges().contains(). Do you know any buildin function I can use to get a maching element from Set<MSentryPrivilege>?
- Na
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67402/#review204350
-----------------------------------------------------------
On June 4, 2018, 3:29 a.m., Na Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67402/
> -----------------------------------------------------------
>
> (Updated June 4, 2018, 3:29 a.m.)
>
>
> Review request for sentry, kalyan kumar kalvagadda and Sergio Pena.
>
>
> Bugs: sentry-2244
> https://issues.apache.org/jira/browse/sentry-2244
>
>
> Repository: sentry
>
>
> Description
> -------
>
> When granting privilege to a user or role, sentry does not need to get individual privilege with specific action from DB again after all its associated privileges are fetched from DB. We just just find the corresponding privilege from list to avoid extra query to DB
>
>
> Diffs
> -----
>
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java 5932335
>
>
> Diff: https://reviews.apache.org/r/67402/diff/2/
>
>
> Testing
> -------
>
> test cases in TestSentryStore succeeded
>
>
> Thanks,
>
> Na Li
>
>
Re: Review Request 67402: SENTRY-2244: Alter sentry role or user at
granting privilege can avoid extra query to database
Posted by kalyan kumar kalvagadda via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67402/#review204350
-----------------------------------------------------------
sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
Line 817 (original), 843 (patched)
<https://reviews.apache.org/r/67402/#comment286824>
How is findMatchPrivileges different from mRole.getPrivileges().contains()? why don't we call
mRole.getPrivileges().contains(convertToMSentryPrivilege(tNotAll))?
- kalyan kumar kalvagadda
On June 4, 2018, 3:29 a.m., Na Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67402/
> -----------------------------------------------------------
>
> (Updated June 4, 2018, 3:29 a.m.)
>
>
> Review request for sentry, kalyan kumar kalvagadda and Sergio Pena.
>
>
> Bugs: sentry-2244
> https://issues.apache.org/jira/browse/sentry-2244
>
>
> Repository: sentry
>
>
> Description
> -------
>
> When granting privilege to a user or role, sentry does not need to get individual privilege with specific action from DB again after all its associated privileges are fetched from DB. We just just find the corresponding privilege from list to avoid extra query to DB
>
>
> Diffs
> -----
>
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java 5932335
>
>
> Diff: https://reviews.apache.org/r/67402/diff/2/
>
>
> Testing
> -------
>
> test cases in TestSentryStore succeeded
>
>
> Thanks,
>
> Na Li
>
>
Re: Review Request 67402: SENTRY-2244: Alter sentry role or user at
granting privilege can avoid extra query to database
Posted by Sergio Pena via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67402/#review204348
-----------------------------------------------------------
Ship it!
Ship It!
- Sergio Pena
On June 4, 2018, 3:29 a.m., Na Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67402/
> -----------------------------------------------------------
>
> (Updated June 4, 2018, 3:29 a.m.)
>
>
> Review request for sentry, kalyan kumar kalvagadda and Sergio Pena.
>
>
> Bugs: sentry-2244
> https://issues.apache.org/jira/browse/sentry-2244
>
>
> Repository: sentry
>
>
> Description
> -------
>
> When granting privilege to a user or role, sentry does not need to get individual privilege with specific action from DB again after all its associated privileges are fetched from DB. We just just find the corresponding privilege from list to avoid extra query to DB
>
>
> Diffs
> -----
>
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java 5932335
>
>
> Diff: https://reviews.apache.org/r/67402/diff/2/
>
>
> Testing
> -------
>
> test cases in TestSentryStore succeeded
>
>
> Thanks,
>
> Na Li
>
>
Re: Review Request 67402: SENTRY-2244: Alter sentry role or user at
granting privilege can avoid extra query to database
Posted by kalyan kumar kalvagadda via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67402/#review204407
-----------------------------------------------------------
Ship it!
Looks good.
- kalyan kumar kalvagadda
On June 4, 2018, 3:29 a.m., Na Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67402/
> -----------------------------------------------------------
>
> (Updated June 4, 2018, 3:29 a.m.)
>
>
> Review request for sentry, kalyan kumar kalvagadda and Sergio Pena.
>
>
> Bugs: sentry-2244
> https://issues.apache.org/jira/browse/sentry-2244
>
>
> Repository: sentry
>
>
> Description
> -------
>
> When granting privilege to a user or role, sentry does not need to get individual privilege with specific action from DB again after all its associated privileges are fetched from DB. We just just find the corresponding privilege from list to avoid extra query to DB
>
>
> Diffs
> -----
>
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java 5932335
>
>
> Diff: https://reviews.apache.org/r/67402/diff/2/
>
>
> Testing
> -------
>
> test cases in TestSentryStore succeeded
>
>
> Thanks,
>
> Na Li
>
>
Re: Review Request 67402: SENTRY-2244: Alter sentry role or user at
granting privilege can avoid extra query to database
Posted by Na Li via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67402/
-----------------------------------------------------------
(Updated June 4, 2018, 3:29 a.m.)
Review request for sentry, kalyan kumar kalvagadda and Sergio Pena.
Bugs: sentry-2244
https://issues.apache.org/jira/browse/sentry-2244
Repository: sentry
Description
-------
When granting privilege to a user or role, sentry does not need to get individual privilege with specific action from DB again after all its associated privileges are fetched from DB. We just just find the corresponding privilege from list to avoid extra query to DB
Diffs (updated)
-----
sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java 5932335
Diff: https://reviews.apache.org/r/67402/diff/2/
Changes: https://reviews.apache.org/r/67402/diff/1-2/
Testing
-------
test cases in TestSentryStore succeeded
Thanks,
Na Li