You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sentry.apache.org by Sergio Pena via Review Board <no...@reviews.apache.org> on 2018/06/27 16:01:41 UTC
Review Request 67759: SENTRY-2284: Add two client API to get all
roles or users privileges mapping
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67759/
-----------------------------------------------------------
Review request for sentry, Arjun Mishra, kalyan kumar kalvagadda, and Na Li.
Bugs: sentry-2284
https://issues.apache.org/jira/browse/sentry-2284
Repository: sentry
Description
-------
Add two API methods to the Sentry client:
- listAllRolesPrivileges
- listAllUsersPrivileges
Both methods return a map object of the form:
- [roleName, set<privileges>]
- [userName, set<privileges>]
Unit tests, thrift API code and Client methods are provided.
Diffs
-----
sentry-dist/src/license/THIRD-PARTY.properties b39e1b6ca7eba8c6a7695a4238104af7cd50da32
sentry-service/sentry-service-api/src/gen/thrift/gen-javabean/org/apache/sentry/api/service/thrift/SentryPolicyService.java d456d7ff64a8146473ff14a65f06e7cb664939b7
sentry-service/sentry-service-api/src/gen/thrift/gen-javabean/org/apache/sentry/api/service/thrift/TSentryPrivilegesRequest.java PRE-CREATION
sentry-service/sentry-service-api/src/gen/thrift/gen-javabean/org/apache/sentry/api/service/thrift/TSentryPrivilegesResponse.java PRE-CREATION
sentry-service/sentry-service-api/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyServiceClient.java dc1d67ba16c9c7acf76eb3318864ba0606e2aa5a
sentry-service/sentry-service-api/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyServiceClientDefaultImpl.java 5f76cb0aa3bd17d6aca4adc52ba59ded5ec0b900
sentry-service/sentry-service-api/src/main/resources/sentry_policy_service.thrift 56aedcb5267495e610e3dace1f38e708d68ffe84
sentry-service/sentry-service-api/src/test/java/org/apache/sentry/api/service/thrift/TestSentryPolicyServiceClientDefaultImpl.java 1666e326462b771674930e52e3790ca92f467778
sentry-service/sentry-service-server/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyStoreProcessor.java 39271500df179e0ddf8ce4f1589eaaa8137afa25
sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java d8ab1fc90255c6ed42c00e3d3aa6103e47a40b29
sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStoreInterface.java f61ae575c99474a76886d3c7a74765fdb067acd6
sentry-service/sentry-service-server/src/test/java/org/apache/sentry/api/service/thrift/TestSentryPolicyStoreProcessor.java de4e0012d521cf402f4773a04b673f8056a5337c
sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java 52ce72cebc8b69f08988b05ba54e2bbedd201c1a
Diff: https://reviews.apache.org/r/67759/diff/1/
Testing
-------
Thanks,
Sergio Pena
Re: Review Request 67759: SENTRY-2284: Add two client API to get all
roles or users privileges mapping
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/67759/#review205471
-----------------------------------------------------------
sentry-dist/src/license/THIRD-PARTY.properties
Line 32 (original), 32 (patched)
<https://reviews.apache.org/r/67759/#comment288385>
this file is still in change list for update 3. Should be removed
- Na Li
On June 27, 2018, 7:33 p.m., Sergio Pena wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67759/
> -----------------------------------------------------------
>
> (Updated June 27, 2018, 7:33 p.m.)
>
>
> Review request for sentry, Arjun Mishra, kalyan kumar kalvagadda, and Na Li.
>
>
> Bugs: sentry-2284
> https://issues.apache.org/jira/browse/sentry-2284
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Add two API methods to the Sentry client:
> - listAllRolesPrivileges
> - listAllUsersPrivileges
>
> Both methods return a map object of the form:
> - [roleName, set<privileges>]
> - [userName, set<privileges>]
>
> Unit tests, thrift API code and Client methods are provided.
>
>
> Diffs
> -----
>
> sentry-dist/src/license/THIRD-PARTY.properties b39e1b6ca7eba8c6a7695a4238104af7cd50da32
> sentry-service/sentry-service-api/src/gen/thrift/gen-javabean/org/apache/sentry/api/service/thrift/SentryPolicyService.java d456d7ff64a8146473ff14a65f06e7cb664939b7
> sentry-service/sentry-service-api/src/gen/thrift/gen-javabean/org/apache/sentry/api/service/thrift/TSentryPrivilegesRequest.java PRE-CREATION
> sentry-service/sentry-service-api/src/gen/thrift/gen-javabean/org/apache/sentry/api/service/thrift/TSentryPrivilegesResponse.java PRE-CREATION
> sentry-service/sentry-service-api/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyServiceClient.java dc1d67ba16c9c7acf76eb3318864ba0606e2aa5a
> sentry-service/sentry-service-api/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyServiceClientDefaultImpl.java 5f76cb0aa3bd17d6aca4adc52ba59ded5ec0b900
> sentry-service/sentry-service-api/src/main/resources/sentry_policy_service.thrift 56aedcb5267495e610e3dace1f38e708d68ffe84
> sentry-service/sentry-service-api/src/test/java/org/apache/sentry/api/service/thrift/TestSentryPolicyServiceClientDefaultImpl.java 1666e326462b771674930e52e3790ca92f467778
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyStoreProcessor.java 39271500df179e0ddf8ce4f1589eaaa8137afa25
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java d8ab1fc90255c6ed42c00e3d3aa6103e47a40b29
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStoreInterface.java f61ae575c99474a76886d3c7a74765fdb067acd6
> sentry-service/sentry-service-server/src/test/java/org/apache/sentry/api/service/thrift/TestSentryPolicyStoreProcessor.java de4e0012d521cf402f4773a04b673f8056a5337c
> sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java 52ce72cebc8b69f08988b05ba54e2bbedd201c1a
>
>
> Diff: https://reviews.apache.org/r/67759/diff/3/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Sergio Pena
>
>
Re: Review Request 67759: SENTRY-2284: Add two client API to get all
roles or users privileges mapping
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/67759/#review205482
-----------------------------------------------------------
Ship it!
Ship It!
- Na Li
On June 27, 2018, 7:33 p.m., Sergio Pena wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67759/
> -----------------------------------------------------------
>
> (Updated June 27, 2018, 7:33 p.m.)
>
>
> Review request for sentry, Arjun Mishra, kalyan kumar kalvagadda, and Na Li.
>
>
> Bugs: sentry-2284
> https://issues.apache.org/jira/browse/sentry-2284
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Add two API methods to the Sentry client:
> - listAllRolesPrivileges
> - listAllUsersPrivileges
>
> Both methods return a map object of the form:
> - [roleName, set<privileges>]
> - [userName, set<privileges>]
>
> Unit tests, thrift API code and Client methods are provided.
>
>
> Diffs
> -----
>
> sentry-dist/src/license/THIRD-PARTY.properties b39e1b6ca7eba8c6a7695a4238104af7cd50da32
> sentry-service/sentry-service-api/src/gen/thrift/gen-javabean/org/apache/sentry/api/service/thrift/SentryPolicyService.java d456d7ff64a8146473ff14a65f06e7cb664939b7
> sentry-service/sentry-service-api/src/gen/thrift/gen-javabean/org/apache/sentry/api/service/thrift/TSentryPrivilegesRequest.java PRE-CREATION
> sentry-service/sentry-service-api/src/gen/thrift/gen-javabean/org/apache/sentry/api/service/thrift/TSentryPrivilegesResponse.java PRE-CREATION
> sentry-service/sentry-service-api/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyServiceClient.java dc1d67ba16c9c7acf76eb3318864ba0606e2aa5a
> sentry-service/sentry-service-api/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyServiceClientDefaultImpl.java 5f76cb0aa3bd17d6aca4adc52ba59ded5ec0b900
> sentry-service/sentry-service-api/src/main/resources/sentry_policy_service.thrift 56aedcb5267495e610e3dace1f38e708d68ffe84
> sentry-service/sentry-service-api/src/test/java/org/apache/sentry/api/service/thrift/TestSentryPolicyServiceClientDefaultImpl.java 1666e326462b771674930e52e3790ca92f467778
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyStoreProcessor.java 39271500df179e0ddf8ce4f1589eaaa8137afa25
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java d8ab1fc90255c6ed42c00e3d3aa6103e47a40b29
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStoreInterface.java f61ae575c99474a76886d3c7a74765fdb067acd6
> sentry-service/sentry-service-server/src/test/java/org/apache/sentry/api/service/thrift/TestSentryPolicyStoreProcessor.java de4e0012d521cf402f4773a04b673f8056a5337c
> sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java 52ce72cebc8b69f08988b05ba54e2bbedd201c1a
>
>
> Diff: https://reviews.apache.org/r/67759/diff/3/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Sergio Pena
>
>
Re: Review Request 67759: SENTRY-2284: Add two client API to get all
roles or users privileges mapping
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/67759/
-----------------------------------------------------------
(Updated June 27, 2018, 7:33 p.m.)
Review request for sentry, Arjun Mishra, kalyan kumar kalvagadda, and Na Li.
Bugs: sentry-2284
https://issues.apache.org/jira/browse/sentry-2284
Repository: sentry
Description
-------
Add two API methods to the Sentry client:
- listAllRolesPrivileges
- listAllUsersPrivileges
Both methods return a map object of the form:
- [roleName, set<privileges>]
- [userName, set<privileges>]
Unit tests, thrift API code and Client methods are provided.
Diffs (updated)
-----
sentry-dist/src/license/THIRD-PARTY.properties b39e1b6ca7eba8c6a7695a4238104af7cd50da32
sentry-service/sentry-service-api/src/gen/thrift/gen-javabean/org/apache/sentry/api/service/thrift/SentryPolicyService.java d456d7ff64a8146473ff14a65f06e7cb664939b7
sentry-service/sentry-service-api/src/gen/thrift/gen-javabean/org/apache/sentry/api/service/thrift/TSentryPrivilegesRequest.java PRE-CREATION
sentry-service/sentry-service-api/src/gen/thrift/gen-javabean/org/apache/sentry/api/service/thrift/TSentryPrivilegesResponse.java PRE-CREATION
sentry-service/sentry-service-api/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyServiceClient.java dc1d67ba16c9c7acf76eb3318864ba0606e2aa5a
sentry-service/sentry-service-api/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyServiceClientDefaultImpl.java 5f76cb0aa3bd17d6aca4adc52ba59ded5ec0b900
sentry-service/sentry-service-api/src/main/resources/sentry_policy_service.thrift 56aedcb5267495e610e3dace1f38e708d68ffe84
sentry-service/sentry-service-api/src/test/java/org/apache/sentry/api/service/thrift/TestSentryPolicyServiceClientDefaultImpl.java 1666e326462b771674930e52e3790ca92f467778
sentry-service/sentry-service-server/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyStoreProcessor.java 39271500df179e0ddf8ce4f1589eaaa8137afa25
sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java d8ab1fc90255c6ed42c00e3d3aa6103e47a40b29
sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStoreInterface.java f61ae575c99474a76886d3c7a74765fdb067acd6
sentry-service/sentry-service-server/src/test/java/org/apache/sentry/api/service/thrift/TestSentryPolicyStoreProcessor.java de4e0012d521cf402f4773a04b673f8056a5337c
sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java 52ce72cebc8b69f08988b05ba54e2bbedd201c1a
Diff: https://reviews.apache.org/r/67759/diff/3/
Changes: https://reviews.apache.org/r/67759/diff/2-3/
Testing
-------
Thanks,
Sergio Pena
Re: Review Request 67759: SENTRY-2284: Add two client API to get all
roles or users privileges mapping
Posted by Sergio Pena via Review Board <no...@reviews.apache.org>.
> On June 27, 2018, 5 p.m., Arjun Mishra wrote:
> > sentry-service/sentry-service-api/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyServiceClient.java
> > Lines 311 (patched)
> > <https://reviews.apache.org/r/67759/diff/2/?file=2046406#file2046406line311>
> >
> > Sergio, maybe this is the right opportunity to have a single method instead of one for roles and one for users and return something like Map<TSentryPrincipal, Set<TSentryPrivileges>> ?
I thought about that early, but the intention of this API is that you could get either all roles and privileges or all users and privileges. If we do return all users and roles, then the caller of this API would need to separate roles and users which would take O(n) time where n is the number of roles + users.
For instance:
Map<TSentryPrincipal, Set<TSentryPrivileges>> privileges = client.list_all_privileges();
Map<String, Set<TSentryPrivileges>> allRoles, allUsers;
for (TSentryPrincipal princ : privileges.keySet()) {
if (princ.getType() == SentryPrincipal.ROLE)
allRoles.put(princ.getName(), privileges.get(princ));
else if (princ.getType() == SentryPrincipal.USER)
allUsers.put(princ.getName(), privileges.get(princ));
else
;// principal not supported
}
With the above code, now the caller can check if a specific role or a specific user has privileges, or display only user privileges or only role privileges.
That's why I didn't go with the above solution. If I go to the current one, then the caller would not need to split users vs roles, and it will be able to check privileges in O(1).
Another idea is to add in the request to return only user privileges or only role privileges, but how can the caller made sure the returned principals are for user or for roles? They would need to check if the princ.geType() is equals to USER or ROLE everytime just to make sure there were no errors.
So, I prefered to sacrify two API calls instead of 1 to let the caller have a simple data type with either roles or users.
- Sergio
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67759/#review205452
-----------------------------------------------------------
On June 27, 2018, 4:49 p.m., Sergio Pena wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67759/
> -----------------------------------------------------------
>
> (Updated June 27, 2018, 4:49 p.m.)
>
>
> Review request for sentry, Arjun Mishra, kalyan kumar kalvagadda, and Na Li.
>
>
> Bugs: sentry-2284
> https://issues.apache.org/jira/browse/sentry-2284
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Add two API methods to the Sentry client:
> - listAllRolesPrivileges
> - listAllUsersPrivileges
>
> Both methods return a map object of the form:
> - [roleName, set<privileges>]
> - [userName, set<privileges>]
>
> Unit tests, thrift API code and Client methods are provided.
>
>
> Diffs
> -----
>
> sentry-service/sentry-service-api/src/gen/thrift/gen-javabean/org/apache/sentry/api/service/thrift/SentryPolicyService.java d456d7ff64a8146473ff14a65f06e7cb664939b7
> sentry-service/sentry-service-api/src/gen/thrift/gen-javabean/org/apache/sentry/api/service/thrift/TSentryPrivilegesRequest.java PRE-CREATION
> sentry-service/sentry-service-api/src/gen/thrift/gen-javabean/org/apache/sentry/api/service/thrift/TSentryPrivilegesResponse.java PRE-CREATION
> sentry-service/sentry-service-api/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyServiceClient.java dc1d67ba16c9c7acf76eb3318864ba0606e2aa5a
> sentry-service/sentry-service-api/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyServiceClientDefaultImpl.java 5f76cb0aa3bd17d6aca4adc52ba59ded5ec0b900
> sentry-service/sentry-service-api/src/main/resources/sentry_policy_service.thrift 56aedcb5267495e610e3dace1f38e708d68ffe84
> sentry-service/sentry-service-api/src/test/java/org/apache/sentry/api/service/thrift/TestSentryPolicyServiceClientDefaultImpl.java 1666e326462b771674930e52e3790ca92f467778
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyStoreProcessor.java 39271500df179e0ddf8ce4f1589eaaa8137afa25
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java d8ab1fc90255c6ed42c00e3d3aa6103e47a40b29
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStoreInterface.java f61ae575c99474a76886d3c7a74765fdb067acd6
> sentry-service/sentry-service-server/src/test/java/org/apache/sentry/api/service/thrift/TestSentryPolicyStoreProcessor.java de4e0012d521cf402f4773a04b673f8056a5337c
> sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java 52ce72cebc8b69f08988b05ba54e2bbedd201c1a
>
>
> Diff: https://reviews.apache.org/r/67759/diff/2/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Sergio Pena
>
>
Re: Review Request 67759: SENTRY-2284: Add two client API to get all
roles or users privileges mapping
Posted by Na Li via Review Board <no...@reviews.apache.org>.
> On June 27, 2018, 5 p.m., Arjun Mishra wrote:
> > sentry-service/sentry-service-api/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyServiceClient.java
> > Lines 311 (patched)
> > <https://reviews.apache.org/r/67759/diff/2/?file=2046406#file2046406line311>
> >
> > Sergio, maybe this is the right opportunity to have a single method instead of one for roles and one for users and return something like Map<TSentryPrincipal, Set<TSentryPrivileges>> ?
>
> Sergio Pena wrote:
> I thought about that early, but the intention of this API is that you could get either all roles and privileges or all users and privileges. If we do return all users and roles, then the caller of this API would need to separate roles and users which would take O(n) time where n is the number of roles + users.
>
> For instance:
> Map<TSentryPrincipal, Set<TSentryPrivileges>> privileges = client.list_all_privileges();
>
> Map<String, Set<TSentryPrivileges>> allRoles, allUsers;
> for (TSentryPrincipal princ : privileges.keySet()) {
> if (princ.getType() == SentryPrincipal.ROLE)
> allRoles.put(princ.getName(), privileges.get(princ));
> else if (princ.getType() == SentryPrincipal.USER)
> allUsers.put(princ.getName(), privileges.get(princ));
> else
> ;// principal not supported
> }
>
> With the above code, now the caller can check if a specific role or a specific user has privileges, or display only user privileges or only role privileges.
> That's why I didn't go with the above solution. If I go to the current one, then the caller would not need to split users vs roles, and it will be able to check privileges in O(1).
>
> Another idea is to add in the request to return only user privileges or only role privileges, but how can the caller made sure the returned principals are for user or for roles? They would need to check if the princ.geType() is equals to USER or ROLE everytime just to make sure there were no errors.
>
> So, I prefered to sacrify two API calls instead of 1 to let the caller have a simple data type with either roles or users.
Sergio, there is another approach can address both Arjun's request and your concern.
Map<type, Map<TSentryPrincipal, Set<TSentryPrivileges>>>. Tye can be user or role. Under user, all entries are about user; under role, all entries are about role. So it does not take O(n).
Also, it can save round trip time. Caller can get all privileges for both user and role in one call, instead of two calls, one for user and another for role. It is particularly important when caller is far away from the sentry server
- Na
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67759/#review205452
-----------------------------------------------------------
On June 27, 2018, 7:33 p.m., Sergio Pena wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67759/
> -----------------------------------------------------------
>
> (Updated June 27, 2018, 7:33 p.m.)
>
>
> Review request for sentry, Arjun Mishra, kalyan kumar kalvagadda, and Na Li.
>
>
> Bugs: sentry-2284
> https://issues.apache.org/jira/browse/sentry-2284
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Add two API methods to the Sentry client:
> - listAllRolesPrivileges
> - listAllUsersPrivileges
>
> Both methods return a map object of the form:
> - [roleName, set<privileges>]
> - [userName, set<privileges>]
>
> Unit tests, thrift API code and Client methods are provided.
>
>
> Diffs
> -----
>
> sentry-dist/src/license/THIRD-PARTY.properties b39e1b6ca7eba8c6a7695a4238104af7cd50da32
> sentry-service/sentry-service-api/src/gen/thrift/gen-javabean/org/apache/sentry/api/service/thrift/SentryPolicyService.java d456d7ff64a8146473ff14a65f06e7cb664939b7
> sentry-service/sentry-service-api/src/gen/thrift/gen-javabean/org/apache/sentry/api/service/thrift/TSentryPrivilegesRequest.java PRE-CREATION
> sentry-service/sentry-service-api/src/gen/thrift/gen-javabean/org/apache/sentry/api/service/thrift/TSentryPrivilegesResponse.java PRE-CREATION
> sentry-service/sentry-service-api/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyServiceClient.java dc1d67ba16c9c7acf76eb3318864ba0606e2aa5a
> sentry-service/sentry-service-api/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyServiceClientDefaultImpl.java 5f76cb0aa3bd17d6aca4adc52ba59ded5ec0b900
> sentry-service/sentry-service-api/src/main/resources/sentry_policy_service.thrift 56aedcb5267495e610e3dace1f38e708d68ffe84
> sentry-service/sentry-service-api/src/test/java/org/apache/sentry/api/service/thrift/TestSentryPolicyServiceClientDefaultImpl.java 1666e326462b771674930e52e3790ca92f467778
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyStoreProcessor.java 39271500df179e0ddf8ce4f1589eaaa8137afa25
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java d8ab1fc90255c6ed42c00e3d3aa6103e47a40b29
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStoreInterface.java f61ae575c99474a76886d3c7a74765fdb067acd6
> sentry-service/sentry-service-server/src/test/java/org/apache/sentry/api/service/thrift/TestSentryPolicyStoreProcessor.java de4e0012d521cf402f4773a04b673f8056a5337c
> sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java 52ce72cebc8b69f08988b05ba54e2bbedd201c1a
>
>
> Diff: https://reviews.apache.org/r/67759/diff/3/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Sergio Pena
>
>
Re: Review Request 67759: SENTRY-2284: Add two client API to get all
roles or users privileges mapping
Posted by Arjun Mishra via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67759/#review205452
-----------------------------------------------------------
sentry-service/sentry-service-api/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyServiceClient.java
Lines 311 (patched)
<https://reviews.apache.org/r/67759/#comment288371>
Sergio, maybe this is the right opportunity to have a single method instead of one for roles and one for users and return something like Map<TSentryPrincipal, Set<TSentryPrivileges>> ?
- Arjun Mishra
On June 27, 2018, 4:49 p.m., Sergio Pena wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67759/
> -----------------------------------------------------------
>
> (Updated June 27, 2018, 4:49 p.m.)
>
>
> Review request for sentry, Arjun Mishra, kalyan kumar kalvagadda, and Na Li.
>
>
> Bugs: sentry-2284
> https://issues.apache.org/jira/browse/sentry-2284
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Add two API methods to the Sentry client:
> - listAllRolesPrivileges
> - listAllUsersPrivileges
>
> Both methods return a map object of the form:
> - [roleName, set<privileges>]
> - [userName, set<privileges>]
>
> Unit tests, thrift API code and Client methods are provided.
>
>
> Diffs
> -----
>
> sentry-service/sentry-service-api/src/gen/thrift/gen-javabean/org/apache/sentry/api/service/thrift/SentryPolicyService.java d456d7ff64a8146473ff14a65f06e7cb664939b7
> sentry-service/sentry-service-api/src/gen/thrift/gen-javabean/org/apache/sentry/api/service/thrift/TSentryPrivilegesRequest.java PRE-CREATION
> sentry-service/sentry-service-api/src/gen/thrift/gen-javabean/org/apache/sentry/api/service/thrift/TSentryPrivilegesResponse.java PRE-CREATION
> sentry-service/sentry-service-api/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyServiceClient.java dc1d67ba16c9c7acf76eb3318864ba0606e2aa5a
> sentry-service/sentry-service-api/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyServiceClientDefaultImpl.java 5f76cb0aa3bd17d6aca4adc52ba59ded5ec0b900
> sentry-service/sentry-service-api/src/main/resources/sentry_policy_service.thrift 56aedcb5267495e610e3dace1f38e708d68ffe84
> sentry-service/sentry-service-api/src/test/java/org/apache/sentry/api/service/thrift/TestSentryPolicyServiceClientDefaultImpl.java 1666e326462b771674930e52e3790ca92f467778
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyStoreProcessor.java 39271500df179e0ddf8ce4f1589eaaa8137afa25
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java d8ab1fc90255c6ed42c00e3d3aa6103e47a40b29
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStoreInterface.java f61ae575c99474a76886d3c7a74765fdb067acd6
> sentry-service/sentry-service-server/src/test/java/org/apache/sentry/api/service/thrift/TestSentryPolicyStoreProcessor.java de4e0012d521cf402f4773a04b673f8056a5337c
> sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java 52ce72cebc8b69f08988b05ba54e2bbedd201c1a
>
>
> Diff: https://reviews.apache.org/r/67759/diff/2/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Sergio Pena
>
>
Re: Review Request 67759: SENTRY-2284: Add two client API to get all
roles or users privileges mapping
Posted by Na Li via Review Board <no...@reviews.apache.org>.
> On June 27, 2018, 5:43 p.m., Arjun Mishra wrote:
> > sentry-service/sentry-service-api/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyServiceClient.java
> > Lines 312 (patched)
> > <https://reviews.apache.org/r/67759/diff/2/?file=2046406#file2046406line312>
> >
> > Also we can return TSentryPrivilegeMap as opposed to this. TSentryPrivilegeMap has the map of name to set of privileges
>
> Sergio Pena wrote:
> I saw that and I used it initially, but what is the reason of creating another object just to wrap the map object? I don't see that structure evolving either, so the less data you return the better.
Sergio, you will need TSentryPrivilegeMap if you use the approach I mentioned above. Instead of Map<type, Map<String, Set<TSentryPrivileges>>>, it will be Map<type, TSentryPrivilegeMap>
- Na
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67759/#review205453
-----------------------------------------------------------
On June 27, 2018, 7:33 p.m., Sergio Pena wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67759/
> -----------------------------------------------------------
>
> (Updated June 27, 2018, 7:33 p.m.)
>
>
> Review request for sentry, Arjun Mishra, kalyan kumar kalvagadda, and Na Li.
>
>
> Bugs: sentry-2284
> https://issues.apache.org/jira/browse/sentry-2284
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Add two API methods to the Sentry client:
> - listAllRolesPrivileges
> - listAllUsersPrivileges
>
> Both methods return a map object of the form:
> - [roleName, set<privileges>]
> - [userName, set<privileges>]
>
> Unit tests, thrift API code and Client methods are provided.
>
>
> Diffs
> -----
>
> sentry-dist/src/license/THIRD-PARTY.properties b39e1b6ca7eba8c6a7695a4238104af7cd50da32
> sentry-service/sentry-service-api/src/gen/thrift/gen-javabean/org/apache/sentry/api/service/thrift/SentryPolicyService.java d456d7ff64a8146473ff14a65f06e7cb664939b7
> sentry-service/sentry-service-api/src/gen/thrift/gen-javabean/org/apache/sentry/api/service/thrift/TSentryPrivilegesRequest.java PRE-CREATION
> sentry-service/sentry-service-api/src/gen/thrift/gen-javabean/org/apache/sentry/api/service/thrift/TSentryPrivilegesResponse.java PRE-CREATION
> sentry-service/sentry-service-api/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyServiceClient.java dc1d67ba16c9c7acf76eb3318864ba0606e2aa5a
> sentry-service/sentry-service-api/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyServiceClientDefaultImpl.java 5f76cb0aa3bd17d6aca4adc52ba59ded5ec0b900
> sentry-service/sentry-service-api/src/main/resources/sentry_policy_service.thrift 56aedcb5267495e610e3dace1f38e708d68ffe84
> sentry-service/sentry-service-api/src/test/java/org/apache/sentry/api/service/thrift/TestSentryPolicyServiceClientDefaultImpl.java 1666e326462b771674930e52e3790ca92f467778
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyStoreProcessor.java 39271500df179e0ddf8ce4f1589eaaa8137afa25
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java d8ab1fc90255c6ed42c00e3d3aa6103e47a40b29
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStoreInterface.java f61ae575c99474a76886d3c7a74765fdb067acd6
> sentry-service/sentry-service-server/src/test/java/org/apache/sentry/api/service/thrift/TestSentryPolicyStoreProcessor.java de4e0012d521cf402f4773a04b673f8056a5337c
> sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java 52ce72cebc8b69f08988b05ba54e2bbedd201c1a
>
>
> Diff: https://reviews.apache.org/r/67759/diff/3/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Sergio Pena
>
>
Re: Review Request 67759: SENTRY-2284: Add two client API to get all
roles or users privileges mapping
Posted by Sergio Pena via Review Board <no...@reviews.apache.org>.
> On June 27, 2018, 5:43 p.m., Arjun Mishra wrote:
> > sentry-service/sentry-service-api/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyServiceClient.java
> > Lines 312 (patched)
> > <https://reviews.apache.org/r/67759/diff/2/?file=2046406#file2046406line312>
> >
> > Also we can return TSentryPrivilegeMap as opposed to this. TSentryPrivilegeMap has the map of name to set of privileges
I saw that and I used it initially, but what is the reason of creating another object just to wrap the map object? I don't see that structure evolving either, so the less data you return the better.
- Sergio
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67759/#review205453
-----------------------------------------------------------
On June 27, 2018, 4:49 p.m., Sergio Pena wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67759/
> -----------------------------------------------------------
>
> (Updated June 27, 2018, 4:49 p.m.)
>
>
> Review request for sentry, Arjun Mishra, kalyan kumar kalvagadda, and Na Li.
>
>
> Bugs: sentry-2284
> https://issues.apache.org/jira/browse/sentry-2284
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Add two API methods to the Sentry client:
> - listAllRolesPrivileges
> - listAllUsersPrivileges
>
> Both methods return a map object of the form:
> - [roleName, set<privileges>]
> - [userName, set<privileges>]
>
> Unit tests, thrift API code and Client methods are provided.
>
>
> Diffs
> -----
>
> sentry-service/sentry-service-api/src/gen/thrift/gen-javabean/org/apache/sentry/api/service/thrift/SentryPolicyService.java d456d7ff64a8146473ff14a65f06e7cb664939b7
> sentry-service/sentry-service-api/src/gen/thrift/gen-javabean/org/apache/sentry/api/service/thrift/TSentryPrivilegesRequest.java PRE-CREATION
> sentry-service/sentry-service-api/src/gen/thrift/gen-javabean/org/apache/sentry/api/service/thrift/TSentryPrivilegesResponse.java PRE-CREATION
> sentry-service/sentry-service-api/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyServiceClient.java dc1d67ba16c9c7acf76eb3318864ba0606e2aa5a
> sentry-service/sentry-service-api/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyServiceClientDefaultImpl.java 5f76cb0aa3bd17d6aca4adc52ba59ded5ec0b900
> sentry-service/sentry-service-api/src/main/resources/sentry_policy_service.thrift 56aedcb5267495e610e3dace1f38e708d68ffe84
> sentry-service/sentry-service-api/src/test/java/org/apache/sentry/api/service/thrift/TestSentryPolicyServiceClientDefaultImpl.java 1666e326462b771674930e52e3790ca92f467778
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyStoreProcessor.java 39271500df179e0ddf8ce4f1589eaaa8137afa25
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java d8ab1fc90255c6ed42c00e3d3aa6103e47a40b29
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStoreInterface.java f61ae575c99474a76886d3c7a74765fdb067acd6
> sentry-service/sentry-service-server/src/test/java/org/apache/sentry/api/service/thrift/TestSentryPolicyStoreProcessor.java de4e0012d521cf402f4773a04b673f8056a5337c
> sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java 52ce72cebc8b69f08988b05ba54e2bbedd201c1a
>
>
> Diff: https://reviews.apache.org/r/67759/diff/2/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Sergio Pena
>
>
Re: Review Request 67759: SENTRY-2284: Add two client API to get all
roles or users privileges mapping
Posted by Arjun Mishra via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67759/#review205453
-----------------------------------------------------------
sentry-service/sentry-service-api/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyServiceClient.java
Lines 312 (patched)
<https://reviews.apache.org/r/67759/#comment288372>
Also we can return TSentryPrivilegeMap as opposed to this. TSentryPrivilegeMap has the map of name to set of privileges
- Arjun Mishra
On June 27, 2018, 4:49 p.m., Sergio Pena wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67759/
> -----------------------------------------------------------
>
> (Updated June 27, 2018, 4:49 p.m.)
>
>
> Review request for sentry, Arjun Mishra, kalyan kumar kalvagadda, and Na Li.
>
>
> Bugs: sentry-2284
> https://issues.apache.org/jira/browse/sentry-2284
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Add two API methods to the Sentry client:
> - listAllRolesPrivileges
> - listAllUsersPrivileges
>
> Both methods return a map object of the form:
> - [roleName, set<privileges>]
> - [userName, set<privileges>]
>
> Unit tests, thrift API code and Client methods are provided.
>
>
> Diffs
> -----
>
> sentry-service/sentry-service-api/src/gen/thrift/gen-javabean/org/apache/sentry/api/service/thrift/SentryPolicyService.java d456d7ff64a8146473ff14a65f06e7cb664939b7
> sentry-service/sentry-service-api/src/gen/thrift/gen-javabean/org/apache/sentry/api/service/thrift/TSentryPrivilegesRequest.java PRE-CREATION
> sentry-service/sentry-service-api/src/gen/thrift/gen-javabean/org/apache/sentry/api/service/thrift/TSentryPrivilegesResponse.java PRE-CREATION
> sentry-service/sentry-service-api/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyServiceClient.java dc1d67ba16c9c7acf76eb3318864ba0606e2aa5a
> sentry-service/sentry-service-api/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyServiceClientDefaultImpl.java 5f76cb0aa3bd17d6aca4adc52ba59ded5ec0b900
> sentry-service/sentry-service-api/src/main/resources/sentry_policy_service.thrift 56aedcb5267495e610e3dace1f38e708d68ffe84
> sentry-service/sentry-service-api/src/test/java/org/apache/sentry/api/service/thrift/TestSentryPolicyServiceClientDefaultImpl.java 1666e326462b771674930e52e3790ca92f467778
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyStoreProcessor.java 39271500df179e0ddf8ce4f1589eaaa8137afa25
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java d8ab1fc90255c6ed42c00e3d3aa6103e47a40b29
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStoreInterface.java f61ae575c99474a76886d3c7a74765fdb067acd6
> sentry-service/sentry-service-server/src/test/java/org/apache/sentry/api/service/thrift/TestSentryPolicyStoreProcessor.java de4e0012d521cf402f4773a04b673f8056a5337c
> sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java 52ce72cebc8b69f08988b05ba54e2bbedd201c1a
>
>
> Diff: https://reviews.apache.org/r/67759/diff/2/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Sergio Pena
>
>
Re: Review Request 67759: SENTRY-2284: Add two client API to get all
roles or users privileges mapping
Posted by Sergio Pena via Review Board <no...@reviews.apache.org>.
> On June 27, 2018, 6:13 p.m., Arjun Mishra wrote:
> > sentry-service/sentry-service-api/src/gen/thrift/gen-javabean/org/apache/sentry/api/service/thrift/TSentryPrivilegesResponse.java
> > Lines 59 (patched)
> > <https://reviews.apache.org/r/67759/diff/2/?file=2046405#file2046405line59>
> >
> > Can we have TSentryPrivilegeMap instead here?
This is answered in a previous question.
- Sergio
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67759/#review205454
-----------------------------------------------------------
On June 27, 2018, 4:49 p.m., Sergio Pena wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67759/
> -----------------------------------------------------------
>
> (Updated June 27, 2018, 4:49 p.m.)
>
>
> Review request for sentry, Arjun Mishra, kalyan kumar kalvagadda, and Na Li.
>
>
> Bugs: sentry-2284
> https://issues.apache.org/jira/browse/sentry-2284
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Add two API methods to the Sentry client:
> - listAllRolesPrivileges
> - listAllUsersPrivileges
>
> Both methods return a map object of the form:
> - [roleName, set<privileges>]
> - [userName, set<privileges>]
>
> Unit tests, thrift API code and Client methods are provided.
>
>
> Diffs
> -----
>
> sentry-service/sentry-service-api/src/gen/thrift/gen-javabean/org/apache/sentry/api/service/thrift/SentryPolicyService.java d456d7ff64a8146473ff14a65f06e7cb664939b7
> sentry-service/sentry-service-api/src/gen/thrift/gen-javabean/org/apache/sentry/api/service/thrift/TSentryPrivilegesRequest.java PRE-CREATION
> sentry-service/sentry-service-api/src/gen/thrift/gen-javabean/org/apache/sentry/api/service/thrift/TSentryPrivilegesResponse.java PRE-CREATION
> sentry-service/sentry-service-api/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyServiceClient.java dc1d67ba16c9c7acf76eb3318864ba0606e2aa5a
> sentry-service/sentry-service-api/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyServiceClientDefaultImpl.java 5f76cb0aa3bd17d6aca4adc52ba59ded5ec0b900
> sentry-service/sentry-service-api/src/main/resources/sentry_policy_service.thrift 56aedcb5267495e610e3dace1f38e708d68ffe84
> sentry-service/sentry-service-api/src/test/java/org/apache/sentry/api/service/thrift/TestSentryPolicyServiceClientDefaultImpl.java 1666e326462b771674930e52e3790ca92f467778
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyStoreProcessor.java 39271500df179e0ddf8ce4f1589eaaa8137afa25
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java d8ab1fc90255c6ed42c00e3d3aa6103e47a40b29
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStoreInterface.java f61ae575c99474a76886d3c7a74765fdb067acd6
> sentry-service/sentry-service-server/src/test/java/org/apache/sentry/api/service/thrift/TestSentryPolicyStoreProcessor.java de4e0012d521cf402f4773a04b673f8056a5337c
> sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java 52ce72cebc8b69f08988b05ba54e2bbedd201c1a
>
>
> Diff: https://reviews.apache.org/r/67759/diff/2/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Sergio Pena
>
>
Re: Review Request 67759: SENTRY-2284: Add two client API to get all
roles or users privileges mapping
Posted by Arjun Mishra via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67759/#review205454
-----------------------------------------------------------
sentry-service/sentry-service-api/src/gen/thrift/gen-javabean/org/apache/sentry/api/service/thrift/TSentryPrivilegesResponse.java
Lines 59 (patched)
<https://reviews.apache.org/r/67759/#comment288373>
Can we have TSentryPrivilegeMap instead here?
- Arjun Mishra
On June 27, 2018, 4:49 p.m., Sergio Pena wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67759/
> -----------------------------------------------------------
>
> (Updated June 27, 2018, 4:49 p.m.)
>
>
> Review request for sentry, Arjun Mishra, kalyan kumar kalvagadda, and Na Li.
>
>
> Bugs: sentry-2284
> https://issues.apache.org/jira/browse/sentry-2284
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Add two API methods to the Sentry client:
> - listAllRolesPrivileges
> - listAllUsersPrivileges
>
> Both methods return a map object of the form:
> - [roleName, set<privileges>]
> - [userName, set<privileges>]
>
> Unit tests, thrift API code and Client methods are provided.
>
>
> Diffs
> -----
>
> sentry-service/sentry-service-api/src/gen/thrift/gen-javabean/org/apache/sentry/api/service/thrift/SentryPolicyService.java d456d7ff64a8146473ff14a65f06e7cb664939b7
> sentry-service/sentry-service-api/src/gen/thrift/gen-javabean/org/apache/sentry/api/service/thrift/TSentryPrivilegesRequest.java PRE-CREATION
> sentry-service/sentry-service-api/src/gen/thrift/gen-javabean/org/apache/sentry/api/service/thrift/TSentryPrivilegesResponse.java PRE-CREATION
> sentry-service/sentry-service-api/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyServiceClient.java dc1d67ba16c9c7acf76eb3318864ba0606e2aa5a
> sentry-service/sentry-service-api/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyServiceClientDefaultImpl.java 5f76cb0aa3bd17d6aca4adc52ba59ded5ec0b900
> sentry-service/sentry-service-api/src/main/resources/sentry_policy_service.thrift 56aedcb5267495e610e3dace1f38e708d68ffe84
> sentry-service/sentry-service-api/src/test/java/org/apache/sentry/api/service/thrift/TestSentryPolicyServiceClientDefaultImpl.java 1666e326462b771674930e52e3790ca92f467778
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyStoreProcessor.java 39271500df179e0ddf8ce4f1589eaaa8137afa25
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java d8ab1fc90255c6ed42c00e3d3aa6103e47a40b29
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStoreInterface.java f61ae575c99474a76886d3c7a74765fdb067acd6
> sentry-service/sentry-service-server/src/test/java/org/apache/sentry/api/service/thrift/TestSentryPolicyStoreProcessor.java de4e0012d521cf402f4773a04b673f8056a5337c
> sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java 52ce72cebc8b69f08988b05ba54e2bbedd201c1a
>
>
> Diff: https://reviews.apache.org/r/67759/diff/2/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Sergio Pena
>
>
Re: Review Request 67759: SENTRY-2284: Add two client API to get all
roles or users privileges mapping
Posted by Sergio Pena via Review Board <no...@reviews.apache.org>.
> On June 27, 2018, 6:15 p.m., Arjun Mishra wrote:
> > sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
> > Lines 4729 (patched)
> > <https://reviews.apache.org/r/67759/diff/2/?file=2046411#file2046411line4729>
> >
> > I think we'll hit performance issues with these two methods. Doing mSentryRole.getPrivileges() or mSentryUser.getPrivileges() will be slow. Do we know if this fetch is lazy?
> >
> > I think we should mimic or improve what is in getMSentryPrivilegesByAuth()
>
> Sergio Pena wrote:
> The intention of the API is to return a map between name -> set<privileges>. That means that I should request all roles or users of the system, and get the privileges assigned to them and put them in a map.
> If I do mimic getMSentryPrivilegesByAuth, then that will return all privileges -> map mixed with roles and users, and then I will need to walk through all of them to map name -> set<privileges>.
>
> Arjun Mishra wrote:
> But you pass in EntityType right. WOuld you still get privileges for all roles and users?
>
> Sergio Pena wrote:
> getMSentryPrivilegesByAuth() returns a set of privileges only, not a map. That means that if a request only by EntityType.ROLE, then it will return all privileges for all roles, but then I will have to map to role to a privilege. That will require O(n) to walk through the set of privileges and then separate them into a map roleName -> privileges.
>
> Btw, I added the fetchGroup to avoid lazy fetching and hit a performance issue.
>
> Na Li wrote:
> Notice that "default-fetch-group="true"" for role, so when getting role from DB, the privileges will be get as well, not lazy load.
>
> <class name="MSentryRole" identity-type="datastore" table="SENTRY_ROLE" detachable="true">
> <datastore-identity>
> <column name="ROLE_ID"/>
> </datastore-identity>
> <field name="roleName">
> <column name="ROLE_NAME" length="128" jdbc-type="VARCHAR"/>
> <index name="SentryRoleName" unique="true"/>
> </field>
> <field name = "createTime">
> <column name = "CREATE_TIME" jdbc-type="BIGINT"/>
> </field>
> <field name = "privileges" table="SENTRY_ROLE_DB_PRIVILEGE_MAP" default-fetch-group="true">
> <collection element-type="org.apache.sentry.provider.db.service.model.MSentryPrivilege"/>
> <join>
> <column name="ROLE_ID"/>
> </join>
> <element>
> <column name="DB_PRIVILEGE_ID"/>
> </element>
> </field>
So the fetch group is already enabled, then I don't need the change onthe patch #3.
Lina, are you ok on patch #2 then?
- Sergio
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67759/#review205455
-----------------------------------------------------------
On June 27, 2018, 7:33 p.m., Sergio Pena wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67759/
> -----------------------------------------------------------
>
> (Updated June 27, 2018, 7:33 p.m.)
>
>
> Review request for sentry, Arjun Mishra, kalyan kumar kalvagadda, and Na Li.
>
>
> Bugs: sentry-2284
> https://issues.apache.org/jira/browse/sentry-2284
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Add two API methods to the Sentry client:
> - listAllRolesPrivileges
> - listAllUsersPrivileges
>
> Both methods return a map object of the form:
> - [roleName, set<privileges>]
> - [userName, set<privileges>]
>
> Unit tests, thrift API code and Client methods are provided.
>
>
> Diffs
> -----
>
> sentry-dist/src/license/THIRD-PARTY.properties b39e1b6ca7eba8c6a7695a4238104af7cd50da32
> sentry-service/sentry-service-api/src/gen/thrift/gen-javabean/org/apache/sentry/api/service/thrift/SentryPolicyService.java d456d7ff64a8146473ff14a65f06e7cb664939b7
> sentry-service/sentry-service-api/src/gen/thrift/gen-javabean/org/apache/sentry/api/service/thrift/TSentryPrivilegesRequest.java PRE-CREATION
> sentry-service/sentry-service-api/src/gen/thrift/gen-javabean/org/apache/sentry/api/service/thrift/TSentryPrivilegesResponse.java PRE-CREATION
> sentry-service/sentry-service-api/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyServiceClient.java dc1d67ba16c9c7acf76eb3318864ba0606e2aa5a
> sentry-service/sentry-service-api/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyServiceClientDefaultImpl.java 5f76cb0aa3bd17d6aca4adc52ba59ded5ec0b900
> sentry-service/sentry-service-api/src/main/resources/sentry_policy_service.thrift 56aedcb5267495e610e3dace1f38e708d68ffe84
> sentry-service/sentry-service-api/src/test/java/org/apache/sentry/api/service/thrift/TestSentryPolicyServiceClientDefaultImpl.java 1666e326462b771674930e52e3790ca92f467778
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyStoreProcessor.java 39271500df179e0ddf8ce4f1589eaaa8137afa25
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java d8ab1fc90255c6ed42c00e3d3aa6103e47a40b29
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStoreInterface.java f61ae575c99474a76886d3c7a74765fdb067acd6
> sentry-service/sentry-service-server/src/test/java/org/apache/sentry/api/service/thrift/TestSentryPolicyStoreProcessor.java de4e0012d521cf402f4773a04b673f8056a5337c
> sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java 52ce72cebc8b69f08988b05ba54e2bbedd201c1a
>
>
> Diff: https://reviews.apache.org/r/67759/diff/3/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Sergio Pena
>
>
Re: Review Request 67759: SENTRY-2284: Add two client API to get all
roles or users privileges mapping
Posted by Arjun Mishra via Review Board <no...@reviews.apache.org>.
> On June 27, 2018, 6:15 p.m., Arjun Mishra wrote:
> > sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
> > Lines 4729 (patched)
> > <https://reviews.apache.org/r/67759/diff/2/?file=2046411#file2046411line4729>
> >
> > I think we'll hit performance issues with these two methods. Doing mSentryRole.getPrivileges() or mSentryUser.getPrivileges() will be slow. Do we know if this fetch is lazy?
> >
> > I think we should mimic or improve what is in getMSentryPrivilegesByAuth()
>
> Sergio Pena wrote:
> The intention of the API is to return a map between name -> set<privileges>. That means that I should request all roles or users of the system, and get the privileges assigned to them and put them in a map.
> If I do mimic getMSentryPrivilegesByAuth, then that will return all privileges -> map mixed with roles and users, and then I will need to walk through all of them to map name -> set<privileges>.
But you pass in EntityType right. WOuld you still get privileges for all roles and users?
- Arjun
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67759/#review205455
-----------------------------------------------------------
On June 27, 2018, 4:49 p.m., Sergio Pena wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67759/
> -----------------------------------------------------------
>
> (Updated June 27, 2018, 4:49 p.m.)
>
>
> Review request for sentry, Arjun Mishra, kalyan kumar kalvagadda, and Na Li.
>
>
> Bugs: sentry-2284
> https://issues.apache.org/jira/browse/sentry-2284
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Add two API methods to the Sentry client:
> - listAllRolesPrivileges
> - listAllUsersPrivileges
>
> Both methods return a map object of the form:
> - [roleName, set<privileges>]
> - [userName, set<privileges>]
>
> Unit tests, thrift API code and Client methods are provided.
>
>
> Diffs
> -----
>
> sentry-service/sentry-service-api/src/gen/thrift/gen-javabean/org/apache/sentry/api/service/thrift/SentryPolicyService.java d456d7ff64a8146473ff14a65f06e7cb664939b7
> sentry-service/sentry-service-api/src/gen/thrift/gen-javabean/org/apache/sentry/api/service/thrift/TSentryPrivilegesRequest.java PRE-CREATION
> sentry-service/sentry-service-api/src/gen/thrift/gen-javabean/org/apache/sentry/api/service/thrift/TSentryPrivilegesResponse.java PRE-CREATION
> sentry-service/sentry-service-api/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyServiceClient.java dc1d67ba16c9c7acf76eb3318864ba0606e2aa5a
> sentry-service/sentry-service-api/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyServiceClientDefaultImpl.java 5f76cb0aa3bd17d6aca4adc52ba59ded5ec0b900
> sentry-service/sentry-service-api/src/main/resources/sentry_policy_service.thrift 56aedcb5267495e610e3dace1f38e708d68ffe84
> sentry-service/sentry-service-api/src/test/java/org/apache/sentry/api/service/thrift/TestSentryPolicyServiceClientDefaultImpl.java 1666e326462b771674930e52e3790ca92f467778
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyStoreProcessor.java 39271500df179e0ddf8ce4f1589eaaa8137afa25
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java d8ab1fc90255c6ed42c00e3d3aa6103e47a40b29
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStoreInterface.java f61ae575c99474a76886d3c7a74765fdb067acd6
> sentry-service/sentry-service-server/src/test/java/org/apache/sentry/api/service/thrift/TestSentryPolicyStoreProcessor.java de4e0012d521cf402f4773a04b673f8056a5337c
> sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java 52ce72cebc8b69f08988b05ba54e2bbedd201c1a
>
>
> Diff: https://reviews.apache.org/r/67759/diff/2/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Sergio Pena
>
>
Re: Review Request 67759: SENTRY-2284: Add two client API to get all
roles or users privileges mapping
Posted by Sergio Pena via Review Board <no...@reviews.apache.org>.
> On June 27, 2018, 6:15 p.m., Arjun Mishra wrote:
> > sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
> > Lines 4729 (patched)
> > <https://reviews.apache.org/r/67759/diff/2/?file=2046411#file2046411line4729>
> >
> > I think we'll hit performance issues with these two methods. Doing mSentryRole.getPrivileges() or mSentryUser.getPrivileges() will be slow. Do we know if this fetch is lazy?
> >
> > I think we should mimic or improve what is in getMSentryPrivilegesByAuth()
>
> Sergio Pena wrote:
> The intention of the API is to return a map between name -> set<privileges>. That means that I should request all roles or users of the system, and get the privileges assigned to them and put them in a map.
> If I do mimic getMSentryPrivilegesByAuth, then that will return all privileges -> map mixed with roles and users, and then I will need to walk through all of them to map name -> set<privileges>.
>
> Arjun Mishra wrote:
> But you pass in EntityType right. WOuld you still get privileges for all roles and users?
getMSentryPrivilegesByAuth() returns a set of privileges only, not a map. That means that if a request only by EntityType.ROLE, then it will return all privileges for all roles, but then I will have to map to role to a privilege. That will require O(n) to walk through the set of privileges and then separate them into a map roleName -> privileges.
Btw, I added the fetchGroup to avoid lazy fetching and hit a performance issue.
- Sergio
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67759/#review205455
-----------------------------------------------------------
On June 27, 2018, 4:49 p.m., Sergio Pena wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67759/
> -----------------------------------------------------------
>
> (Updated June 27, 2018, 4:49 p.m.)
>
>
> Review request for sentry, Arjun Mishra, kalyan kumar kalvagadda, and Na Li.
>
>
> Bugs: sentry-2284
> https://issues.apache.org/jira/browse/sentry-2284
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Add two API methods to the Sentry client:
> - listAllRolesPrivileges
> - listAllUsersPrivileges
>
> Both methods return a map object of the form:
> - [roleName, set<privileges>]
> - [userName, set<privileges>]
>
> Unit tests, thrift API code and Client methods are provided.
>
>
> Diffs
> -----
>
> sentry-service/sentry-service-api/src/gen/thrift/gen-javabean/org/apache/sentry/api/service/thrift/SentryPolicyService.java d456d7ff64a8146473ff14a65f06e7cb664939b7
> sentry-service/sentry-service-api/src/gen/thrift/gen-javabean/org/apache/sentry/api/service/thrift/TSentryPrivilegesRequest.java PRE-CREATION
> sentry-service/sentry-service-api/src/gen/thrift/gen-javabean/org/apache/sentry/api/service/thrift/TSentryPrivilegesResponse.java PRE-CREATION
> sentry-service/sentry-service-api/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyServiceClient.java dc1d67ba16c9c7acf76eb3318864ba0606e2aa5a
> sentry-service/sentry-service-api/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyServiceClientDefaultImpl.java 5f76cb0aa3bd17d6aca4adc52ba59ded5ec0b900
> sentry-service/sentry-service-api/src/main/resources/sentry_policy_service.thrift 56aedcb5267495e610e3dace1f38e708d68ffe84
> sentry-service/sentry-service-api/src/test/java/org/apache/sentry/api/service/thrift/TestSentryPolicyServiceClientDefaultImpl.java 1666e326462b771674930e52e3790ca92f467778
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyStoreProcessor.java 39271500df179e0ddf8ce4f1589eaaa8137afa25
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java d8ab1fc90255c6ed42c00e3d3aa6103e47a40b29
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStoreInterface.java f61ae575c99474a76886d3c7a74765fdb067acd6
> sentry-service/sentry-service-server/src/test/java/org/apache/sentry/api/service/thrift/TestSentryPolicyStoreProcessor.java de4e0012d521cf402f4773a04b673f8056a5337c
> sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java 52ce72cebc8b69f08988b05ba54e2bbedd201c1a
>
>
> Diff: https://reviews.apache.org/r/67759/diff/2/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Sergio Pena
>
>
Re: Review Request 67759: SENTRY-2284: Add two client API to get all
roles or users privileges mapping
Posted by Na Li via Review Board <no...@reviews.apache.org>.
> On June 27, 2018, 6:15 p.m., Arjun Mishra wrote:
> > sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
> > Lines 4729 (patched)
> > <https://reviews.apache.org/r/67759/diff/2/?file=2046411#file2046411line4729>
> >
> > I think we'll hit performance issues with these two methods. Doing mSentryRole.getPrivileges() or mSentryUser.getPrivileges() will be slow. Do we know if this fetch is lazy?
> >
> > I think we should mimic or improve what is in getMSentryPrivilegesByAuth()
>
> Sergio Pena wrote:
> The intention of the API is to return a map between name -> set<privileges>. That means that I should request all roles or users of the system, and get the privileges assigned to them and put them in a map.
> If I do mimic getMSentryPrivilegesByAuth, then that will return all privileges -> map mixed with roles and users, and then I will need to walk through all of them to map name -> set<privileges>.
>
> Arjun Mishra wrote:
> But you pass in EntityType right. WOuld you still get privileges for all roles and users?
>
> Sergio Pena wrote:
> getMSentryPrivilegesByAuth() returns a set of privileges only, not a map. That means that if a request only by EntityType.ROLE, then it will return all privileges for all roles, but then I will have to map to role to a privilege. That will require O(n) to walk through the set of privileges and then separate them into a map roleName -> privileges.
>
> Btw, I added the fetchGroup to avoid lazy fetching and hit a performance issue.
Notice that "default-fetch-group="true"" for role, so when getting role from DB, the privileges will be get as well, not lazy load.
<class name="MSentryRole" identity-type="datastore" table="SENTRY_ROLE" detachable="true">
<datastore-identity>
<column name="ROLE_ID"/>
</datastore-identity>
<field name="roleName">
<column name="ROLE_NAME" length="128" jdbc-type="VARCHAR"/>
<index name="SentryRoleName" unique="true"/>
</field>
<field name = "createTime">
<column name = "CREATE_TIME" jdbc-type="BIGINT"/>
</field>
<field name = "privileges" table="SENTRY_ROLE_DB_PRIVILEGE_MAP" default-fetch-group="true">
<collection element-type="org.apache.sentry.provider.db.service.model.MSentryPrivilege"/>
<join>
<column name="ROLE_ID"/>
</join>
<element>
<column name="DB_PRIVILEGE_ID"/>
</element>
</field>
- Na
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67759/#review205455
-----------------------------------------------------------
On June 27, 2018, 7:33 p.m., Sergio Pena wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67759/
> -----------------------------------------------------------
>
> (Updated June 27, 2018, 7:33 p.m.)
>
>
> Review request for sentry, Arjun Mishra, kalyan kumar kalvagadda, and Na Li.
>
>
> Bugs: sentry-2284
> https://issues.apache.org/jira/browse/sentry-2284
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Add two API methods to the Sentry client:
> - listAllRolesPrivileges
> - listAllUsersPrivileges
>
> Both methods return a map object of the form:
> - [roleName, set<privileges>]
> - [userName, set<privileges>]
>
> Unit tests, thrift API code and Client methods are provided.
>
>
> Diffs
> -----
>
> sentry-dist/src/license/THIRD-PARTY.properties b39e1b6ca7eba8c6a7695a4238104af7cd50da32
> sentry-service/sentry-service-api/src/gen/thrift/gen-javabean/org/apache/sentry/api/service/thrift/SentryPolicyService.java d456d7ff64a8146473ff14a65f06e7cb664939b7
> sentry-service/sentry-service-api/src/gen/thrift/gen-javabean/org/apache/sentry/api/service/thrift/TSentryPrivilegesRequest.java PRE-CREATION
> sentry-service/sentry-service-api/src/gen/thrift/gen-javabean/org/apache/sentry/api/service/thrift/TSentryPrivilegesResponse.java PRE-CREATION
> sentry-service/sentry-service-api/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyServiceClient.java dc1d67ba16c9c7acf76eb3318864ba0606e2aa5a
> sentry-service/sentry-service-api/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyServiceClientDefaultImpl.java 5f76cb0aa3bd17d6aca4adc52ba59ded5ec0b900
> sentry-service/sentry-service-api/src/main/resources/sentry_policy_service.thrift 56aedcb5267495e610e3dace1f38e708d68ffe84
> sentry-service/sentry-service-api/src/test/java/org/apache/sentry/api/service/thrift/TestSentryPolicyServiceClientDefaultImpl.java 1666e326462b771674930e52e3790ca92f467778
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyStoreProcessor.java 39271500df179e0ddf8ce4f1589eaaa8137afa25
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java d8ab1fc90255c6ed42c00e3d3aa6103e47a40b29
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStoreInterface.java f61ae575c99474a76886d3c7a74765fdb067acd6
> sentry-service/sentry-service-server/src/test/java/org/apache/sentry/api/service/thrift/TestSentryPolicyStoreProcessor.java de4e0012d521cf402f4773a04b673f8056a5337c
> sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java 52ce72cebc8b69f08988b05ba54e2bbedd201c1a
>
>
> Diff: https://reviews.apache.org/r/67759/diff/3/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Sergio Pena
>
>
Re: Review Request 67759: SENTRY-2284: Add two client API to get all
roles or users privileges mapping
Posted by Sergio Pena via Review Board <no...@reviews.apache.org>.
> On June 27, 2018, 6:15 p.m., Arjun Mishra wrote:
> > sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
> > Lines 4729 (patched)
> > <https://reviews.apache.org/r/67759/diff/2/?file=2046411#file2046411line4729>
> >
> > I think we'll hit performance issues with these two methods. Doing mSentryRole.getPrivileges() or mSentryUser.getPrivileges() will be slow. Do we know if this fetch is lazy?
> >
> > I think we should mimic or improve what is in getMSentryPrivilegesByAuth()
The intention of the API is to return a map between name -> set<privileges>. That means that I should request all roles or users of the system, and get the privileges assigned to them and put them in a map.
If I do mimic getMSentryPrivilegesByAuth, then that will return all privileges -> map mixed with roles and users, and then I will need to walk through all of them to map name -> set<privileges>.
- Sergio
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67759/#review205455
-----------------------------------------------------------
On June 27, 2018, 4:49 p.m., Sergio Pena wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67759/
> -----------------------------------------------------------
>
> (Updated June 27, 2018, 4:49 p.m.)
>
>
> Review request for sentry, Arjun Mishra, kalyan kumar kalvagadda, and Na Li.
>
>
> Bugs: sentry-2284
> https://issues.apache.org/jira/browse/sentry-2284
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Add two API methods to the Sentry client:
> - listAllRolesPrivileges
> - listAllUsersPrivileges
>
> Both methods return a map object of the form:
> - [roleName, set<privileges>]
> - [userName, set<privileges>]
>
> Unit tests, thrift API code and Client methods are provided.
>
>
> Diffs
> -----
>
> sentry-service/sentry-service-api/src/gen/thrift/gen-javabean/org/apache/sentry/api/service/thrift/SentryPolicyService.java d456d7ff64a8146473ff14a65f06e7cb664939b7
> sentry-service/sentry-service-api/src/gen/thrift/gen-javabean/org/apache/sentry/api/service/thrift/TSentryPrivilegesRequest.java PRE-CREATION
> sentry-service/sentry-service-api/src/gen/thrift/gen-javabean/org/apache/sentry/api/service/thrift/TSentryPrivilegesResponse.java PRE-CREATION
> sentry-service/sentry-service-api/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyServiceClient.java dc1d67ba16c9c7acf76eb3318864ba0606e2aa5a
> sentry-service/sentry-service-api/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyServiceClientDefaultImpl.java 5f76cb0aa3bd17d6aca4adc52ba59ded5ec0b900
> sentry-service/sentry-service-api/src/main/resources/sentry_policy_service.thrift 56aedcb5267495e610e3dace1f38e708d68ffe84
> sentry-service/sentry-service-api/src/test/java/org/apache/sentry/api/service/thrift/TestSentryPolicyServiceClientDefaultImpl.java 1666e326462b771674930e52e3790ca92f467778
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyStoreProcessor.java 39271500df179e0ddf8ce4f1589eaaa8137afa25
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java d8ab1fc90255c6ed42c00e3d3aa6103e47a40b29
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStoreInterface.java f61ae575c99474a76886d3c7a74765fdb067acd6
> sentry-service/sentry-service-server/src/test/java/org/apache/sentry/api/service/thrift/TestSentryPolicyStoreProcessor.java de4e0012d521cf402f4773a04b673f8056a5337c
> sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java 52ce72cebc8b69f08988b05ba54e2bbedd201c1a
>
>
> Diff: https://reviews.apache.org/r/67759/diff/2/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Sergio Pena
>
>
Re: Review Request 67759: SENTRY-2284: Add two client API to get all
roles or users privileges mapping
Posted by Arjun Mishra via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67759/#review205455
-----------------------------------------------------------
sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
Lines 4729 (patched)
<https://reviews.apache.org/r/67759/#comment288374>
I think we'll hit performance issues with these two methods. Doing mSentryRole.getPrivileges() or mSentryUser.getPrivileges() will be slow. Do we know if this fetch is lazy?
I think we should mimic or improve what is in getMSentryPrivilegesByAuth()
- Arjun Mishra
On June 27, 2018, 4:49 p.m., Sergio Pena wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67759/
> -----------------------------------------------------------
>
> (Updated June 27, 2018, 4:49 p.m.)
>
>
> Review request for sentry, Arjun Mishra, kalyan kumar kalvagadda, and Na Li.
>
>
> Bugs: sentry-2284
> https://issues.apache.org/jira/browse/sentry-2284
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Add two API methods to the Sentry client:
> - listAllRolesPrivileges
> - listAllUsersPrivileges
>
> Both methods return a map object of the form:
> - [roleName, set<privileges>]
> - [userName, set<privileges>]
>
> Unit tests, thrift API code and Client methods are provided.
>
>
> Diffs
> -----
>
> sentry-service/sentry-service-api/src/gen/thrift/gen-javabean/org/apache/sentry/api/service/thrift/SentryPolicyService.java d456d7ff64a8146473ff14a65f06e7cb664939b7
> sentry-service/sentry-service-api/src/gen/thrift/gen-javabean/org/apache/sentry/api/service/thrift/TSentryPrivilegesRequest.java PRE-CREATION
> sentry-service/sentry-service-api/src/gen/thrift/gen-javabean/org/apache/sentry/api/service/thrift/TSentryPrivilegesResponse.java PRE-CREATION
> sentry-service/sentry-service-api/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyServiceClient.java dc1d67ba16c9c7acf76eb3318864ba0606e2aa5a
> sentry-service/sentry-service-api/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyServiceClientDefaultImpl.java 5f76cb0aa3bd17d6aca4adc52ba59ded5ec0b900
> sentry-service/sentry-service-api/src/main/resources/sentry_policy_service.thrift 56aedcb5267495e610e3dace1f38e708d68ffe84
> sentry-service/sentry-service-api/src/test/java/org/apache/sentry/api/service/thrift/TestSentryPolicyServiceClientDefaultImpl.java 1666e326462b771674930e52e3790ca92f467778
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyStoreProcessor.java 39271500df179e0ddf8ce4f1589eaaa8137afa25
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java d8ab1fc90255c6ed42c00e3d3aa6103e47a40b29
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStoreInterface.java f61ae575c99474a76886d3c7a74765fdb067acd6
> sentry-service/sentry-service-server/src/test/java/org/apache/sentry/api/service/thrift/TestSentryPolicyStoreProcessor.java de4e0012d521cf402f4773a04b673f8056a5337c
> sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java 52ce72cebc8b69f08988b05ba54e2bbedd201c1a
>
>
> Diff: https://reviews.apache.org/r/67759/diff/2/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Sergio Pena
>
>
Re: Review Request 67759: SENTRY-2284: Add two client API to get all
roles or users privileges mapping
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/67759/
-----------------------------------------------------------
(Updated June 27, 2018, 4:49 p.m.)
Review request for sentry, Arjun Mishra, kalyan kumar kalvagadda, and Na Li.
Bugs: sentry-2284
https://issues.apache.org/jira/browse/sentry-2284
Repository: sentry
Description
-------
Add two API methods to the Sentry client:
- listAllRolesPrivileges
- listAllUsersPrivileges
Both methods return a map object of the form:
- [roleName, set<privileges>]
- [userName, set<privileges>]
Unit tests, thrift API code and Client methods are provided.
Diffs (updated)
-----
sentry-service/sentry-service-api/src/gen/thrift/gen-javabean/org/apache/sentry/api/service/thrift/SentryPolicyService.java d456d7ff64a8146473ff14a65f06e7cb664939b7
sentry-service/sentry-service-api/src/gen/thrift/gen-javabean/org/apache/sentry/api/service/thrift/TSentryPrivilegesRequest.java PRE-CREATION
sentry-service/sentry-service-api/src/gen/thrift/gen-javabean/org/apache/sentry/api/service/thrift/TSentryPrivilegesResponse.java PRE-CREATION
sentry-service/sentry-service-api/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyServiceClient.java dc1d67ba16c9c7acf76eb3318864ba0606e2aa5a
sentry-service/sentry-service-api/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyServiceClientDefaultImpl.java 5f76cb0aa3bd17d6aca4adc52ba59ded5ec0b900
sentry-service/sentry-service-api/src/main/resources/sentry_policy_service.thrift 56aedcb5267495e610e3dace1f38e708d68ffe84
sentry-service/sentry-service-api/src/test/java/org/apache/sentry/api/service/thrift/TestSentryPolicyServiceClientDefaultImpl.java 1666e326462b771674930e52e3790ca92f467778
sentry-service/sentry-service-server/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyStoreProcessor.java 39271500df179e0ddf8ce4f1589eaaa8137afa25
sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java d8ab1fc90255c6ed42c00e3d3aa6103e47a40b29
sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStoreInterface.java f61ae575c99474a76886d3c7a74765fdb067acd6
sentry-service/sentry-service-server/src/test/java/org/apache/sentry/api/service/thrift/TestSentryPolicyStoreProcessor.java de4e0012d521cf402f4773a04b673f8056a5337c
sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java 52ce72cebc8b69f08988b05ba54e2bbedd201c1a
Diff: https://reviews.apache.org/r/67759/diff/2/
Changes: https://reviews.apache.org/r/67759/diff/1-2/
Testing
-------
Thanks,
Sergio Pena
Re: Review Request 67759: SENTRY-2284: Add two client API to get all
roles or users privileges mapping
Posted by Arjun Mishra via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67759/#review205448
-----------------------------------------------------------
sentry-dist/src/license/THIRD-PARTY.properties
Line 32 (original), 32 (patched)
<https://reviews.apache.org/r/67759/#comment288369>
Please don't check in this file
- Arjun Mishra
On June 27, 2018, 4:01 p.m., Sergio Pena wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67759/
> -----------------------------------------------------------
>
> (Updated June 27, 2018, 4:01 p.m.)
>
>
> Review request for sentry, Arjun Mishra, kalyan kumar kalvagadda, and Na Li.
>
>
> Bugs: sentry-2284
> https://issues.apache.org/jira/browse/sentry-2284
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Add two API methods to the Sentry client:
> - listAllRolesPrivileges
> - listAllUsersPrivileges
>
> Both methods return a map object of the form:
> - [roleName, set<privileges>]
> - [userName, set<privileges>]
>
> Unit tests, thrift API code and Client methods are provided.
>
>
> Diffs
> -----
>
> sentry-dist/src/license/THIRD-PARTY.properties b39e1b6ca7eba8c6a7695a4238104af7cd50da32
> sentry-service/sentry-service-api/src/gen/thrift/gen-javabean/org/apache/sentry/api/service/thrift/SentryPolicyService.java d456d7ff64a8146473ff14a65f06e7cb664939b7
> sentry-service/sentry-service-api/src/gen/thrift/gen-javabean/org/apache/sentry/api/service/thrift/TSentryPrivilegesRequest.java PRE-CREATION
> sentry-service/sentry-service-api/src/gen/thrift/gen-javabean/org/apache/sentry/api/service/thrift/TSentryPrivilegesResponse.java PRE-CREATION
> sentry-service/sentry-service-api/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyServiceClient.java dc1d67ba16c9c7acf76eb3318864ba0606e2aa5a
> sentry-service/sentry-service-api/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyServiceClientDefaultImpl.java 5f76cb0aa3bd17d6aca4adc52ba59ded5ec0b900
> sentry-service/sentry-service-api/src/main/resources/sentry_policy_service.thrift 56aedcb5267495e610e3dace1f38e708d68ffe84
> sentry-service/sentry-service-api/src/test/java/org/apache/sentry/api/service/thrift/TestSentryPolicyServiceClientDefaultImpl.java 1666e326462b771674930e52e3790ca92f467778
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyStoreProcessor.java 39271500df179e0ddf8ce4f1589eaaa8137afa25
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java d8ab1fc90255c6ed42c00e3d3aa6103e47a40b29
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStoreInterface.java f61ae575c99474a76886d3c7a74765fdb067acd6
> sentry-service/sentry-service-server/src/test/java/org/apache/sentry/api/service/thrift/TestSentryPolicyStoreProcessor.java de4e0012d521cf402f4773a04b673f8056a5337c
> sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java 52ce72cebc8b69f08988b05ba54e2bbedd201c1a
>
>
> Diff: https://reviews.apache.org/r/67759/diff/1/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Sergio Pena
>
>