> On June 16, 2015, 10:43 p.m., Sravya Tirukkovalur wrote: > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java, > > line 2029 > > <https://reviews.apache.org/r/31070/diff/5/?file=976411#file976411line2029> > > > > When will roleNames be non empty?
You're right, in the loop, the group always be a new one for sentryGroupNameRoleNamesMap, so the roleNames should always be empty. Update the code. > On June 16, 2015, 10:43 p.m., Sravya Tirukkovalur wrote: > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java, > > line 2207 > > <https://reviews.apache.org/r/31070/diff/5/?file=976411#file976411line2207> > > > > Looks like the import logic is quite complex. Cant we just keep it to > > this? > > > > if(overwrite) { > > pm.newQuery(MSentryRole.class).deletePersistentAll(); > > pm.newQuery(MSentryPrivilege.class).deletePersistentAll(); > > pm.newQuery(MSentryGroup.class).deletePersistentAll(); > > } > > > > for(role:roles) > > create role if not exists > > for(privilege:privileges) > > grant privilege to role > > for(group:groups) > > for(role:group.getRoles) > > grant role to group > > > > > > Also, please comment the code more liberally. I agree the import logic is complex, and the current patch is more simplify. For the overwrite, from Prasad's comments, I think we need overwrite the exist role, not clear the DB and using the imported data. So, I didn't use the overwrite logic in your example code. For the import logic, the current patch is similiar as yours. - Colin ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31070/#review87973 ----------------------------------------------------------- On June 19, 2015, 1:22 a.m., Colin Ma wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/31070/ > ----------------------------------------------------------- > > (Updated June 19, 2015, 1:22 a.m.) > > > Review request for sentry and Sravya Tirukkovalur. > > > Repository: sentry > > > Description > ------- > > Update SentryStore for import/export feature > > > Diffs > ----- > > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java > d7937d0 > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStoreImportExport.java > PRE-CREATION > > Diff: https://reviews.apache.org/r/31070/diff/ > > > Testing > ------- > > > Thanks, > > Colin Ma > >
