-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62007/#review184387
-----------------------------------------------------------




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/#comment260556>

    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.


- Vadim Spector


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
> 
>

Reply via email to