----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31070/#review83272 -----------------------------------------------------------
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. sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java <https://reviews.apache.org/r/31070/#comment134175> 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. sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java <https://reviews.apache.org/r/31070/#comment134193> It would be useful to provide an option of merging and overwritting the existing roles sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java <https://reviews.apache.org/r/31070/#comment134194> Should it handle merging of privilege, eg if you already have 'all' or appending 'all' then the others can be discarded. - Prasad Mujumdar 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 > >
