> On Sept. 29, 2015, 7:29 a.m., sandeep samudrala wrote:
> > client/src/main/java/org/apache/falcon/cli/FalconCLI.java, line 724
> > <https://reviews.apache.org/r/38794/diff/3/?file=1086487#file1086487line724>
> >
> >     Does it makes sense to make it generic to get entity type and log "Get 
> > missing {} ..", entityType to support for process to later on?

This is just a help message, we can update it when we add support for process, 
it won't be a backward incompatible change.


> On Sept. 29, 2015, 7:29 a.m., sandeep samudrala wrote:
> > client/src/main/java/org/apache/falcon/client/FalconClient.java, line 200
> > <https://reviews.apache.org/r/38794/diff/3/?file=1086488#file1086488line200>
> >
> >     slaAlert ? NIT. not sure.

Urls should not contain camel case, it's a bad practice, though we have it at 
some places.


> On Sept. 29, 2015, 7:29 a.m., sandeep samudrala wrote:
> > docs/src/site/twiki/FalconCLI.twiki, line 174
> > <https://reviews.apache.org/r/38794/diff/3/?file=1086495#file1086495line174>
> >
> >     Not related to here. How are the high,Low being sent to user? It would 
> > be good to let user give sla Levels for a given entity as the user better 
> > understands the sla Level. 
> >     
> >     If its already from the user definition ignore this comment.
> >     
> >     If its not, we should plan for it.

Yes, users can specify the sla through the sla tag in entity definition.


> On Sept. 29, 2015, 7:29 a.m., sandeep samudrala wrote:
> > prism/src/main/java/org/apache/falcon/service/FeedSLAMonitoringService.java,
> >  line 439
> > <https://reviews.apache.org/r/38794/diff/3/?file=1086501#file1086501line439>
> >
> >     The code is repeated in getFeedSLAMissPendingAlerts.
> >     Can this getFeedSLAMissPendingAlerts call 
> > getFeedSLAMissPendingAlerts(feedName) for each feed?. It will be make code 
> > readible.

The common code is already extracted out in a method called getSLAStatus.


> On Sept. 29, 2015, 7:29 a.m., sandeep samudrala wrote:
> > client/src/main/java/org/apache/falcon/entity/v0/SchemaHelper.java, line 57
> > <https://reviews.apache.org/r/38794/diff/3/?file=1086489#file1086489line57>
> >
> >     space after '+'

Will fix while committing. I want to avoid another cycle of preparing and 
testing patch, just for one space.


- Ajay


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


On Sept. 29, 2015, 6:10 a.m., Ajay Yadava wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38794/
> -----------------------------------------------------------
> 
> (Updated Sept. 29, 2015, 6:10 a.m.)
> 
> 
> Review request for Falcon.
> 
> 
> Bugs: FALCON-1473
>     https://issues.apache.org/jira/browse/FALCON-1473
> 
> 
> Repository: falcon-git
> 
> 
> Description
> -------
> 
> This api lists all the pending feed instances in a given time range which 
> have missed sla.
> 
> 
> Diffs
> -----
> 
>   client/src/main/java/org/apache/falcon/ResponseHelper.java a13682b 
>   client/src/main/java/org/apache/falcon/cli/FalconCLI.java c914649 
>   client/src/main/java/org/apache/falcon/client/FalconClient.java 981559b 
>   client/src/main/java/org/apache/falcon/entity/v0/SchemaHelper.java 62b810c 
>   
> client/src/main/java/org/apache/falcon/resource/SchedulableEntityInstance.java
>  f5be63d 
>   
> client/src/main/java/org/apache/falcon/resource/SchedulableEntityInstanceResult.java
>  PRE-CREATION 
>   common/src/main/java/org/apache/falcon/entity/FeedHelper.java 572923b 
>   common/src/main/java/org/apache/falcon/entity/parser/FeedEntityParser.java 
> 4f5599e 
>   common/src/main/resources/startup.properties 9db460c 
>   docs/src/site/twiki/FalconCLI.twiki 4f72bf8 
>   docs/src/site/twiki/restapi/FeedSLA.twiki PRE-CREATION 
>   docs/src/site/twiki/restapi/ResourceList.twiki ea3e3b6 
>   prism/src/main/java/org/apache/falcon/resource/AbstractEntityManager.java 
> 2682257 
>   
> prism/src/main/java/org/apache/falcon/resource/AbstractSchedulableEntityManager.java
>  3280789 
>   
> prism/src/main/java/org/apache/falcon/resource/proxy/SchedulableEntityManagerProxy.java
>  47038e5 
>   prism/src/main/java/org/apache/falcon/service/FeedSLAMonitoringService.java 
> 8bf43b8 
>   prism/src/test/java/org/apache/falcon/service/FeedSLAMonitoringTest.java 
> PRE-CREATION 
>   src/conf/startup.properties 8f3bc35 
>   
> webapp/src/main/java/org/apache/falcon/resource/SchedulableEntityManager.java 
> 1c0fc74 
> 
> Diff: https://reviews.apache.org/r/38794/diff/
> 
> 
> Testing
> -------
> 
> Unit tests added.
> Tested manually by deploying.
> 
> 
> Thanks,
> 
> Ajay Yadava
> 
>

Reply via email to