You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@falcon.apache.org by sh...@inmobi.com on 2014/09/01 08:28:03 UTC

Re: Review Request 25211: Delegated feed eviction to the appropriate Storage implementation

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



common/src/main/java/org/apache/falcon/entity/Storage.java
<https://reviews.apache.org/r/25211/#comment90691>

    shoudl throw only FalconException


move the corresponding UTs as well

- shwethags


On Aug. 30, 2014, 7:22 a.m., Ajay Yadava wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25211/
> -----------------------------------------------------------
> 
> (Updated Aug. 30, 2014, 7:22 a.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 
> 
> Diff: https://reviews.apache.org/r/25211/diff/
> 
> 
> Testing
> -------
> 
> All FeedEvictor tests passed.
> 
> 
> Thanks,
> 
> Ajay Yadava
> 
>


Re: Review Request 25211: Delegated feed eviction to the appropriate Storage implementation

Posted by Ajay Yadava <aj...@gmail.com>.

> On Sept. 1, 2014, 6:28 a.m., shwethags wrote:
> > common/src/main/java/org/apache/falcon/entity/Storage.java, line 98
> > <https://reviews.apache.org/r/25211/diff/1/?file=672661#file672661line98>
> >
> >     shoudl throw only FalconException

Will make the necessary changes and submit a new patch.


- Ajay


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


On Aug. 30, 2014, 7:22 a.m., Ajay Yadava wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25211/
> -----------------------------------------------------------
> 
> (Updated Aug. 30, 2014, 7:22 a.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 
> 
> Diff: https://reviews.apache.org/r/25211/diff/
> 
> 
> Testing
> -------
> 
> All FeedEvictor tests passed.
> 
> 
> Thanks,
> 
> Ajay Yadava
> 
>


Re: Review Request 25211: Delegated feed eviction to the appropriate Storage implementation

Posted by Ajay Yadava <aj...@gmail.com>.

On Sept. 1, 2014, 6:28 a.m., Ajay Yadava wrote:
> > move the corresponding UTs as well

Good Point. I gave it some thought and observed that the UTs are around various aspects of feed eviction only and are intended to test FeedEvictor, with fileSystemStorage being used for storage, falcon-retention still seems more natural place. Let me know if you still want me to move it to falcon-common.


- Ajay


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


On Aug. 30, 2014, 7:22 a.m., Ajay Yadava wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25211/
> -----------------------------------------------------------
> 
> (Updated Aug. 30, 2014, 7:22 a.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 
> 
> Diff: https://reviews.apache.org/r/25211/diff/
> 
> 
> Testing
> -------
> 
> All FeedEvictor tests passed.
> 
> 
> Thanks,
> 
> Ajay Yadava
> 
>