> On Sept. 1, 2017, 7 p.m., Vadim Spector wrote: > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/FullUpdateModifier.java > > Line 142 (original), 143 (patched) > > <https://reviews.apache.org/r/62007/diff/3/?file=1808686#file1808686line143> > > > > Applicable to more than one place in both FullUpdateModifier and > > FullUpdateInitializer: seems odd that first Set is replaced with > > Collection, only to follow by downcasting Collection back to Set. Why not > > to have method signatures keep the actual type? Replacing subclass with > > su[erclass makes sense when either a) you trully do not need subclass, so > > you just keep your code more generic, or b) the code of the method can > > downcast to more than one subclass, depending on the circumstances. Neither > > seems to apply. Besides, cast exception is always cleaner to be thrown up > > the call chain.
Even if its justified, then perhaps add little javadoc explaining this? - Vadim ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62007/#review184387 ----------------------------------------------------------- On Aug. 31, 2017, 4:14 p.m., Alexander Kolbasov wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/62007/ > ----------------------------------------------------------- > > (Updated Aug. 31, 2017, 4:14 p.m.) > > > Review request for sentry, Arjun Mishra, Brian Towles, Misha Dmitriev, Na Li, > Sergio Pena, Vamsee Yarlagadda, and Vadim Spector. > > > Bugs: SENTRY-1909 > https://issues.apache.org/jira/browse/SENTRY-1909 > > > Repository: sentry > > > Description > ------- > > SENTRY-1909 Improvements for memory usage when full path snapshot is sent > from Sentry to NN > > > Diffs > ----- > > > sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/PathImageRetriever.java > 898c7bec2e35e6f1424478666282ba78222da709 > > sentry-hdfs/sentry-hdfs-service/src/test/java/org/apache/sentry/hdfs/TestImageRetriever.java > 20b3e108cc976207a3809bc6a214a34e10788200 > > sentry-hdfs/sentry-hdfs-service/src/test/java/org/apache/sentry/hdfs/TestSentryHDFSServiceProcessor.java > e403d7cca5dca5c39785490f92e562dc9a3b4daf > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MAuthzPathsMapping.java > fee5279a7cd3d60df4e60a5f25d2e8604c37d04b > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/PathsImage.java > 409a557fe89d85df888c1f16b3f5aacdd9aa96b0 > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java > 593b92f96b47f959266280bce3d0809f6a80c362 > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/FullUpdateInitializer.java > 760a2b55e98ac1fb305666cbbedfba11d26f2fcc > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/FullUpdateModifier.java > 2cd18ea9d2ca40b5f46aaafde532fb3bf9769c07 > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryHMSClient.java > 4a8fb953ce486e1aeb1042884de56872b5539cd0 > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java > 3da6a4e998d36cada7e9ea77285b11f07cea5f25 > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/TestFullUpdateInitializer.java > 9d7bddd339513180e627286d8a749b7814c824f2 > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/TestFullUpdateModifier.java > 7deccb064fd75b39af779796d9e95caf9c718b32 > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/TestHMSFollower.java > f56384a99e128b3e9880cbc5692107d61f2f500f > > > Diff: https://reviews.apache.org/r/62007/diff/3/ > > > Testing > ------- > > > Thanks, > > Alexander Kolbasov > >
