Re: Review Request 63596: SENTRY-1951 - Old SentryStore.retrieveFullPathsImage() should be removed

[Arjun Mishra via Review Board <no...@reviews.apache.org>]

> On Nov. 30, 2017, 3:09 p.m., Xinran Tinney wrote:
> > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java
> > Lines 3574-3580 (patched)
> > <https://reviews.apache.org/r/63596/diff/5/?file=1903825#file1903825line3576>
> >
> >     Since this part is repeating through multiple files, is it helpful to create a function and call the function?

The purpose of buildPathsImageMap() method was to do that. The resof it won't make sense since all calls are pass by value. I could create a private class but I don't see a need for that


- Arjun


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


On Nov. 29, 2017, 5:23 p.m., Arjun Mishra wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63596/
> -----------------------------------------------------------
> 
> (Updated Nov. 29, 2017, 5:23 p.m.)
> 
> 
> Review request for sentry, Alexander Kolbasov, kalyan kumar kalvagadda, Na Li, Sergio Pena, and Vadim Spector.
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> The old retrieveFullPathsImage() method in SentryStore is no longer used by actual code (retrieveFullPathsImageUpdate(final String[] prefixes) is used instead. It was preserved because it is used by test which now doesn't make much sense.
> 
> 
> Diffs
> -----
> 
>   sentry-hdfs/sentry-hdfs-service/src/test/java/org/apache/sentry/hdfs/TestImageRetriever.java b355630e7 
>   sentry-hdfs/sentry-hdfs-service/src/test/java/org/apache/sentry/hdfs/TestSentryHDFSServiceProcessor.java f09d1b228 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java f32a745ed 
>   sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java 24b11f724 
> 
> 
> Diff: https://reviews.apache.org/r/63596/diff/5/
> 
> 
> Testing
> -------
> 
> mvn -f sentry-provider/sentry-provider-db/pom.xml test -Dtest=TestSentryStore
> mvn -f sentry-hdfs/sentry-hdfs-service/pom.xml test -Dtest=TestSentryHDFSServiceProcessor
> mvn -f sentry-hdfs/sentry-hdfs-service/pom.xml test -Dtest=TestImageRetriever.java
> 
> 
> Thanks,
> 
> Arjun Mishra
> 
>