> 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 > >
