> On June 21, 2016, 8:44 p.m., Colin McCabe wrote: > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java, > > line 2004 > > <https://reviews.apache.org/r/48770/diff/2/?file=1423133#file1423133line2004> > > > > should rollbackTransaction be set to false here? It's a bit concerning > > if the unit test coverage isn't catching this?
The rollbackTransaction will check if the PersistenceManager is closed and if the transaction is still active before roll back. That is why the test did not give any error. > On June 21, 2016, 8:44 p.m., Colin McCabe wrote: > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/package.jdo, > > line 245 > > <https://reviews.apache.org/r/48770/diff/2/?file=1423132#file1423132line245> > > > > How do we know that 128 characters is long enough for any authZ object > > name? Good question! So I checked the hive schema, it looks like database name, table name and column name are all defined as 128 characters long. So we can define it as 384(128*3) characters? > On June 21, 2016, 8:44 p.m., Colin McCabe wrote: > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MAuthzPathsMapping.java, > > line 67 > > <https://reviews.apache.org/r/48770/diff/2/?file=1423131#file1423131line67> > > > > Hmm. Should createTime be included in the toString output? Will add it. - Hao ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/48770/#review138930 ----------------------------------------------------------- On June 17, 2016, 8:32 p.m., Hao Hao wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/48770/ > ----------------------------------------------------------- > > (Updated June 17, 2016, 8:32 p.m.) > > > Review request for sentry, Colin McCabe and Sravya Tirukkovalur. > > > Repository: sentry > > > Description > ------- > > SENTRY-1325: Store HMSPaths in Sentry DB to allow fast failover > > Change-Id: Icfb86d86b8cdbbf786c48780cc3ee8ec8a2b2698 > > Added schema for storing HMSPaths: MSentryPathsUpdate. It has hiveObj, paths, > createTime. > Added datastore APIs: retrieveFullPathsImage, and createSentryPathsUpdate. > > > Diffs > ----- > > > sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/HMSPaths.java > 6d2ab23c3f65af5430ea02563e13c4c4c49aa1c6 > > sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/HMSPathsDumper.java > e759ff1229353570c5fc1a8547f47127e2dabff1 > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MAuthzPathsMapping.java > PRE-CREATION > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/package.jdo > b3b949400c2a6dd5797a678da8966617b0598cca > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java > b7ef0e94fac911e97e4ae9ba1cde49edbe1eb62b > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java > bc7fe12b732e523ad6f50fd1daf5078cc37614f1 > > Diff: https://reviews.apache.org/r/48770/diff/ > > > Testing > ------- > > Added a test case in TestSentryStore#testPathsUpdate. > > > Thanks, > > Hao Hao > >
