----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45727/#review127039 -----------------------------------------------------------
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java (line 2050) <https://reviews.apache.org/r/45727/#comment190171> We shall consider to share code between the two versions getGroupNameRoleNamesMap instead two very similar copies.For example: public Map<String, Set<String>> getGroupNameRoleNamesMap() { return getGroupNameRoleNamesMap(null); } And implement getGroupNameRoleNamesMap with role names parameter to handle the case of null in which means all role names. sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java (line 2120) <https://reviews.apache.org/r/45727/#comment190172> It is possible to share most of the code with the existing getRoleNameTPrivilegesMap method without parameters similar the above suggestion. getRoleNameTPrivilegesMap() { return getRoleNameTPrivilegesMap(null, null); } Currently, we see quite different approach for the two methods but they can be consistent and share the code. sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java (line 2138) <https://reviews.apache.org/r/45727/#comment190174> sentryRolePrivilegesMap: local variable, doen'st have to be so long. It can be as simple as rolePrivilegesMap. It also nice to put the code from 2138 to 2153 into method so simplify the code logic and also reduce levels of code blocks. - Jerry Chen On April 5, 2016, 3:51 a.m., Colin Ma wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/45727/ > ----------------------------------------------------------- > > (Updated April 5, 2016, 3:51 a.m.) > > > Review request for sentry. > > > Repository: sentry > > > Description > ------- > > Update SentryStore for export with specific auth object > > > Diffs > ----- > > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java > dbb5d8e > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStoreImportExport.java > a9e4ed6 > > Diff: https://reviews.apache.org/r/45727/diff/ > > > Testing > ------- > > > Thanks, > > Colin Ma > >
