----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58284/#review171918 -----------------------------------------------------------
sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/FullUpdateInitializer.java Line 220 (original), 305 (patched) <https://reviews.apache.org/r/58284/#comment244925> DbTask is returning an empty object mapping, why is this not necessary with tables? sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/FullUpdateInitializer.java Lines 361 (patched) <https://reviews.apache.org/r/58284/#comment244923> do we want to process a DB without a location? sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/FullUpdateInitializer.java Lines 362 (patched) <https://reviews.apache.org/r/58284/#comment244924> shouldn't be better to have a singleton for emptyObjectMapping? or we want different ObjectMapping with empty maps? sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/FullUpdateInitializer.java Line 314 (original), 415 (patched) <https://reviews.apache.org/r/58284/#comment244919> why is the 'failOnRetry' not needed anymore? isn't needed? sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/FullUpdateInitializer.java Line 317 (original), 416 (patched) <https://reviews.apache.org/r/58284/#comment244920> are all executors killed when an exception is thrown here? if not, do we want to kill the current work because of the failure? sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/FullUpdateInitializer.java Lines 419 (patched) <https://reviews.apache.org/r/58284/#comment244921> should it be good to have a new method for this merging code? sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/PathsUpdate.java Line 99 (original), 100 (patched) <https://reviews.apache.org/r/58284/#comment244927> isn't setDefaultSchema a better name? Btw, Is it Scheme or Schema? If it is for teting, do we want to keep it package-private only? sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/PathsUpdate.java Lines 118 (patched) <https://reviews.apache.org/r/58284/#comment244926> the method javadoc indicates that null is returned if path is null/empty." but here we're throwing an exception. Which one is true? - Sergio Pena On April 10, 2017, 5:42 a.m., Alexander Kolbasov wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/58284/ > ----------------------------------------------------------- > > (Updated April 10, 2017, 5:42 a.m.) > > > Review request for sentry, Hao Hao, kalyan kumar kalvagadda, Na Li, Sergio > Pena, Vamsee Yarlagadda, and Vadim Spector. > > > Bugs: SENTRY-1687 > https://issues.apache.org/jira/browse/SENTRY-1687 > > > Repository: sentry > > > Description > ------- > > SENTRY-1687 FullUpdateInitializer can be more efficient > > > Diffs > ----- > > > sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/FullUpdateInitializer.java > 90aaaef0e15306627d7108f12a74a29848055c0b > > sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/PathsUpdate.java > 14e967aa1065f16e8d4c3f61db2f9055959fa9e6 > > sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/ServiceConstants.java > 23552c2512902a8500bfacb1c745ca4b10498cc8 > > sentry-hdfs/sentry-hdfs-common/src/test/java/org/apache/sentry/hdfs/TestFullUpdateInitializer.java > f338ce8abddcde128536a0efef77983990325aa6 > > sentry-hdfs/sentry-hdfs-common/src/test/java/org/apache/sentry/hdfs/TestPathsUpdate.java > b5cbea9d295385bb38b912c13903edace04d7589 > > sentry-hdfs/sentry-hdfs-common/src/test/java/org/apache/sentry/hdfs/TestUpdateableAuthzPaths.java > e643e01a45de77ef7f465d6921c5ae9e7ce925a2 > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSFollower.java > 16676fb13b0d5015aefe892a6f7e46812ea75124 > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationAdvanced.java > 24ab1a8b392f23bc75759733bef7cecd4bc2ac34 > > > Diff: https://reviews.apache.org/r/58284/diff/1/ > > > Testing > ------- > > Updated the unit test to test for bigger HMS snapshots > > > Thanks, > > Alexander Kolbasov > >
