You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@falcon.apache.org by PRAGYA MITTAL <mi...@gmail.com> on 2015/12/15 15:48:08 UTC
Review Request 41400: Add test for SLA monitoring API
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41400/
-----------------------------------------------------------
Review request for Falcon and Ajay Yadava.
Bugs: FALCON-1566
https://issues.apache.org/jira/browse/FALCON-1566
Repository: falcon-git
Description
-------
Add test for SLA monitoring API
Diffs
-----
falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/helpers/entity/AbstractEntityHelper.java e406cae
falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/response/ServiceResponse.java 55e862c
falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/util/Util.java ccd083b
falcon-regression/merlin/src/test/java/org/apache/falcon/regression/FeedSLATest.java 28ddbd7
falcon-regression/merlin/src/test/java/org/apache/falcon/regression/ProcessSLATest.java f4c9b30
falcon-regression/merlin/src/test/java/org/apache/falcon/regression/SLA/FeedSLAMonitoring.java PRE-CREATION
falcon-regression/merlin/src/test/java/org/apache/falcon/regression/SLA/FeedSLATest.java PRE-CREATION
falcon-regression/merlin/src/test/java/org/apache/falcon/regression/SLA/ProcessSLATest.java PRE-CREATION
Diff: https://reviews.apache.org/r/41400/diff/
Testing
-------
Tetsed in distributed mode.
Thanks,
PRAGYA MITTAL
Re: Review Request 41400: Add test for SLA monitoring API
Posted by pavan kumar kolamuri <pa...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41400/#review110622
-----------------------------------------------------------
falcon-regression/merlin/src/test/java/org/apache/falcon/regression/SLA/FeedSLAMonitoring.java (line 60)
<https://reviews.apache.org/r/41400/#comment170581>
Name it FeedSLAMonitoringTest
falcon-regression/merlin/src/test/java/org/apache/falcon/regression/SLA/FeedSLAMonitoring.java (line 80)
<https://reviews.apache.org/r/41400/#comment170586>
SchedulableEntityInstance has already compareTo method please use it.
falcon-regression/merlin/src/test/java/org/apache/falcon/regression/SLA/FeedSLAMonitoring.java (line 81)
<https://reviews.apache.org/r/41400/#comment170587>
Looks like tags is missing in compareTo you can add it over there if required
falcon-regression/merlin/src/test/java/org/apache/falcon/regression/SLA/FeedSLAMonitoring.java (line 107)
<https://reviews.apache.org/r/41400/#comment170599>
Can you add description to this method , what are trying to setup ?
falcon-regression/merlin/src/test/java/org/apache/falcon/regression/SLA/FeedSLAMonitoring.java (line 152)
<https://reviews.apache.org/r/41400/#comment170603>
is it possible to split these method into different methods
falcon-regression/merlin/src/test/java/org/apache/falcon/regression/SLA/FeedSLAMonitoring.java (line 153)
<https://reviews.apache.org/r/41400/#comment170606>
Please split into diffeernt tests if one test depends on other we can use dependson anotation
falcon-regression/merlin/src/test/java/org/apache/falcon/regression/SLA/FeedSLAMonitoring.java (line 165)
<https://reviews.apache.org/r/41400/#comment170608>
Make class variable noOfFeeds and use in for loop across places;
falcon-regression/merlin/src/test/java/org/apache/falcon/regression/SLA/FeedSLAMonitoring.java (line 184)
<https://reviews.apache.org/r/41400/#comment170611>
It should be 60 right ? otherwise waiting 10 minutes is so long
falcon-regression/merlin/src/test/java/org/apache/falcon/regression/SLA/FeedSLAMonitoring.java (line 186)
<https://reviews.apache.org/r/41400/#comment170613>
Can you move this method and make arguments startTime and endTime and return SchedulableEntityInstanceResult. Since it used across multiple places
falcon-regression/merlin/src/test/java/org/apache/falcon/regression/SLA/FeedSLAMonitoring.java (line 220)
<https://reviews.apache.org/r/41400/#comment170616>
Move this also to method
falcon-regression/merlin/src/test/java/org/apache/falcon/regression/SLA/FeedSLAMonitoring.java (line 231)
<https://reviews.apache.org/r/41400/#comment170617>
Correct me, if i am wrong you are testing basic scenario and delete right ? Can you also add case for update if possible.
- pavan kumar kolamuri
On Dec. 15, 2015, 2:48 p.m., PRAGYA MITTAL wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41400/
> -----------------------------------------------------------
>
> (Updated Dec. 15, 2015, 2:48 p.m.)
>
>
> Review request for Falcon and Ajay Yadava.
>
>
> Bugs: FALCON-1566
> https://issues.apache.org/jira/browse/FALCON-1566
>
>
> Repository: falcon-git
>
>
> Description
> -------
>
> Add test for SLA monitoring API
>
>
> Diffs
> -----
>
> falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/helpers/entity/AbstractEntityHelper.java e406cae
> falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/response/ServiceResponse.java 55e862c
> falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/util/Util.java ccd083b
> falcon-regression/merlin/src/test/java/org/apache/falcon/regression/FeedSLATest.java 28ddbd7
> falcon-regression/merlin/src/test/java/org/apache/falcon/regression/ProcessSLATest.java f4c9b30
> falcon-regression/merlin/src/test/java/org/apache/falcon/regression/SLA/FeedSLAMonitoring.java PRE-CREATION
> falcon-regression/merlin/src/test/java/org/apache/falcon/regression/SLA/FeedSLATest.java PRE-CREATION
> falcon-regression/merlin/src/test/java/org/apache/falcon/regression/SLA/ProcessSLATest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/41400/diff/
>
>
> Testing
> -------
>
> Tetsed in distributed mode.
>
>
> Thanks,
>
> PRAGYA MITTAL
>
>
Re: Review Request 41400: Add test for SLA monitoring API
Posted by Karishma Gulati <gu...@yahoo.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41400/#review111745
-----------------------------------------------------------
falcon-regression/merlin/src/test/java/org/apache/falcon/regression/SLA/FeedSLAMonitoring.java (line 76)
<https://reviews.apache.org/r/41400/#comment171986>
The same Comparator is present in PipelineInstanceDependencyTest. You should consider putting this in Utils, since multiple tests are anyway using this.
falcon-regression/merlin/src/test/java/org/apache/falcon/regression/SLA/FeedSLAMonitoring.java (line 147)
<https://reviews.apache.org/r/41400/#comment171987>
Use cleanTestsDirs(); (an existing function in BaseTestClass) instead of this.
falcon-regression/merlin/src/test/java/org/apache/falcon/regression/SLA/FeedSLATest.java (line 74)
<https://reviews.apache.org/r/41400/#comment171989>
Add cleanTestDirs() here.
falcon-regression/merlin/src/test/java/org/apache/falcon/regression/SLA/FeedSLATest.java (line 207)
<https://reviews.apache.org/r/41400/#comment171993>
Add this to Utils. It's a generic validation.
falcon-regression/merlin/src/test/java/org/apache/falcon/regression/SLA/ProcessSLATest.java (line 76)
<https://reviews.apache.org/r/41400/#comment171995>
Add cleanTestDirs() here
falcon-regression/merlin/src/test/java/org/apache/falcon/regression/SLA/ProcessSLATest.java (line 166)
<https://reviews.apache.org/r/41400/#comment171983>
This is a generic method. Add it to Utils.
- Karishma Gulati
On Dec. 15, 2015, 2:48 p.m., PRAGYA MITTAL wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41400/
> -----------------------------------------------------------
>
> (Updated Dec. 15, 2015, 2:48 p.m.)
>
>
> Review request for Falcon and Ajay Yadava.
>
>
> Bugs: FALCON-1566
> https://issues.apache.org/jira/browse/FALCON-1566
>
>
> Repository: falcon-git
>
>
> Description
> -------
>
> Add test for SLA monitoring API
>
>
> Diffs
> -----
>
> falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/helpers/entity/AbstractEntityHelper.java e406cae
> falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/response/ServiceResponse.java 55e862c
> falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/util/Util.java ccd083b
> falcon-regression/merlin/src/test/java/org/apache/falcon/regression/FeedSLATest.java 28ddbd7
> falcon-regression/merlin/src/test/java/org/apache/falcon/regression/ProcessSLATest.java f4c9b30
> falcon-regression/merlin/src/test/java/org/apache/falcon/regression/SLA/FeedSLAMonitoring.java PRE-CREATION
> falcon-regression/merlin/src/test/java/org/apache/falcon/regression/SLA/FeedSLATest.java PRE-CREATION
> falcon-regression/merlin/src/test/java/org/apache/falcon/regression/SLA/ProcessSLATest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/41400/diff/
>
>
> Testing
> -------
>
> Tetsed in distributed mode.
>
>
> Thanks,
>
> PRAGYA MITTAL
>
>
Re: Review Request 41400: Add test for SLA monitoring API
Posted by sandeep samudrala <sa...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41400/#review110624
-----------------------------------------------------------
falcon-regression/merlin/src/test/java/org/apache/falcon/regression/SLA/FeedSLAMonitoring.java (line 118)
<https://reviews.apache.org/r/41400/#comment170588>
oldInputFeedName? baseFeedName is more appropriate?
falcon-regression/merlin/src/test/java/org/apache/falcon/regression/SLA/FeedSLAMonitoring.java (line 119)
<https://reviews.apache.org/r/41400/#comment170593>
no Process here to denote it as input.
feedFrequencies.
falcon-regression/merlin/src/test/java/org/apache/falcon/regression/SLA/FeedSLAMonitoring.java (line 184)
<https://reviews.apache.org/r/41400/#comment170620>
10 minutes is too long, reduce it.
falcon-regression/merlin/src/test/java/org/apache/falcon/regression/SLA/FeedSLAMonitoring.java (line 203)
<https://reviews.apache.org/r/41400/#comment170624>
Can you add a case to check for instance of a feed where there is no _SUCCESS flag to check for sla upon directory creation?
falcon-regression/merlin/src/test/java/org/apache/falcon/regression/SLA/FeedSLAMonitoring.java (line 212)
<https://reviews.apache.org/r/41400/#comment170626>
Create separate methods for different tests,(deletion, creation, validation with and without _SUCCESS flag).
- sandeep samudrala
On Dec. 15, 2015, 2:48 p.m., PRAGYA MITTAL wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41400/
> -----------------------------------------------------------
>
> (Updated Dec. 15, 2015, 2:48 p.m.)
>
>
> Review request for Falcon and Ajay Yadava.
>
>
> Bugs: FALCON-1566
> https://issues.apache.org/jira/browse/FALCON-1566
>
>
> Repository: falcon-git
>
>
> Description
> -------
>
> Add test for SLA monitoring API
>
>
> Diffs
> -----
>
> falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/helpers/entity/AbstractEntityHelper.java e406cae
> falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/response/ServiceResponse.java 55e862c
> falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/util/Util.java ccd083b
> falcon-regression/merlin/src/test/java/org/apache/falcon/regression/FeedSLATest.java 28ddbd7
> falcon-regression/merlin/src/test/java/org/apache/falcon/regression/ProcessSLATest.java f4c9b30
> falcon-regression/merlin/src/test/java/org/apache/falcon/regression/SLA/FeedSLAMonitoring.java PRE-CREATION
> falcon-regression/merlin/src/test/java/org/apache/falcon/regression/SLA/FeedSLATest.java PRE-CREATION
> falcon-regression/merlin/src/test/java/org/apache/falcon/regression/SLA/ProcessSLATest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/41400/diff/
>
>
> Testing
> -------
>
> Tetsed in distributed mode.
>
>
> Thanks,
>
> PRAGYA MITTAL
>
>
Re: Review Request 41400: Add test for SLA monitoring API
Posted by Karishma Gulati <gu...@yahoo.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41400/#review111750
-----------------------------------------------------------
falcon-regression/merlin/src/test/java/org/apache/falcon/regression/SLA/FeedSLATest.java (line 85)
<https://reviews.apache.org/r/41400/#comment172004>
Clear feedClusters in before Method since you're doing it in all tests.
falcon-regression/merlin/src/test/java/org/apache/falcon/regression/SLA/FeedSLATest.java (line 108)
<https://reviews.apache.org/r/41400/#comment172005>
move to before Method
falcon-regression/merlin/src/test/java/org/apache/falcon/regression/SLA/FeedSLATest.java (line 138)
<https://reviews.apache.org/r/41400/#comment172006>
move to before Method
falcon-regression/merlin/src/test/java/org/apache/falcon/regression/SLA/FeedSLATest.java (line 164)
<https://reviews.apache.org/r/41400/#comment172007>
Move to before Method
falcon-regression/merlin/src/test/java/org/apache/falcon/regression/SLA/FeedSLATest.java (line 192)
<https://reviews.apache.org/r/41400/#comment172008>
move to before Method
falcon-regression/merlin/src/test/java/org/apache/falcon/regression/SLA/ProcessSLATest.java (line 86)
<https://reviews.apache.org/r/41400/#comment171998>
Move this line to before method. And declare processMerlin a global variable.
falcon-regression/merlin/src/test/java/org/apache/falcon/regression/SLA/ProcessSLATest.java (line 101)
<https://reviews.apache.org/r/41400/#comment171999>
Move to before Method
falcon-regression/merlin/src/test/java/org/apache/falcon/regression/SLA/ProcessSLATest.java (line 116)
<https://reviews.apache.org/r/41400/#comment172001>
Move to before Method.
falcon-regression/merlin/src/test/java/org/apache/falcon/regression/SLA/ProcessSLATest.java (line 137)
<https://reviews.apache.org/r/41400/#comment172002>
Move to before Method
falcon-regression/merlin/src/test/java/org/apache/falcon/regression/SLA/ProcessSLATest.java (line 153)
<https://reviews.apache.org/r/41400/#comment172003>
Move to before Method
- Karishma Gulati
On Dec. 15, 2015, 2:48 p.m., PRAGYA MITTAL wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41400/
> -----------------------------------------------------------
>
> (Updated Dec. 15, 2015, 2:48 p.m.)
>
>
> Review request for Falcon and Ajay Yadava.
>
>
> Bugs: FALCON-1566
> https://issues.apache.org/jira/browse/FALCON-1566
>
>
> Repository: falcon-git
>
>
> Description
> -------
>
> Add test for SLA monitoring API
>
>
> Diffs
> -----
>
> falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/helpers/entity/AbstractEntityHelper.java e406cae
> falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/response/ServiceResponse.java 55e862c
> falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/util/Util.java ccd083b
> falcon-regression/merlin/src/test/java/org/apache/falcon/regression/FeedSLATest.java 28ddbd7
> falcon-regression/merlin/src/test/java/org/apache/falcon/regression/ProcessSLATest.java f4c9b30
> falcon-regression/merlin/src/test/java/org/apache/falcon/regression/SLA/FeedSLAMonitoring.java PRE-CREATION
> falcon-regression/merlin/src/test/java/org/apache/falcon/regression/SLA/FeedSLATest.java PRE-CREATION
> falcon-regression/merlin/src/test/java/org/apache/falcon/regression/SLA/ProcessSLATest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/41400/diff/
>
>
> Testing
> -------
>
> Tetsed in distributed mode.
>
>
> Thanks,
>
> PRAGYA MITTAL
>
>