You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sentry.apache.org by Hao Hao <ha...@cloudera.com> on 2016/02/05 01:57:15 UTC

Review Request 43234: SENTRY-993: list_sentry_privileges_by_authorizable() gone in API v2.

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43234/
-----------------------------------------------------------

Review request for sentry.


Repository: sentry


Description
-------

Change-Id: I679341bfd24a3653060a024799c09c9ab907bc49

SENTRY-993: list_sentry_privileges_by_authorizable() gone in API v2.

Change-Id: If070e96f9c6a66f84a25f40a2bbbdcbc2de28e36


Added list_sentry_privileges_by_authorizable() for generic model thrift API. The interface will return a <Authorizables, <Role, Set<Privileges>>> mapping.


Diffs
-----

  sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericPolicyService.java b42159852e1a3cc14f34b106c9c60f8436d6fdd7 
  sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/generic/service/thrift/TListSentryPrivilegesByAuthRequest.java PRE-CREATION 
  sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/generic/service/thrift/TListSentryPrivilegesByAuthResponse.java PRE-CREATION 
  sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/generic/service/thrift/TSentryPrivilegeMap.java PRE-CREATION 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/DelegateSentryStore.java e1c15fa5304b553f69ef4d7e5053d587efb92ae5 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/PrivilegeOperatePersistence.java c3b0be8694c746cb09797425f98578b8faef8b4a 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/SentryStoreLayer.java f6d73e728f04cbb2a54595dc4d7d2b78fcd02838 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericPolicyProcessor.java 78d38473f938b1aa2b357dd858b00f833147ee5d 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClientDefaultImpl.java ce5751389670e5f2de07c1664346d1c5c26a3445 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java 521d945222c240552ecde7a66b35bc5baef84a41 
  sentry-provider/sentry-provider-db/src/main/resources/sentry_generic_policy_service.thrift 91ff672ec943003460e38892fd536444596b5795 
  sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/service/persistent/TestPrivilegeOperatePersistence.java 189eabb2724e4187a39b1d1341fd231b1ac928b7 
  sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/service/thrift/TestSentryGenericPolicyProcessor.java b86c6b2c1350924683c3d3ed293502d211bcde2d 

Diff: https://reviews.apache.org/r/43234/diff/


Testing
-------

Tested in TestSentryGenericPolicyProcessor and TestPrivilegeOperatePersistence.


Thanks,

Hao Hao


Re: Review Request 43234: SENTRY-993: list_sentry_privileges_by_authorizable() gone in API v2.

Posted by Colin Ma <ju...@intel.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43234/#review118600
-----------------------------------------------------------


Ship it!




Ship It!

- Colin Ma


On Feb. 10, 2016, 4:10 a.m., Hao Hao wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43234/
> -----------------------------------------------------------
> 
> (Updated Feb. 10, 2016, 4:10 a.m.)
> 
> 
> Review request for sentry.
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> Change-Id: I679341bfd24a3653060a024799c09c9ab907bc49
> 
> SENTRY-993: list_sentry_privileges_by_authorizable() gone in API v2.
> 
> Change-Id: If070e96f9c6a66f84a25f40a2bbbdcbc2de28e36
> 
> 
> Added list_sentry_privileges_by_authorizable() for generic model thrift API. The interface will return a <Authorizables, <Role, Set<Privileges>>> mapping.
> 
> 
> Diffs
> -----
> 
>   sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericPolicyService.java b42159852e1a3cc14f34b106c9c60f8436d6fdd7 
>   sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/generic/service/thrift/TListSentryPrivilegesByAuthRequest.java PRE-CREATION 
>   sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/generic/service/thrift/TListSentryPrivilegesByAuthResponse.java PRE-CREATION 
>   sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/generic/service/thrift/TSentryPrivilegeMap.java PRE-CREATION 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/DelegateSentryStore.java e1c15fa5304b553f69ef4d7e5053d587efb92ae5 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/PrivilegeOperatePersistence.java c3b0be8694c746cb09797425f98578b8faef8b4a 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/SentryStoreLayer.java f6d73e728f04cbb2a54595dc4d7d2b78fcd02838 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericPolicyProcessor.java 78d38473f938b1aa2b357dd858b00f833147ee5d 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClientDefaultImpl.java ce5751389670e5f2de07c1664346d1c5c26a3445 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java 521d945222c240552ecde7a66b35bc5baef84a41 
>   sentry-provider/sentry-provider-db/src/main/resources/sentry_generic_policy_service.thrift 91ff672ec943003460e38892fd536444596b5795 
>   sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/service/persistent/TestPrivilegeOperatePersistence.java 189eabb2724e4187a39b1d1341fd231b1ac928b7 
>   sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/service/thrift/TestSentryGenericPolicyProcessor.java b86c6b2c1350924683c3d3ed293502d211bcde2d 
> 
> Diff: https://reviews.apache.org/r/43234/diff/
> 
> 
> Testing
> -------
> 
> Tested in TestSentryGenericPolicyProcessor and TestPrivilegeOperatePersistence.
> 
> 
> Thanks,
> 
> Hao Hao
> 
>


Re: Review Request 43234: SENTRY-993: list_sentry_privileges_by_authorizable() gone in API v2.

Posted by Hao Hao <ha...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43234/
-----------------------------------------------------------

(Updated Feb. 10, 2016, 4:10 a.m.)


Review request for sentry.


Repository: sentry


Description
-------

Change-Id: I679341bfd24a3653060a024799c09c9ab907bc49

SENTRY-993: list_sentry_privileges_by_authorizable() gone in API v2.

Change-Id: If070e96f9c6a66f84a25f40a2bbbdcbc2de28e36


Added list_sentry_privileges_by_authorizable() for generic model thrift API. The interface will return a <Authorizables, <Role, Set<Privileges>>> mapping.


Diffs (updated)
-----

  sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericPolicyService.java b42159852e1a3cc14f34b106c9c60f8436d6fdd7 
  sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/generic/service/thrift/TListSentryPrivilegesByAuthRequest.java PRE-CREATION 
  sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/generic/service/thrift/TListSentryPrivilegesByAuthResponse.java PRE-CREATION 
  sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/generic/service/thrift/TSentryPrivilegeMap.java PRE-CREATION 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/DelegateSentryStore.java e1c15fa5304b553f69ef4d7e5053d587efb92ae5 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/PrivilegeOperatePersistence.java c3b0be8694c746cb09797425f98578b8faef8b4a 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/SentryStoreLayer.java f6d73e728f04cbb2a54595dc4d7d2b78fcd02838 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericPolicyProcessor.java 78d38473f938b1aa2b357dd858b00f833147ee5d 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClientDefaultImpl.java ce5751389670e5f2de07c1664346d1c5c26a3445 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java 521d945222c240552ecde7a66b35bc5baef84a41 
  sentry-provider/sentry-provider-db/src/main/resources/sentry_generic_policy_service.thrift 91ff672ec943003460e38892fd536444596b5795 
  sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/service/persistent/TestPrivilegeOperatePersistence.java 189eabb2724e4187a39b1d1341fd231b1ac928b7 
  sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/service/thrift/TestSentryGenericPolicyProcessor.java b86c6b2c1350924683c3d3ed293502d211bcde2d 

Diff: https://reviews.apache.org/r/43234/diff/


Testing
-------

Tested in TestSentryGenericPolicyProcessor and TestPrivilegeOperatePersistence.


Thanks,

Hao Hao


Re: Review Request 43234: SENTRY-993: list_sentry_privileges_by_authorizable() gone in API v2.

Posted by Hao Hao <ha...@cloudera.com>.

> On Feb. 9, 2016, 9:40 a.m., Lenni Kuff wrote:
> > sentry-provider/sentry-provider-db/src/main/resources/sentry_generic_policy_service.thrift, line 225
> > <https://reviews.apache.org/r/43234/diff/2/?file=1237849#file1237849line225>
> >
> >     comment.
> >     
> >     Also, why do we need to pass groups in the req, shouldn't the service resolve this? Do we need to use a set, or can a list work?

The groups in the req is for admin to request info for a specific groups. This aligns with the api implementation in v1. Added comments. Using a set can avoid duplicate here.


- Hao


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43234/#review118377
-----------------------------------------------------------


On Feb. 8, 2016, 11:23 p.m., Hao Hao wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43234/
> -----------------------------------------------------------
> 
> (Updated Feb. 8, 2016, 11:23 p.m.)
> 
> 
> Review request for sentry.
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> Change-Id: I679341bfd24a3653060a024799c09c9ab907bc49
> 
> SENTRY-993: list_sentry_privileges_by_authorizable() gone in API v2.
> 
> Change-Id: If070e96f9c6a66f84a25f40a2bbbdcbc2de28e36
> 
> 
> Added list_sentry_privileges_by_authorizable() for generic model thrift API. The interface will return a <Authorizables, <Role, Set<Privileges>>> mapping.
> 
> 
> Diffs
> -----
> 
>   sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericPolicyService.java b42159852e1a3cc14f34b106c9c60f8436d6fdd7 
>   sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/generic/service/thrift/TListSentryPrivilegesByAuthRequest.java PRE-CREATION 
>   sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/generic/service/thrift/TListSentryPrivilegesByAuthResponse.java PRE-CREATION 
>   sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/generic/service/thrift/TSentryPrivilegeMap.java PRE-CREATION 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/DelegateSentryStore.java e1c15fa5304b553f69ef4d7e5053d587efb92ae5 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/PrivilegeOperatePersistence.java c3b0be8694c746cb09797425f98578b8faef8b4a 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/SentryStoreLayer.java f6d73e728f04cbb2a54595dc4d7d2b78fcd02838 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericPolicyProcessor.java 78d38473f938b1aa2b357dd858b00f833147ee5d 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClientDefaultImpl.java ce5751389670e5f2de07c1664346d1c5c26a3445 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java 521d945222c240552ecde7a66b35bc5baef84a41 
>   sentry-provider/sentry-provider-db/src/main/resources/sentry_generic_policy_service.thrift 91ff672ec943003460e38892fd536444596b5795 
>   sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/service/persistent/TestPrivilegeOperatePersistence.java 189eabb2724e4187a39b1d1341fd231b1ac928b7 
>   sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/service/thrift/TestSentryGenericPolicyProcessor.java b86c6b2c1350924683c3d3ed293502d211bcde2d 
> 
> Diff: https://reviews.apache.org/r/43234/diff/
> 
> 
> Testing
> -------
> 
> Tested in TestSentryGenericPolicyProcessor and TestPrivilegeOperatePersistence.
> 
> 
> Thanks,
> 
> Hao Hao
> 
>


Re: Review Request 43234: SENTRY-993: list_sentry_privileges_by_authorizable() gone in API v2.

Posted by Lenni Kuff <ls...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43234/#review118377
-----------------------------------------------------------




sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericPolicyProcessor.java (line 639)
<https://reviews.apache.org/r/43234/#comment179618>

    add string constant for "Access denied to: " so we ensure error messages are unified.



sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java (line 2072)
<https://reviews.apache.org/r/43234/#comment179617>

    comment on rval when no roles exist (is it null or empty list)



sentry-provider/sentry-provider-db/src/main/resources/sentry_generic_policy_service.thrift (line 219)
<https://reviews.apache.org/r/43234/#comment179615>

    Add brief comment on how req/resp is used



sentry-provider/sentry-provider-db/src/main/resources/sentry_generic_policy_service.thrift (line 225)
<https://reviews.apache.org/r/43234/#comment179616>

    comment.
    
    Also, why do we need to pass groups in the req, shouldn't the service resolve this? Do we need to use a set, or can a list work?


+1 from me, would be good to get +1 from colin as well since he had some good feedback.

- Lenni Kuff


On Feb. 8, 2016, 11:23 p.m., Hao Hao wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43234/
> -----------------------------------------------------------
> 
> (Updated Feb. 8, 2016, 11:23 p.m.)
> 
> 
> Review request for sentry.
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> Change-Id: I679341bfd24a3653060a024799c09c9ab907bc49
> 
> SENTRY-993: list_sentry_privileges_by_authorizable() gone in API v2.
> 
> Change-Id: If070e96f9c6a66f84a25f40a2bbbdcbc2de28e36
> 
> 
> Added list_sentry_privileges_by_authorizable() for generic model thrift API. The interface will return a <Authorizables, <Role, Set<Privileges>>> mapping.
> 
> 
> Diffs
> -----
> 
>   sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericPolicyService.java b42159852e1a3cc14f34b106c9c60f8436d6fdd7 
>   sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/generic/service/thrift/TListSentryPrivilegesByAuthRequest.java PRE-CREATION 
>   sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/generic/service/thrift/TListSentryPrivilegesByAuthResponse.java PRE-CREATION 
>   sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/generic/service/thrift/TSentryPrivilegeMap.java PRE-CREATION 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/DelegateSentryStore.java e1c15fa5304b553f69ef4d7e5053d587efb92ae5 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/PrivilegeOperatePersistence.java c3b0be8694c746cb09797425f98578b8faef8b4a 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/SentryStoreLayer.java f6d73e728f04cbb2a54595dc4d7d2b78fcd02838 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericPolicyProcessor.java 78d38473f938b1aa2b357dd858b00f833147ee5d 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClientDefaultImpl.java ce5751389670e5f2de07c1664346d1c5c26a3445 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java 521d945222c240552ecde7a66b35bc5baef84a41 
>   sentry-provider/sentry-provider-db/src/main/resources/sentry_generic_policy_service.thrift 91ff672ec943003460e38892fd536444596b5795 
>   sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/service/persistent/TestPrivilegeOperatePersistence.java 189eabb2724e4187a39b1d1341fd231b1ac928b7 
>   sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/service/thrift/TestSentryGenericPolicyProcessor.java b86c6b2c1350924683c3d3ed293502d211bcde2d 
> 
> Diff: https://reviews.apache.org/r/43234/diff/
> 
> 
> Testing
> -------
> 
> Tested in TestSentryGenericPolicyProcessor and TestPrivilegeOperatePersistence.
> 
> 
> Thanks,
> 
> Hao Hao
> 
>


Re: Review Request 43234: SENTRY-993: list_sentry_privileges_by_authorizable() gone in API v2.

Posted by Hao Hao <ha...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43234/
-----------------------------------------------------------

(Updated Feb. 8, 2016, 11:23 p.m.)


Review request for sentry.


Repository: sentry


Description
-------

Change-Id: I679341bfd24a3653060a024799c09c9ab907bc49

SENTRY-993: list_sentry_privileges_by_authorizable() gone in API v2.

Change-Id: If070e96f9c6a66f84a25f40a2bbbdcbc2de28e36


Added list_sentry_privileges_by_authorizable() for generic model thrift API. The interface will return a <Authorizables, <Role, Set<Privileges>>> mapping.


Diffs (updated)
-----

  sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericPolicyService.java b42159852e1a3cc14f34b106c9c60f8436d6fdd7 
  sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/generic/service/thrift/TListSentryPrivilegesByAuthRequest.java PRE-CREATION 
  sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/generic/service/thrift/TListSentryPrivilegesByAuthResponse.java PRE-CREATION 
  sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/generic/service/thrift/TSentryPrivilegeMap.java PRE-CREATION 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/DelegateSentryStore.java e1c15fa5304b553f69ef4d7e5053d587efb92ae5 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/PrivilegeOperatePersistence.java c3b0be8694c746cb09797425f98578b8faef8b4a 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/SentryStoreLayer.java f6d73e728f04cbb2a54595dc4d7d2b78fcd02838 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericPolicyProcessor.java 78d38473f938b1aa2b357dd858b00f833147ee5d 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClientDefaultImpl.java ce5751389670e5f2de07c1664346d1c5c26a3445 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java 521d945222c240552ecde7a66b35bc5baef84a41 
  sentry-provider/sentry-provider-db/src/main/resources/sentry_generic_policy_service.thrift 91ff672ec943003460e38892fd536444596b5795 
  sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/service/persistent/TestPrivilegeOperatePersistence.java 189eabb2724e4187a39b1d1341fd231b1ac928b7 
  sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/service/thrift/TestSentryGenericPolicyProcessor.java b86c6b2c1350924683c3d3ed293502d211bcde2d 

Diff: https://reviews.apache.org/r/43234/diff/


Testing
-------

Tested in TestSentryGenericPolicyProcessor and TestPrivilegeOperatePersistence.


Thanks,

Hao Hao


Re: Review Request 43234: SENTRY-993: list_sentry_privileges_by_authorizable() gone in API v2.

Posted by Hao Hao <ha...@cloudera.com>.

> On Feb. 8, 2016, 7:46 p.m., Hao Hao wrote:
> > sentry-provider/sentry-provider-db/src/main/resources/sentry_generic_policy_service.thrift, line 231
> > <https://reviews.apache.org/r/43234/diff/1/?file=1233881#file1233881line231>
> >
> >     It is because the key is the authorizable hierarchys which are a list of authorizable in generic model. And it will be from the input of  authorizablesSet. Agree it is weird. Maybe I can create an object to store the list? BTW, I also feel using list<authorizable> to store the authorizable hierarchys may be not a good idea? since it is really depend on the order of the list.
> 
> Colin Ma wrote:
>     Is it possible to use the string with the following format as the key:
>     resourceType1=resourceName1->resourceType2=resourceName2->resourceType3=resourceName3
>     
>     The thrift api will be as the following:
>     struct TListSentryPrivilegesByAuthRequest {
>     ......
>     5: required set<string> authorizablesSet # authorizable hierarchys
>     ......
>     }
>     
>     struct TListSentryPrivilegesByAuthResponse {
>     ......
>     2: optional map<string>, TSentryPrivilegeMap> privilegesMapByAuth # will not be set in case of an error
>     }
>     
>     Is it ok for your requirement? Feel free to discuss.
> 
> Hao Hao wrote:
>     I am ok with using string format to store authorizable hierarchys, and can add comments on the api. A little concern is that is it ok list_sentry_privileges_for_provider and list_sentry_privileges_by_authorizable have different ways of storing authorizable hierarchys?

Changed it based on the comment.


- Hao


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43234/#review118279
-----------------------------------------------------------


On Feb. 10, 2016, 4:10 a.m., Hao Hao wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43234/
> -----------------------------------------------------------
> 
> (Updated Feb. 10, 2016, 4:10 a.m.)
> 
> 
> Review request for sentry.
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> Change-Id: I679341bfd24a3653060a024799c09c9ab907bc49
> 
> SENTRY-993: list_sentry_privileges_by_authorizable() gone in API v2.
> 
> Change-Id: If070e96f9c6a66f84a25f40a2bbbdcbc2de28e36
> 
> 
> Added list_sentry_privileges_by_authorizable() for generic model thrift API. The interface will return a <Authorizables, <Role, Set<Privileges>>> mapping.
> 
> 
> Diffs
> -----
> 
>   sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericPolicyService.java b42159852e1a3cc14f34b106c9c60f8436d6fdd7 
>   sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/generic/service/thrift/TListSentryPrivilegesByAuthRequest.java PRE-CREATION 
>   sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/generic/service/thrift/TListSentryPrivilegesByAuthResponse.java PRE-CREATION 
>   sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/generic/service/thrift/TSentryPrivilegeMap.java PRE-CREATION 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/DelegateSentryStore.java e1c15fa5304b553f69ef4d7e5053d587efb92ae5 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/PrivilegeOperatePersistence.java c3b0be8694c746cb09797425f98578b8faef8b4a 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/SentryStoreLayer.java f6d73e728f04cbb2a54595dc4d7d2b78fcd02838 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericPolicyProcessor.java 78d38473f938b1aa2b357dd858b00f833147ee5d 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClientDefaultImpl.java ce5751389670e5f2de07c1664346d1c5c26a3445 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java 521d945222c240552ecde7a66b35bc5baef84a41 
>   sentry-provider/sentry-provider-db/src/main/resources/sentry_generic_policy_service.thrift 91ff672ec943003460e38892fd536444596b5795 
>   sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/service/persistent/TestPrivilegeOperatePersistence.java 189eabb2724e4187a39b1d1341fd231b1ac928b7 
>   sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/service/thrift/TestSentryGenericPolicyProcessor.java b86c6b2c1350924683c3d3ed293502d211bcde2d 
> 
> Diff: https://reviews.apache.org/r/43234/diff/
> 
> 
> Testing
> -------
> 
> Tested in TestSentryGenericPolicyProcessor and TestPrivilegeOperatePersistence.
> 
> 
> Thanks,
> 
> Hao Hao
> 
>


Re: Review Request 43234: SENTRY-993: list_sentry_privileges_by_authorizable() gone in API v2.

Posted by Hao Hao <ha...@cloudera.com>.

> On Feb. 8, 2016, 7:46 p.m., Hao Hao wrote:
> > sentry-provider/sentry-provider-db/src/main/resources/sentry_generic_policy_service.thrift, line 231
> > <https://reviews.apache.org/r/43234/diff/1/?file=1233881#file1233881line231>
> >
> >     It is because the key is the authorizable hierarchys which are a list of authorizable in generic model. And it will be from the input of  authorizablesSet. Agree it is weird. Maybe I can create an object to store the list? BTW, I also feel using list<authorizable> to store the authorizable hierarchys may be not a good idea? since it is really depend on the order of the list.
> 
> Colin Ma wrote:
>     Is it possible to use the string with the following format as the key:
>     resourceType1=resourceName1->resourceType2=resourceName2->resourceType3=resourceName3
>     
>     The thrift api will be as the following:
>     struct TListSentryPrivilegesByAuthRequest {
>     ......
>     5: required set<string> authorizablesSet # authorizable hierarchys
>     ......
>     }
>     
>     struct TListSentryPrivilegesByAuthResponse {
>     ......
>     2: optional map<string>, TSentryPrivilegeMap> privilegesMapByAuth # will not be set in case of an error
>     }
>     
>     Is it ok for your requirement? Feel free to discuss.

I am ok with using string format to store authorizable hierarchys, and can add comments on the api. A little concern is that is it ok list_sentry_privileges_for_provider and list_sentry_privileges_by_authorizable have different ways of storing authorizable hierarchys?


- Hao


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43234/#review118279
-----------------------------------------------------------


On Feb. 8, 2016, 11:23 p.m., Hao Hao wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43234/
> -----------------------------------------------------------
> 
> (Updated Feb. 8, 2016, 11:23 p.m.)
> 
> 
> Review request for sentry.
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> Change-Id: I679341bfd24a3653060a024799c09c9ab907bc49
> 
> SENTRY-993: list_sentry_privileges_by_authorizable() gone in API v2.
> 
> Change-Id: If070e96f9c6a66f84a25f40a2bbbdcbc2de28e36
> 
> 
> Added list_sentry_privileges_by_authorizable() for generic model thrift API. The interface will return a <Authorizables, <Role, Set<Privileges>>> mapping.
> 
> 
> Diffs
> -----
> 
>   sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericPolicyService.java b42159852e1a3cc14f34b106c9c60f8436d6fdd7 
>   sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/generic/service/thrift/TListSentryPrivilegesByAuthRequest.java PRE-CREATION 
>   sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/generic/service/thrift/TListSentryPrivilegesByAuthResponse.java PRE-CREATION 
>   sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/generic/service/thrift/TSentryPrivilegeMap.java PRE-CREATION 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/DelegateSentryStore.java e1c15fa5304b553f69ef4d7e5053d587efb92ae5 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/PrivilegeOperatePersistence.java c3b0be8694c746cb09797425f98578b8faef8b4a 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/SentryStoreLayer.java f6d73e728f04cbb2a54595dc4d7d2b78fcd02838 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericPolicyProcessor.java 78d38473f938b1aa2b357dd858b00f833147ee5d 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClientDefaultImpl.java ce5751389670e5f2de07c1664346d1c5c26a3445 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java 521d945222c240552ecde7a66b35bc5baef84a41 
>   sentry-provider/sentry-provider-db/src/main/resources/sentry_generic_policy_service.thrift 91ff672ec943003460e38892fd536444596b5795 
>   sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/service/persistent/TestPrivilegeOperatePersistence.java 189eabb2724e4187a39b1d1341fd231b1ac928b7 
>   sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/service/thrift/TestSentryGenericPolicyProcessor.java b86c6b2c1350924683c3d3ed293502d211bcde2d 
> 
> Diff: https://reviews.apache.org/r/43234/diff/
> 
> 
> Testing
> -------
> 
> Tested in TestSentryGenericPolicyProcessor and TestPrivilegeOperatePersistence.
> 
> 
> Thanks,
> 
> Hao Hao
> 
>


Re: Review Request 43234: SENTRY-993: list_sentry_privileges_by_authorizable() gone in API v2.

Posted by Colin Ma <ju...@intel.com>.

> On Feb. 8, 2016, 7:46 p.m., Hao Hao wrote:
> > sentry-provider/sentry-provider-db/src/main/resources/sentry_generic_policy_service.thrift, line 231
> > <https://reviews.apache.org/r/43234/diff/1/?file=1233881#file1233881line231>
> >
> >     It is because the key is the authorizable hierarchys which are a list of authorizable in generic model. And it will be from the input of  authorizablesSet. Agree it is weird. Maybe I can create an object to store the list? BTW, I also feel using list<authorizable> to store the authorizable hierarchys may be not a good idea? since it is really depend on the order of the list.

Is it possible to use the string with the following format as the key:
resourceType1=resourceName1->resourceType2=resourceName2->resourceType3=resourceName3

The thrift api will be as the following:
struct TListSentryPrivilegesByAuthRequest {
......
5: required set<string> authorizablesSet # authorizable hierarchys
......
}

struct TListSentryPrivilegesByAuthResponse {
......
2: optional map<string>, TSentryPrivilegeMap> privilegesMapByAuth # will not be set in case of an error
}

Is it ok for your requirement? Feel free to discuss.


- Colin


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43234/#review118279
-----------------------------------------------------------


On Feb. 8, 2016, 11:23 p.m., Hao Hao wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43234/
> -----------------------------------------------------------
> 
> (Updated Feb. 8, 2016, 11:23 p.m.)
> 
> 
> Review request for sentry.
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> Change-Id: I679341bfd24a3653060a024799c09c9ab907bc49
> 
> SENTRY-993: list_sentry_privileges_by_authorizable() gone in API v2.
> 
> Change-Id: If070e96f9c6a66f84a25f40a2bbbdcbc2de28e36
> 
> 
> Added list_sentry_privileges_by_authorizable() for generic model thrift API. The interface will return a <Authorizables, <Role, Set<Privileges>>> mapping.
> 
> 
> Diffs
> -----
> 
>   sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericPolicyService.java b42159852e1a3cc14f34b106c9c60f8436d6fdd7 
>   sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/generic/service/thrift/TListSentryPrivilegesByAuthRequest.java PRE-CREATION 
>   sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/generic/service/thrift/TListSentryPrivilegesByAuthResponse.java PRE-CREATION 
>   sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/generic/service/thrift/TSentryPrivilegeMap.java PRE-CREATION 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/DelegateSentryStore.java e1c15fa5304b553f69ef4d7e5053d587efb92ae5 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/PrivilegeOperatePersistence.java c3b0be8694c746cb09797425f98578b8faef8b4a 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/SentryStoreLayer.java f6d73e728f04cbb2a54595dc4d7d2b78fcd02838 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericPolicyProcessor.java 78d38473f938b1aa2b357dd858b00f833147ee5d 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClientDefaultImpl.java ce5751389670e5f2de07c1664346d1c5c26a3445 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java 521d945222c240552ecde7a66b35bc5baef84a41 
>   sentry-provider/sentry-provider-db/src/main/resources/sentry_generic_policy_service.thrift 91ff672ec943003460e38892fd536444596b5795 
>   sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/service/persistent/TestPrivilegeOperatePersistence.java 189eabb2724e4187a39b1d1341fd231b1ac928b7 
>   sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/service/thrift/TestSentryGenericPolicyProcessor.java b86c6b2c1350924683c3d3ed293502d211bcde2d 
> 
> Diff: https://reviews.apache.org/r/43234/diff/
> 
> 
> Testing
> -------
> 
> Tested in TestSentryGenericPolicyProcessor and TestPrivilegeOperatePersistence.
> 
> 
> Thanks,
> 
> Hao Hao
> 
>


Re: Review Request 43234: SENTRY-993: list_sentry_privileges_by_authorizable() gone in API v2.

Posted by Hao Hao <ha...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43234/#review118279
-----------------------------------------------------------




sentry-provider/sentry-provider-db/src/main/resources/sentry_generic_policy_service.thrift (line 231)
<https://reviews.apache.org/r/43234/#comment179504>

    It is because the key is the authorizable hierarchys which are a list of authorizable in generic model. And it will be from the input of  authorizablesSet. Agree it is weird. Maybe I can create an object to store the list? BTW, I also feel using list<authorizable> to store the authorizable hierarchys may be not a good idea? since it is really depend on the order of the list.



sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/service/persistent/TestPrivilegeOperatePersistence.java (line 957)
<https://reviews.apache.org/r/43234/#comment179505>

    Will do.


- Hao Hao


On Feb. 5, 2016, 12:57 a.m., Hao Hao wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43234/
> -----------------------------------------------------------
> 
> (Updated Feb. 5, 2016, 12:57 a.m.)
> 
> 
> Review request for sentry.
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> Change-Id: I679341bfd24a3653060a024799c09c9ab907bc49
> 
> SENTRY-993: list_sentry_privileges_by_authorizable() gone in API v2.
> 
> Change-Id: If070e96f9c6a66f84a25f40a2bbbdcbc2de28e36
> 
> 
> Added list_sentry_privileges_by_authorizable() for generic model thrift API. The interface will return a <Authorizables, <Role, Set<Privileges>>> mapping.
> 
> 
> Diffs
> -----
> 
>   sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericPolicyService.java b42159852e1a3cc14f34b106c9c60f8436d6fdd7 
>   sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/generic/service/thrift/TListSentryPrivilegesByAuthRequest.java PRE-CREATION 
>   sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/generic/service/thrift/TListSentryPrivilegesByAuthResponse.java PRE-CREATION 
>   sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/generic/service/thrift/TSentryPrivilegeMap.java PRE-CREATION 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/DelegateSentryStore.java e1c15fa5304b553f69ef4d7e5053d587efb92ae5 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/PrivilegeOperatePersistence.java c3b0be8694c746cb09797425f98578b8faef8b4a 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/SentryStoreLayer.java f6d73e728f04cbb2a54595dc4d7d2b78fcd02838 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericPolicyProcessor.java 78d38473f938b1aa2b357dd858b00f833147ee5d 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClientDefaultImpl.java ce5751389670e5f2de07c1664346d1c5c26a3445 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java 521d945222c240552ecde7a66b35bc5baef84a41 
>   sentry-provider/sentry-provider-db/src/main/resources/sentry_generic_policy_service.thrift 91ff672ec943003460e38892fd536444596b5795 
>   sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/service/persistent/TestPrivilegeOperatePersistence.java 189eabb2724e4187a39b1d1341fd231b1ac928b7 
>   sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/service/thrift/TestSentryGenericPolicyProcessor.java b86c6b2c1350924683c3d3ed293502d211bcde2d 
> 
> Diff: https://reviews.apache.org/r/43234/diff/
> 
> 
> Testing
> -------
> 
> Tested in TestSentryGenericPolicyProcessor and TestPrivilegeOperatePersistence.
> 
> 
> Thanks,
> 
> Hao Hao
> 
>


Re: Review Request 43234: SENTRY-993: list_sentry_privileges_by_authorizable() gone in API v2.

Posted by Hao Hao <ha...@cloudera.com>.

> On Feb. 6, 2016, 6:22 a.m., Colin Ma wrote:
> > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/DelegateSentryStore.java, line 426
> > <https://reviews.apache.org/r/43234/diff/1/?file=1233875#file1233875line426>
> >
> >     Can we reuse the getPrivilegesByProvider()? 
> >     If pass the null as groups parameter, they have the same logic.

The problem is the return value are not the same. We need the role to MSentryGMPrivilege mapping, which has been swallowed in getPrivilegesByProvider.


> On Feb. 6, 2016, 6:22 a.m., Colin Ma wrote:
> > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java, line 2081
> > <https://reviews.apache.org/r/43234/diff/1/?file=1233880#file1233880line2081>
> >
> >     There is a method already there, private Set<String> getAllRoleNames(PersistenceManager pm).
> >     Wrap this as:
> >     
> >     .....
> >     existRoleNames = getAllRoleNames(pm);
> >     .....

Will do.


- Hao


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43234/#review118137
-----------------------------------------------------------


On Feb. 5, 2016, 12:57 a.m., Hao Hao wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43234/
> -----------------------------------------------------------
> 
> (Updated Feb. 5, 2016, 12:57 a.m.)
> 
> 
> Review request for sentry.
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> Change-Id: I679341bfd24a3653060a024799c09c9ab907bc49
> 
> SENTRY-993: list_sentry_privileges_by_authorizable() gone in API v2.
> 
> Change-Id: If070e96f9c6a66f84a25f40a2bbbdcbc2de28e36
> 
> 
> Added list_sentry_privileges_by_authorizable() for generic model thrift API. The interface will return a <Authorizables, <Role, Set<Privileges>>> mapping.
> 
> 
> Diffs
> -----
> 
>   sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericPolicyService.java b42159852e1a3cc14f34b106c9c60f8436d6fdd7 
>   sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/generic/service/thrift/TListSentryPrivilegesByAuthRequest.java PRE-CREATION 
>   sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/generic/service/thrift/TListSentryPrivilegesByAuthResponse.java PRE-CREATION 
>   sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/generic/service/thrift/TSentryPrivilegeMap.java PRE-CREATION 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/DelegateSentryStore.java e1c15fa5304b553f69ef4d7e5053d587efb92ae5 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/PrivilegeOperatePersistence.java c3b0be8694c746cb09797425f98578b8faef8b4a 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/SentryStoreLayer.java f6d73e728f04cbb2a54595dc4d7d2b78fcd02838 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericPolicyProcessor.java 78d38473f938b1aa2b357dd858b00f833147ee5d 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClientDefaultImpl.java ce5751389670e5f2de07c1664346d1c5c26a3445 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java 521d945222c240552ecde7a66b35bc5baef84a41 
>   sentry-provider/sentry-provider-db/src/main/resources/sentry_generic_policy_service.thrift 91ff672ec943003460e38892fd536444596b5795 
>   sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/service/persistent/TestPrivilegeOperatePersistence.java 189eabb2724e4187a39b1d1341fd231b1ac928b7 
>   sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/service/thrift/TestSentryGenericPolicyProcessor.java b86c6b2c1350924683c3d3ed293502d211bcde2d 
> 
> Diff: https://reviews.apache.org/r/43234/diff/
> 
> 
> Testing
> -------
> 
> Tested in TestSentryGenericPolicyProcessor and TestPrivilegeOperatePersistence.
> 
> 
> Thanks,
> 
> Hao Hao
> 
>


Re: Review Request 43234: SENTRY-993: list_sentry_privileges_by_authorizable() gone in API v2.

Posted by Colin Ma <ju...@intel.com>.

> On Feb. 6, 2016, 6:22 a.m., Colin Ma wrote:
> > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/DelegateSentryStore.java, line 426
> > <https://reviews.apache.org/r/43234/diff/1/?file=1233875#file1233875line426>
> >
> >     Can we reuse the getPrivilegesByProvider()? 
> >     If pass the null as groups parameter, they have the same logic.
> 
> Hao Hao wrote:
>     The problem is the return value are not the same. We need the role to MSentryGMPrivilege mapping, which has been swallowed in getPrivilegesByProvider.

got your point.


- Colin


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43234/#review118137
-----------------------------------------------------------


On Feb. 8, 2016, 11:23 p.m., Hao Hao wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43234/
> -----------------------------------------------------------
> 
> (Updated Feb. 8, 2016, 11:23 p.m.)
> 
> 
> Review request for sentry.
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> Change-Id: I679341bfd24a3653060a024799c09c9ab907bc49
> 
> SENTRY-993: list_sentry_privileges_by_authorizable() gone in API v2.
> 
> Change-Id: If070e96f9c6a66f84a25f40a2bbbdcbc2de28e36
> 
> 
> Added list_sentry_privileges_by_authorizable() for generic model thrift API. The interface will return a <Authorizables, <Role, Set<Privileges>>> mapping.
> 
> 
> Diffs
> -----
> 
>   sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericPolicyService.java b42159852e1a3cc14f34b106c9c60f8436d6fdd7 
>   sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/generic/service/thrift/TListSentryPrivilegesByAuthRequest.java PRE-CREATION 
>   sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/generic/service/thrift/TListSentryPrivilegesByAuthResponse.java PRE-CREATION 
>   sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/generic/service/thrift/TSentryPrivilegeMap.java PRE-CREATION 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/DelegateSentryStore.java e1c15fa5304b553f69ef4d7e5053d587efb92ae5 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/PrivilegeOperatePersistence.java c3b0be8694c746cb09797425f98578b8faef8b4a 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/SentryStoreLayer.java f6d73e728f04cbb2a54595dc4d7d2b78fcd02838 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericPolicyProcessor.java 78d38473f938b1aa2b357dd858b00f833147ee5d 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClientDefaultImpl.java ce5751389670e5f2de07c1664346d1c5c26a3445 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java 521d945222c240552ecde7a66b35bc5baef84a41 
>   sentry-provider/sentry-provider-db/src/main/resources/sentry_generic_policy_service.thrift 91ff672ec943003460e38892fd536444596b5795 
>   sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/service/persistent/TestPrivilegeOperatePersistence.java 189eabb2724e4187a39b1d1341fd231b1ac928b7 
>   sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/service/thrift/TestSentryGenericPolicyProcessor.java b86c6b2c1350924683c3d3ed293502d211bcde2d 
> 
> Diff: https://reviews.apache.org/r/43234/diff/
> 
> 
> Testing
> -------
> 
> Tested in TestSentryGenericPolicyProcessor and TestPrivilegeOperatePersistence.
> 
> 
> Thanks,
> 
> Hao Hao
> 
>


Re: Review Request 43234: SENTRY-993: list_sentry_privileges_by_authorizable() gone in API v2.

Posted by Colin Ma <ju...@intel.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43234/#review118137
-----------------------------------------------------------




sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericPolicyService.java (line 7885)
<https://reviews.apache.org/r/43234/#comment179389>

    The white space should be removed even it is generated by thrift tool.



sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/DelegateSentryStore.java (line 21)
<https://reviews.apache.org/r/43234/#comment179382>

    It's better not use asterisk in import.



sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/DelegateSentryStore.java (line 39)
<https://reviews.apache.org/r/43234/#comment179383>

    It's better not use asterisk in import.



sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/DelegateSentryStore.java (line 419)
<https://reviews.apache.org/r/43234/#comment179384>

    Can we reuse the getPrivilegesByProvider()? 
    If pass the null as groups parameter, they have the same logic.



sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java (line 2081)
<https://reviews.apache.org/r/43234/#comment179386>

    There is a method already there, private Set<String> getAllRoleNames(PersistenceManager pm).
    Wrap this as:
    
    .....
    existRoleNames = getAllRoleNames(pm);
    .....



sentry-provider/sentry-provider-db/src/main/resources/sentry_generic_policy_service.thrift (line 231)
<https://reviews.apache.org/r/43234/#comment179387>

    The list as a key of Map, it's so wired, can you give an example for how to use the result map privilegesMapByAuth?



sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/service/persistent/TestPrivilegeOperatePersistence.java (line 957)
<https://reviews.apache.org/r/43234/#comment179388>

    The following condition should be tested:
    activeRole == empty 
    authorizablesSet != null


- Colin Ma


On Feb. 5, 2016, 12:57 a.m., Hao Hao wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43234/
> -----------------------------------------------------------
> 
> (Updated Feb. 5, 2016, 12:57 a.m.)
> 
> 
> Review request for sentry.
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> Change-Id: I679341bfd24a3653060a024799c09c9ab907bc49
> 
> SENTRY-993: list_sentry_privileges_by_authorizable() gone in API v2.
> 
> Change-Id: If070e96f9c6a66f84a25f40a2bbbdcbc2de28e36
> 
> 
> Added list_sentry_privileges_by_authorizable() for generic model thrift API. The interface will return a <Authorizables, <Role, Set<Privileges>>> mapping.
> 
> 
> Diffs
> -----
> 
>   sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericPolicyService.java b42159852e1a3cc14f34b106c9c60f8436d6fdd7 
>   sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/generic/service/thrift/TListSentryPrivilegesByAuthRequest.java PRE-CREATION 
>   sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/generic/service/thrift/TListSentryPrivilegesByAuthResponse.java PRE-CREATION 
>   sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/generic/service/thrift/TSentryPrivilegeMap.java PRE-CREATION 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/DelegateSentryStore.java e1c15fa5304b553f69ef4d7e5053d587efb92ae5 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/PrivilegeOperatePersistence.java c3b0be8694c746cb09797425f98578b8faef8b4a 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/SentryStoreLayer.java f6d73e728f04cbb2a54595dc4d7d2b78fcd02838 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericPolicyProcessor.java 78d38473f938b1aa2b357dd858b00f833147ee5d 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClientDefaultImpl.java ce5751389670e5f2de07c1664346d1c5c26a3445 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java 521d945222c240552ecde7a66b35bc5baef84a41 
>   sentry-provider/sentry-provider-db/src/main/resources/sentry_generic_policy_service.thrift 91ff672ec943003460e38892fd536444596b5795 
>   sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/service/persistent/TestPrivilegeOperatePersistence.java 189eabb2724e4187a39b1d1341fd231b1ac928b7 
>   sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/service/thrift/TestSentryGenericPolicyProcessor.java b86c6b2c1350924683c3d3ed293502d211bcde2d 
> 
> Diff: https://reviews.apache.org/r/43234/diff/
> 
> 
> Testing
> -------
> 
> Tested in TestSentryGenericPolicyProcessor and TestPrivilegeOperatePersistence.
> 
> 
> Thanks,
> 
> Hao Hao
> 
>