You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sentry.apache.org by Colin Ma <ju...@intel.com> on 2015/07/22 03:54:06 UTC
Re: Review Request 35485: SENTRY-769: [Improve error handling] Make
sure groups in list_sentry_privileges_for_provider is not empty
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35485/
-----------------------------------------------------------
(Updated July 22, 2015, 1:54 a.m.)
Review request for sentry and Sravya Tirukkovalur.
Repository: sentry
Description
-------
Make sure groups in list_sentry_privileges_for_provider is not empty
Diffs (updated)
-----
sentry-binding/sentry-binding-hive/src/test/java/org/apache/sentry/binding/hive/TestHiveAuthzBindings.java 0622b43
sentry-binding/sentry-binding-solr/src/test/java/org/apache/sentry/binding/solr/TestSolrAuthzBinding.java c37f8ff
sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/AuthorizationProvider.java a88d2f8
sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/ProviderBackend.java ddb9cf9
sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/ResourceAuthorizationProvider.java 06573b7
sentry-solr/solr-sentry-handlers/src/main/resources/sentry-handlers/sentry/test-authz-provider.ini 8f48a8c
sentry-solr/solr-sentry-handlers/src/test/java/org/apache/solr/sentry/SentryIndexAuthorizationSingletonTest.java a3d7d19
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestUserManagement.java fa34c33
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/metastore/TestAuthorizingObjectStore.java 30041c5
sentry-tests/sentry-tests-solr/src/test/resources/solr/sentry/test-authz-provider.ini 34a030d
Diff: https://reviews.apache.org/r/35485/diff/
Testing
-------
Thanks,
Colin Ma
Re: Review Request 35485: SENTRY-769: [Improve error handling] Make
sure groups in list_sentry_privileges_for_provider is not empty
Posted by Colin Ma <ju...@intel.com>.
> On Oct. 8, 2015, 10:09 p.m., Sravya Tirukkovalur wrote:
> > sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/ResourceAuthorizationProvider.java, line 100
> > <https://reviews.apache.org/r/35485/diff/3/?file=1018348#file1018348line100>
> >
> > SentryConfigurationException is for issues in the configuration of the service. I think we should just throw SentryAccessDeniedException with a proper message instead. What do you think?
You're right, but the SentryAccessDeniedException is in sentry-provider-db package, and it isn't a runtime exception. It's unconvenience to reuse this in ResourceAuthorizationProvider.
Created a new exception SentryGroupNotFoundException in sentry-provider-common instead.
- Colin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35485/#review101968
-----------------------------------------------------------
On Oct. 9, 2015, 7:23 a.m., Colin Ma wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35485/
> -----------------------------------------------------------
>
> (Updated Oct. 9, 2015, 7:23 a.m.)
>
>
> Review request for sentry and Sravya Tirukkovalur.
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Make sure groups in list_sentry_privileges_for_provider is not empty
>
>
> Diffs
> -----
>
> sentry-binding/sentry-binding-hive/src/test/java/org/apache/sentry/binding/hive/TestHiveAuthzBindings.java 0622b43
> sentry-binding/sentry-binding-solr/src/test/java/org/apache/sentry/binding/solr/TestSolrAuthzBinding.java c37f8ff
> sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/HadoopGroupMappingService.java fb335a3
> sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/SentryGroupNotFoundException.java PRE-CREATION
> sentry-provider/sentry-provider-file/src/main/java/org/apache/sentry/provider/file/LocalGroupMappingService.java e22e6b6
> sentry-provider/sentry-provider-file/src/test/java/org/apache/sentry/provider/file/TestLocalGroupMapping.java c436009
> sentry-solr/solr-sentry-handlers/src/main/resources/sentry-handlers/sentry/test-authz-provider.ini 8f48a8c
> sentry-solr/solr-sentry-handlers/src/test/java/org/apache/solr/sentry/SentryIndexAuthorizationSingletonTest.java a3d7d19
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestUserManagement.java 471af1a
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/metastore/TestAuthorizingObjectStore.java 44ed096
> sentry-tests/sentry-tests-solr/src/test/resources/solr/sentry/test-authz-provider.ini 34a030d
>
> Diff: https://reviews.apache.org/r/35485/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Colin Ma
>
>
Re: Review Request 35485: SENTRY-769: [Improve error handling] Make
sure groups in list_sentry_privileges_for_provider is not empty
Posted by Sravya Tirukkovalur <sr...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35485/#review101968
-----------------------------------------------------------
sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/ResourceAuthorizationProvider.java (line 99)
<https://reviews.apache.org/r/35485/#comment159480>
SentryConfigurationException is for issues in the configuration of the service. I think we should just throw SentryAccessDeniedException with a proper message instead. What do you think?
- Sravya Tirukkovalur
On July 22, 2015, 1:54 a.m., Colin Ma wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35485/
> -----------------------------------------------------------
>
> (Updated July 22, 2015, 1:54 a.m.)
>
>
> Review request for sentry and Sravya Tirukkovalur.
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Make sure groups in list_sentry_privileges_for_provider is not empty
>
>
> Diffs
> -----
>
> sentry-binding/sentry-binding-hive/src/test/java/org/apache/sentry/binding/hive/TestHiveAuthzBindings.java 0622b43
> sentry-binding/sentry-binding-solr/src/test/java/org/apache/sentry/binding/solr/TestSolrAuthzBinding.java c37f8ff
> sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/AuthorizationProvider.java a88d2f8
> sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/ProviderBackend.java ddb9cf9
> sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/ResourceAuthorizationProvider.java 06573b7
> sentry-solr/solr-sentry-handlers/src/main/resources/sentry-handlers/sentry/test-authz-provider.ini 8f48a8c
> sentry-solr/solr-sentry-handlers/src/test/java/org/apache/solr/sentry/SentryIndexAuthorizationSingletonTest.java a3d7d19
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestUserManagement.java fa34c33
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/metastore/TestAuthorizingObjectStore.java 30041c5
> sentry-tests/sentry-tests-solr/src/test/resources/solr/sentry/test-authz-provider.ini 34a030d
>
> Diff: https://reviews.apache.org/r/35485/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Colin Ma
>
>
Re: Review Request 35485: SENTRY-769: [Improve error handling] Make
sure groups in list_sentry_privileges_for_provider is not empty
Posted by Colin Ma <ju...@intel.com>.
> On Oct. 8, 2015, 10:13 p.m., Sravya Tirukkovalur wrote:
> > sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/ResourceAuthorizationProvider.java, line 99
> > <https://reviews.apache.org/r/35485/diff/3/?file=1018348#file1018348line99>
> >
> > Also, it might be best to throw an exception in getGroups itself, otherwise we have multiple entry points into this function where we might want have to handle this.
It make sense, the patch is updated as your comments, thanks for review.
- Colin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35485/#review101969
-----------------------------------------------------------
On Oct. 9, 2015, 7:23 a.m., Colin Ma wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35485/
> -----------------------------------------------------------
>
> (Updated Oct. 9, 2015, 7:23 a.m.)
>
>
> Review request for sentry and Sravya Tirukkovalur.
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Make sure groups in list_sentry_privileges_for_provider is not empty
>
>
> Diffs
> -----
>
> sentry-binding/sentry-binding-hive/src/test/java/org/apache/sentry/binding/hive/TestHiveAuthzBindings.java 0622b43
> sentry-binding/sentry-binding-solr/src/test/java/org/apache/sentry/binding/solr/TestSolrAuthzBinding.java c37f8ff
> sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/HadoopGroupMappingService.java fb335a3
> sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/SentryGroupNotFoundException.java PRE-CREATION
> sentry-provider/sentry-provider-file/src/main/java/org/apache/sentry/provider/file/LocalGroupMappingService.java e22e6b6
> sentry-provider/sentry-provider-file/src/test/java/org/apache/sentry/provider/file/TestLocalGroupMapping.java c436009
> sentry-solr/solr-sentry-handlers/src/main/resources/sentry-handlers/sentry/test-authz-provider.ini 8f48a8c
> sentry-solr/solr-sentry-handlers/src/test/java/org/apache/solr/sentry/SentryIndexAuthorizationSingletonTest.java a3d7d19
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestUserManagement.java 471af1a
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/metastore/TestAuthorizingObjectStore.java 44ed096
> sentry-tests/sentry-tests-solr/src/test/resources/solr/sentry/test-authz-provider.ini 34a030d
>
> Diff: https://reviews.apache.org/r/35485/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Colin Ma
>
>
Re: Review Request 35485: SENTRY-769: [Improve error handling] Make
sure groups in list_sentry_privileges_for_provider is not empty
Posted by Sravya Tirukkovalur <sr...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35485/#review101969
-----------------------------------------------------------
sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/ResourceAuthorizationProvider.java (line 98)
<https://reviews.apache.org/r/35485/#comment159481>
Also, it might be best to throw an exception in getGroups itself, otherwise we have multiple entry points into this function where we might want have to handle this.
- Sravya Tirukkovalur
On July 22, 2015, 1:54 a.m., Colin Ma wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35485/
> -----------------------------------------------------------
>
> (Updated July 22, 2015, 1:54 a.m.)
>
>
> Review request for sentry and Sravya Tirukkovalur.
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Make sure groups in list_sentry_privileges_for_provider is not empty
>
>
> Diffs
> -----
>
> sentry-binding/sentry-binding-hive/src/test/java/org/apache/sentry/binding/hive/TestHiveAuthzBindings.java 0622b43
> sentry-binding/sentry-binding-solr/src/test/java/org/apache/sentry/binding/solr/TestSolrAuthzBinding.java c37f8ff
> sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/AuthorizationProvider.java a88d2f8
> sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/ProviderBackend.java ddb9cf9
> sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/ResourceAuthorizationProvider.java 06573b7
> sentry-solr/solr-sentry-handlers/src/main/resources/sentry-handlers/sentry/test-authz-provider.ini 8f48a8c
> sentry-solr/solr-sentry-handlers/src/test/java/org/apache/solr/sentry/SentryIndexAuthorizationSingletonTest.java a3d7d19
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestUserManagement.java fa34c33
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/metastore/TestAuthorizingObjectStore.java 30041c5
> sentry-tests/sentry-tests-solr/src/test/resources/solr/sentry/test-authz-provider.ini 34a030d
>
> Diff: https://reviews.apache.org/r/35485/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Colin Ma
>
>
Re: Review Request 35485: SENTRY-769: [Improve error handling] Make
sure groups in list_sentry_privileges_for_provider is not empty
Posted by Colin Ma <ju...@intel.com>.
> On Oct. 10, 2015, 6:21 p.m., Sravya Tirukkovalur wrote:
> > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/metastore/TestAuthorizingObjectStore.java, lines 68-69
> > <https://reviews.apache.org/r/35485/diff/5/?file=1094400#file1094400line68>
> >
> > Why was this change required?
Add user ACCESSAllMETAUSER for the test case testPrivilegesForUserNameCaseSensitive. Otherwise, SentryGroupNotFoundException will be thrown in testPrivilegesForUserNameCaseSensitive, what is not we wanted.
> On Oct. 10, 2015, 6:21 p.m., Sravya Tirukkovalur wrote:
> > sentry-solr/solr-sentry-handlers/src/main/resources/sentry-handlers/sentry/test-authz-provider.ini, line 36
> > <https://reviews.apache.org/r/35485/diff/5/?file=1094397#file1094397line36>
> >
> > Why was this needed?
Clarify this in the overall comments, thanks for your review.
> On Oct. 10, 2015, 6:21 p.m., Sravya Tirukkovalur wrote:
> > sentry-tests/sentry-tests-solr/src/test/resources/solr/sentry/test-authz-provider.ini, line 118
> > <https://reviews.apache.org/r/35485/diff/5/?file=1094401#file1094401line118>
> >
> > Why is this change required?
Clarify this in the overall comments.
On Oct. 10, 2015, 6:21 p.m., Colin Ma wrote:
> > Thanks for the changes Colin! Looks good to me.
> >
> > I see that there have been some changes to solr tests which I did not understand why they were required. Could you clarify?
Hi Sravya, thanks for the review. For these test changes, before this patch, these users haven't any group, the output for getGroups() will be an empty collection. The privileges will be also an empty collection, and the test cases will be passed.
But with this patch, there will be SentryGroupNotFoundException if getGroups() returns empty collection. I want to get the empty privileges as before to pass the test case, so I add the temp group for some users.
- Colin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35485/#review102174
-----------------------------------------------------------
On Oct. 10, 2015, 2:28 a.m., Colin Ma wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35485/
> -----------------------------------------------------------
>
> (Updated Oct. 10, 2015, 2:28 a.m.)
>
>
> Review request for sentry and Sravya Tirukkovalur.
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Make sure groups in list_sentry_privileges_for_provider is not empty
>
>
> Diffs
> -----
>
> sentry-binding/sentry-binding-hive/src/test/java/org/apache/sentry/binding/hive/TestHiveAuthzBindings.java 0622b43
> sentry-binding/sentry-binding-solr/src/test/java/org/apache/sentry/binding/solr/TestSolrAuthzBinding.java c37f8ff
> sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/HadoopGroupMappingService.java fb335a3
> sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/SentryGroupNotFoundException.java PRE-CREATION
> sentry-provider/sentry-provider-file/src/main/java/org/apache/sentry/provider/file/LocalGroupMappingService.java e22e6b6
> sentry-provider/sentry-provider-file/src/test/java/org/apache/sentry/provider/file/TestLocalGroupMapping.java c436009
> sentry-solr/solr-sentry-handlers/src/main/resources/sentry-handlers/sentry/test-authz-provider.ini 8f48a8c
> sentry-solr/solr-sentry-handlers/src/test/java/org/apache/solr/sentry/SentryIndexAuthorizationSingletonTest.java a3d7d19
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestUserManagement.java 471af1a
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/metastore/TestAuthorizingObjectStore.java 44ed096
> sentry-tests/sentry-tests-solr/src/test/resources/solr/sentry/test-authz-provider.ini 34a030d
>
> Diff: https://reviews.apache.org/r/35485/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Colin Ma
>
>
Re: Review Request 35485: SENTRY-769: [Improve error handling] Make
sure groups in list_sentry_privileges_for_provider is not empty
Posted by Sravya Tirukkovalur <sr...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35485/#review102174
-----------------------------------------------------------
sentry-solr/solr-sentry-handlers/src/main/resources/sentry-handlers/sentry/test-authz-provider.ini (line 36)
<https://reviews.apache.org/r/35485/#comment159729>
Why was this needed?
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/metastore/TestAuthorizingObjectStore.java (lines 68 - 69)
<https://reviews.apache.org/r/35485/#comment159730>
Why was this change required?
sentry-tests/sentry-tests-solr/src/test/resources/solr/sentry/test-authz-provider.ini (line 118)
<https://reviews.apache.org/r/35485/#comment159731>
Why is this change required?
Thanks for the changes Colin! Looks good to me.
I see that there have been some changes to solr tests which I did not understand why they were required. Could you clarify?
- Sravya Tirukkovalur
On Oct. 10, 2015, 2:28 a.m., Colin Ma wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35485/
> -----------------------------------------------------------
>
> (Updated Oct. 10, 2015, 2:28 a.m.)
>
>
> Review request for sentry and Sravya Tirukkovalur.
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Make sure groups in list_sentry_privileges_for_provider is not empty
>
>
> Diffs
> -----
>
> sentry-binding/sentry-binding-hive/src/test/java/org/apache/sentry/binding/hive/TestHiveAuthzBindings.java 0622b43
> sentry-binding/sentry-binding-solr/src/test/java/org/apache/sentry/binding/solr/TestSolrAuthzBinding.java c37f8ff
> sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/HadoopGroupMappingService.java fb335a3
> sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/SentryGroupNotFoundException.java PRE-CREATION
> sentry-provider/sentry-provider-file/src/main/java/org/apache/sentry/provider/file/LocalGroupMappingService.java e22e6b6
> sentry-provider/sentry-provider-file/src/test/java/org/apache/sentry/provider/file/TestLocalGroupMapping.java c436009
> sentry-solr/solr-sentry-handlers/src/main/resources/sentry-handlers/sentry/test-authz-provider.ini 8f48a8c
> sentry-solr/solr-sentry-handlers/src/test/java/org/apache/solr/sentry/SentryIndexAuthorizationSingletonTest.java a3d7d19
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestUserManagement.java 471af1a
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/metastore/TestAuthorizingObjectStore.java 44ed096
> sentry-tests/sentry-tests-solr/src/test/resources/solr/sentry/test-authz-provider.ini 34a030d
>
> Diff: https://reviews.apache.org/r/35485/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Colin Ma
>
>
Re: Review Request 35485: SENTRY-769: [Improve error handling] Make
sure groups in list_sentry_privileges_for_provider is not empty
Posted by Sravya Tirukkovalur <sr...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35485/#review102202
-----------------------------------------------------------
Ship it!
Ship It!
- Sravya Tirukkovalur
On Oct. 10, 2015, 2:28 a.m., Colin Ma wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35485/
> -----------------------------------------------------------
>
> (Updated Oct. 10, 2015, 2:28 a.m.)
>
>
> Review request for sentry and Sravya Tirukkovalur.
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Make sure groups in list_sentry_privileges_for_provider is not empty
>
>
> Diffs
> -----
>
> sentry-binding/sentry-binding-hive/src/test/java/org/apache/sentry/binding/hive/TestHiveAuthzBindings.java 0622b43
> sentry-binding/sentry-binding-solr/src/test/java/org/apache/sentry/binding/solr/TestSolrAuthzBinding.java c37f8ff
> sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/HadoopGroupMappingService.java fb335a3
> sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/SentryGroupNotFoundException.java PRE-CREATION
> sentry-provider/sentry-provider-file/src/main/java/org/apache/sentry/provider/file/LocalGroupMappingService.java e22e6b6
> sentry-provider/sentry-provider-file/src/test/java/org/apache/sentry/provider/file/TestLocalGroupMapping.java c436009
> sentry-solr/solr-sentry-handlers/src/main/resources/sentry-handlers/sentry/test-authz-provider.ini 8f48a8c
> sentry-solr/solr-sentry-handlers/src/test/java/org/apache/solr/sentry/SentryIndexAuthorizationSingletonTest.java a3d7d19
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestUserManagement.java 471af1a
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/metastore/TestAuthorizingObjectStore.java 44ed096
> sentry-tests/sentry-tests-solr/src/test/resources/solr/sentry/test-authz-provider.ini 34a030d
>
> Diff: https://reviews.apache.org/r/35485/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Colin Ma
>
>
Re: Review Request 35485: SENTRY-769: [Improve error handling] Make
sure groups in list_sentry_privileges_for_provider is not empty
Posted by Colin Ma <ju...@intel.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35485/
-----------------------------------------------------------
(Updated Oct. 10, 2015, 2:28 a.m.)
Review request for sentry and Sravya Tirukkovalur.
Repository: sentry
Description
-------
Make sure groups in list_sentry_privileges_for_provider is not empty
Diffs (updated)
-----
sentry-binding/sentry-binding-hive/src/test/java/org/apache/sentry/binding/hive/TestHiveAuthzBindings.java 0622b43
sentry-binding/sentry-binding-solr/src/test/java/org/apache/sentry/binding/solr/TestSolrAuthzBinding.java c37f8ff
sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/HadoopGroupMappingService.java fb335a3
sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/SentryGroupNotFoundException.java PRE-CREATION
sentry-provider/sentry-provider-file/src/main/java/org/apache/sentry/provider/file/LocalGroupMappingService.java e22e6b6
sentry-provider/sentry-provider-file/src/test/java/org/apache/sentry/provider/file/TestLocalGroupMapping.java c436009
sentry-solr/solr-sentry-handlers/src/main/resources/sentry-handlers/sentry/test-authz-provider.ini 8f48a8c
sentry-solr/solr-sentry-handlers/src/test/java/org/apache/solr/sentry/SentryIndexAuthorizationSingletonTest.java a3d7d19
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestUserManagement.java 471af1a
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/metastore/TestAuthorizingObjectStore.java 44ed096
sentry-tests/sentry-tests-solr/src/test/resources/solr/sentry/test-authz-provider.ini 34a030d
Diff: https://reviews.apache.org/r/35485/diff/
Testing
-------
Thanks,
Colin Ma
Re: Review Request 35485: SENTRY-769: [Improve error handling] Make
sure groups in list_sentry_privileges_for_provider is not empty
Posted by Sravya Tirukkovalur <sr...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35485/#review102106
-----------------------------------------------------------
sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/HadoopGroupMappingService.java (line 65)
<https://reviews.apache.org/r/35485/#comment159620>
If there is an IOException, we should not swallow it. I think it would be best to wrap it in Group not found exception and throw it.
- Sravya Tirukkovalur
On Oct. 9, 2015, 7:23 a.m., Colin Ma wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35485/
> -----------------------------------------------------------
>
> (Updated Oct. 9, 2015, 7:23 a.m.)
>
>
> Review request for sentry and Sravya Tirukkovalur.
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Make sure groups in list_sentry_privileges_for_provider is not empty
>
>
> Diffs
> -----
>
> sentry-binding/sentry-binding-hive/src/test/java/org/apache/sentry/binding/hive/TestHiveAuthzBindings.java 0622b43
> sentry-binding/sentry-binding-solr/src/test/java/org/apache/sentry/binding/solr/TestSolrAuthzBinding.java c37f8ff
> sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/HadoopGroupMappingService.java fb335a3
> sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/SentryGroupNotFoundException.java PRE-CREATION
> sentry-provider/sentry-provider-file/src/main/java/org/apache/sentry/provider/file/LocalGroupMappingService.java e22e6b6
> sentry-provider/sentry-provider-file/src/test/java/org/apache/sentry/provider/file/TestLocalGroupMapping.java c436009
> sentry-solr/solr-sentry-handlers/src/main/resources/sentry-handlers/sentry/test-authz-provider.ini 8f48a8c
> sentry-solr/solr-sentry-handlers/src/test/java/org/apache/solr/sentry/SentryIndexAuthorizationSingletonTest.java a3d7d19
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestUserManagement.java 471af1a
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/metastore/TestAuthorizingObjectStore.java 44ed096
> sentry-tests/sentry-tests-solr/src/test/resources/solr/sentry/test-authz-provider.ini 34a030d
>
> Diff: https://reviews.apache.org/r/35485/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Colin Ma
>
>
Re: Review Request 35485: SENTRY-769: [Improve error handling] Make
sure groups in list_sentry_privileges_for_provider is not empty
Posted by Colin Ma <ju...@intel.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35485/
-----------------------------------------------------------
(Updated Oct. 9, 2015, 7:23 a.m.)
Review request for sentry and Sravya Tirukkovalur.
Repository: sentry
Description
-------
Make sure groups in list_sentry_privileges_for_provider is not empty
Diffs (updated)
-----
sentry-binding/sentry-binding-hive/src/test/java/org/apache/sentry/binding/hive/TestHiveAuthzBindings.java 0622b43
sentry-binding/sentry-binding-solr/src/test/java/org/apache/sentry/binding/solr/TestSolrAuthzBinding.java c37f8ff
sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/HadoopGroupMappingService.java fb335a3
sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/SentryGroupNotFoundException.java PRE-CREATION
sentry-provider/sentry-provider-file/src/main/java/org/apache/sentry/provider/file/LocalGroupMappingService.java e22e6b6
sentry-provider/sentry-provider-file/src/test/java/org/apache/sentry/provider/file/TestLocalGroupMapping.java c436009
sentry-solr/solr-sentry-handlers/src/main/resources/sentry-handlers/sentry/test-authz-provider.ini 8f48a8c
sentry-solr/solr-sentry-handlers/src/test/java/org/apache/solr/sentry/SentryIndexAuthorizationSingletonTest.java a3d7d19
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestUserManagement.java 471af1a
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/metastore/TestAuthorizingObjectStore.java 44ed096
sentry-tests/sentry-tests-solr/src/test/resources/solr/sentry/test-authz-provider.ini 34a030d
Diff: https://reviews.apache.org/r/35485/diff/
Testing
-------
Thanks,
Colin Ma