----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/42867/#review116619 -----------------------------------------------------------
sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/PathsUpdate.java (line 113) <https://reviews.apache.org/r/42867/#comment177692> Creating the config object is expensive, you should create this once as a static variable and then use it from then on. You may need a test hook for local testing to pass in a modified configuration object. This should only happen for local testing though since we want to validate that on a real cluster we are testing the product code. There is a flag for external service which you can consider using sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/PathsUpdate.java (line 118) <https://reviews.apache.org/r/42867/#comment177693> Add "Preconditions.checkNotNull(scheme)". Add comment saying that non-HDFS paths are skipped and paths without a scheme default to default scheme. sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegration.java (line 1399) <https://reviews.apache.org/r/42867/#comment177694> nit: scheme-less sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegration.java (line 1401) <https://reviews.apache.org/r/42867/#comment177695> Need some validation that HDFS sync is doing the right thing here - Lenni Kuff On Jan. 27, 2016, 8:40 p.m., Hao Hao wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/42867/ > ----------------------------------------------------------- > > (Updated Jan. 27, 2016, 8:40 p.m.) > > > Review request for sentry. > > > Repository: sentry > > > Description > ------- > > Change-Id: I9976086dbdf3c57c931d729870eeeaa6f7e6fa29 > There a few edge cases where Paths update may be passed a path which lacks a > URI Scheme. > i.e /user/hive/warehouse > In this situation PathsUpdate will throw an uncaught NPE at the Precondition > check. > Use the default scheme of the filesystem and silently ignore other schemeless > paths. > > > Diffs > ----- > > > sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/PathsUpdate.java > 1dcb75a3dc7735e11289ab75424199ab1a086ce2 > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegration.java > fc7f3245a274a60d811fdbbe10119ad54fa687cd > > Diff: https://reviews.apache.org/r/42867/diff/ > > > Testing > ------- > > Test done in TestHDFSIntegration.testMissingScheme. > > > Thanks, > > Hao Hao > >
