----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54729/#review159412 -----------------------------------------------------------
sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/PathsUpdate.java (line 161) <https://reviews.apache.org/r/54729/#comment231969> Okease add javadocs for the new functions sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/PathsUpdate.java (line 162) <https://reviews.apache.org/r/54729/#comment231942> Can we define "/" as a constant (if it isn't in some hadoop libraries that we use already)? sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/PermissionsUpdate.java (line 113) <https://reviews.apache.org/r/54729/#comment231977> Can we have a common static functions for serialization/deserialization that are used both here and in PathsUpdate so that we don't duplicate code? sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/Updateable.java (line 44) <https://reviews.apache.org/r/54729/#comment231973> Please add javadoc for methods sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/Updateable.java (line 45) <https://reviews.apache.org/r/54729/#comment231975> Naming - may be change to JSONSerialize and JSONDeserialize? sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPathChange.java (line 26) <https://reviews.apache.org/r/54729/#comment230427> You need to use some HTML formatting here - use < and > for <> and <p> for empty line - otherwise javadoc looks as one big long line. sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPathChange.java (line 82) <https://reviews.apache.org/r/54729/#comment230428> What is the idea here of multiplying 31 by one? And why changeId is not participating in the calculation? Can we just use changeId as the hashcode? sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPermChange.java (line 29) <https://reviews.apache.org/r/54729/#comment230430> Add <p> - Alexander Kolbasov On Dec. 14, 2016, 2:35 a.m., Hao Hao wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/54729/ > ----------------------------------------------------------- > > (Updated Dec. 14, 2016, 2:35 a.m.) > > > Review request for sentry, Alexander Kolbasov, kalyan kumar kalvagadda, and > Vamsee Yarlagadda. > > > Repository: sentry > > > Description > ------- > > Create schema for storing HMS path change and Sentry permission change. It > contains change ID, timestamp and the change event in JSON format. > > > Diffs > ----- > > > sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/PathsUpdate.java > 7cfb3bf57bd1a425b07df6c08db31b9691dd17f5 > > sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/PermissionsUpdate.java > 98349232bc658c39791e58b64949ecb975fff7a0 > > sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/Updateable.java > 4dc3a0cebdff89ee2f9070e4d822a28dbd164c08 > > sentry-hdfs/sentry-hdfs-common/src/test/java/org/apache/sentry/hdfs/TestPathsUpdate.java > 53243b44329997e64ab70db5e5d8c3614ab974d6 > > sentry-hdfs/sentry-hdfs-common/src/test/java/org/apache/sentry/hdfs/TestPermissionUpdate.java > PRE-CREATION > > sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/UpdateForwarder.java > ea1c8f62f300abd03206fdd9ec76fbf216fc6a2d > > sentry-hdfs/sentry-hdfs-service/src/test/java/org/apache/sentry/hdfs/TestUpdateForwarder.java > 315d4b3a80b9ab3ac8704ecbab81876f141423f1 > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPathChange.java > PRE-CREATION > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPermChange.java > PRE-CREATION > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/package.jdo > fb5470f635e03c762beb5bc2c6b06641b2476ce9 > > Diff: https://reviews.apache.org/r/54729/diff/ > > > Testing > ------- > > > Thanks, > > Hao Hao > >
