----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34087/#review127561 -----------------------------------------------------------
sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/ResourceAuthorizationProvider.java (line 179) <https://reviews.apache.org/r/34087/#comment190935> Suggest to remove: Set<String> result = Sets.newHashSet(); And use the following instead: Set<String> result = policy... sentry-provider/sentry-provider-file/src/main/java/org/apache/sentry/provider/file/PolicyFile.java (line 99) <https://reviews.apache.org/r/34087/#comment190937> Question: What is appendUserGroupsMappings for? Do we need this? Is it used only for test cases? The naming may need to be reconsidered if we want to keep it. Names like UserGroupsMappings is very tricky. Consider a simple name: addUserGroupMappings? sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java (line 259) <https://reviews.apache.org/r/34087/#comment190944> Unclear what this is for: show the reason how the users and groups is related in testing roles for user feature? - Jerry Chen On April 6, 2016, 2:58 p.m., Colin Ma wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/34087/ > ----------------------------------------------------------- > > (Updated April 6, 2016, 2:58 p.m.) > > > Review request for sentry, Dapeng Sun and Jerry Chen. > > > Repository: sentry > > > Description > ------- > > Update AuthorizationProvider and e2e test for grant user to role > > > Diffs > ----- > > > sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/ResourceAuthorizationProvider.java > 0cf0b5d > > sentry-provider/sentry-provider-common/src/test/java/org/apache/sentry/provider/common/TestGetGroupMapping.java > 14af2d4 > > sentry-provider/sentry-provider-file/src/main/java/org/apache/sentry/provider/file/PolicyFile.java > 991a95f > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestGrantUserToRole.java > PRE-CREATION > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java > 8515a2b > > Diff: https://reviews.apache.org/r/34087/diff/ > > > Testing > ------- > > > Thanks, > > Colin Ma > >
