> On Sept. 2, 2014, 4:57 a.m., shwethags wrote: > > common/src/main/java/org/apache/falcon/entity/Storage.java, line 102 > > <https://reviews.apache.org/r/25211/diff/2/?file=673444#file673444line102> > > > > should have args - retentionLimit and timezone. It can probably return > > back list of instances deleted. FeedEvictor class can then log these > > instances as its common to both catalog storage and filesystem storage
Your suggestion is completely correct and the way this code is written is slightly convoluted e.g. the OUT is being used by Tests (passed through feedevictor) As part of this JIRA I have just separated the concerns to respective storage systems and kept true to original implementation. I was planning to handle the refactoring of Tests and Evictor logic, in separate JIRA. Thoughts? - Ajay ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25211/#review52014 ----------------------------------------------------------- On Sept. 1, 2014, 1:31 p.m., Ajay Yadava wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/25211/ > ----------------------------------------------------------- > > (Updated Sept. 1, 2014, 1:31 p.m.) > > > Review request for Falcon and shwethags. > > > Repository: falcon-git > > > Description > ------- > > Earlier FeedEviction class contained logic for both FileSystemStorage and > TableStorage. Corresponding code was being called using an if else. To make > the code cleaner and more manageable I moved the code to the appropriate > storage class and delegated feed eviction to the appropriate Storage > implementation. Needed to add evict method to Storage Interface and make some > minor changes here and there. > > > Diffs > ----- > > common/src/main/java/org/apache/falcon/entity/CatalogStorage.java 7ad0716 > common/src/main/java/org/apache/falcon/entity/FileSystemStorage.java > 4eb3d60 > common/src/main/java/org/apache/falcon/entity/Storage.java f88e139 > retention/src/main/java/org/apache/falcon/retention/FeedEvictor.java > 4de7938 > > retention/src/test/java/org/apache/falcon/retention/FeedEvictorFileSystemTest.java > PRE-CREATION > retention/src/test/java/org/apache/falcon/retention/FeedEvictorTest.java > eb4173e > > Diff: https://reviews.apache.org/r/25211/diff/ > > > Testing > ------- > > All FeedEvictor tests passed. > > > Thanks, > > Ajay Yadava > >
