> On Dec. 14, 2015, 1:53 p.m., Peeyush Bishnoi wrote:
> > common/src/main/java/org/apache/falcon/entity/FeedHelper.java, line 1015
> > <https://reviews.apache.org/r/41339/diff/1/?file=1162237#file1162237line1015>
> >
> >     If there is any code commonality between getRetentionFrequency and 
> > getLifecycleRetentionFrequency, can you think to merge that and invoke it 
> > appropriately from single function. I know that the 
> > getLifecycleRetentionFrequency is for lifecycle and getRetentionFrequency 
> > is for normal retention.

If the retention stage is null, then in case of getLifecycleRetentionFrequency, 
we return null, whereas for the getRetentionFrequency, we return the retention 
limit of the cluster. If we try to merge them, we won't be able to handle this 
case. So, IMO its better if we keep them separate. What do you think?


- Narayan


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


On Dec. 14, 2015, 10:17 a.m., Narayan Periwal wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41339/
> -----------------------------------------------------------
> 
> (Updated Dec. 14, 2015, 10:17 a.m.)
> 
> 
> Review request for Falcon.
> 
> 
> Bugs: FALCON-1617
>     https://issues.apache.org/jira/browse/FALCON-1617
> 
> 
> Repository: falcon-git
> 
> 
> Description
> -------
> 
> Currently on enabling SLA monitoring it doesn't consider instances which had 
> nominal time in past for SLA monitoring. With this JIRA we would like to 
> enable this.
> 
> 
> Diffs
> -----
> 
>   common/src/main/java/org/apache/falcon/entity/FeedHelper.java 29daff3 
>   prism/src/main/java/org/apache/falcon/service/FeedSLAMonitoringService.java 
> b302539 
> 
> Diff: https://reviews.apache.org/r/41339/diff/
> 
> 
> Testing
> -------
> 
> Done.
> 
> 
> Thanks,
> 
> Narayan Periwal
> 
>

Reply via email to