> On May 11, 2015, 9:15 p.m., Prasad Mujumdar wrote: > > At high level, > > - please see if the existing methods can be reused to load or update the > > meadata. Having multiple routines to do similar tasks make the code hard to > > maintain. > > - It would be a good idea to provide an option of merging and overwriting > > the existing data during import.
Thanks for the comments, for the point1, the methods for import [group, role] and [role, privilege] are reimplemented, I think the new code will be easy to maintain using the exist method in SentryStore. for point2, the option is added for import. > On May 11, 2015, 9:15 p.m., Prasad Mujumdar wrote: > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java, > > line 2013 > > <https://reviews.apache.org/r/31070/diff/3/?file=888609#file888609line2013> > > > > I think we are duplicating the code here for getting privileges for > > role. eg. there's already getAllTSentryPrivilegesByRoleName(). > > Please check if it's possible to consolidate the code for some of these > > operations. The code is updated for this problem, thanks. > On May 11, 2015, 9:15 p.m., Prasad Mujumdar wrote: > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java, > > line 2162 > > <https://reviews.apache.org/r/31070/diff/3/?file=888609#file888609line2162> > > > > It would be useful to provide an option of merging and overwritting the > > existing roles done. > On May 11, 2015, 9:15 p.m., Prasad Mujumdar wrote: > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java, > > line 2225 > > <https://reviews.apache.org/r/31070/diff/3/?file=888609#file888609line2225> > > > > Should it handle merging of privilege, eg if you already have 'all' or > > appending 'all' then the others can be discarded. Update the import logical, the exist method alterSentryRoleGrantPrivilegeCore() is used for import privileges. - Colin ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31070/#review83272 ----------------------------------------------------------- On March 9, 2015, 2:49 a.m., Colin Ma wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/31070/ > ----------------------------------------------------------- > > (Updated March 9, 2015, 2:49 a.m.) > > > Review request for sentry. > > > 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 > 136dab6 > > 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 > >
