falcon-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Raghav Gautam" <rag...@apache.org>
Subject Re: Review Request 32814: Migrate methods related to *Merlin.java classes from InstanceUtil.java and Bundle.java
Date Wed, 08 Apr 2015 22:51:47 GMT

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



falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/Entities/ClusterMerlin.java
<https://reviews.apache.org/r/32814/#comment128805>

    space after :



falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/Entities/ClusterMerlin.java
<https://reviews.apache.org/r/32814/#comment128804>

    you can use == instead of .equals for comparison of enum.



falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/Entities/FeedMerlin.java
<https://reviews.apache.org/r/32814/#comment128807>

    Is it ok to assume that the path that you want to set will always be the first element
in the list ?



falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/Entities/FeedMerlin.java
<https://reviews.apache.org/r/32814/#comment128809>

    It seems that we have assumend that the element of interest will be the first element
in the list. I see that this is the case at many places. Is there an easy way to get rid of
this assumption. If not can you plase double check if we are doing this correctly.
    
    The problem that concerns me is that if there is a test bug, it will be intermitent reproducible.



falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/util/BundleUtil.java
<https://reviews.apache.org/r/32814/#comment128813>

    The method should probably move to ClusterMerlin.java.



falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/util/Config.java
<https://reviews.apache.org/r/32814/#comment128818>

    Probably this change was not intentional.



falcon-regression/merlin/src/test/java/org/apache/falcon/regression/NewRetryTest.java
<https://reviews.apache.org/r/32814/#comment128825>

    Consider using TimeUtil.dateToOozieDate().



falcon-regression/merlin/src/test/java/org/apache/falcon/regression/ProcessLateRerunTest.java
<https://reviews.apache.org/r/32814/#comment128826>

    We have repeated the same code thrice here and a few more time at other places.
    String inputName = bundles[0].getProcessObject().getInputs().getInputs().get(0).getName();
    
    Consider adding a getOnlyInputName()/getFirstInputName() method to ProcessMerlin.
    
    If you go with getOnlyInputName() - you might want to add an assertion that there is only
one input feed for the process.



falcon-regression/merlin/src/test/java/org/apache/falcon/regression/ui/ProcessUITest.java
<https://reviews.apache.org/r/32814/#comment128829>

    increment of j has been removed - this will create an issue as all the feeds will have
same name!


- Raghav Gautam


On April 8, 2015, 8:40 a.m., Ruslan Ostafiychuk wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32814/
> -----------------------------------------------------------
> 
> (Updated April 8, 2015, 8:40 a.m.)
> 
> 
> Review request for Falcon.
> 
> 
> Bugs: Falcon-1135
>     https://issues.apache.org/jira/browse/Falcon-1135
> 
> 
> Repository: falcon-git
> 
> 
> Description
> -------
> 
> Migrate methods related to *Merlin.java classes from InstanceUtil.java and Bundle.java
> 
> 
> Diffs
> -----
> 
>   falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/Entities/ClusterMerlin.java
22ec5da 
>   falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/Entities/FeedMerlin.java
70e2e73 
>   falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/Entities/ProcessMerlin.java
01fdd04 
>   falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/bundle/Bundle.java
b0fa0a5 
>   falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/util/BundleUtil.java
0b2c4e1 
>   falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/util/Config.java
ba32d11 
>   falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/util/InstanceUtil.java
4620787 
>   falcon-regression/merlin/src/test/java/org/apache/falcon/regression/AuthorizationTest.java
eaa69f0 
>   falcon-regression/merlin/src/test/java/org/apache/falcon/regression/ELExpCurrentAndLastWeekTest.java
b7eb77f 
>   falcon-regression/merlin/src/test/java/org/apache/falcon/regression/ELValidationsTest.java
41e3002 
>   falcon-regression/merlin/src/test/java/org/apache/falcon/regression/ExternalFSTest.java
6b227d6 
>   falcon-regression/merlin/src/test/java/org/apache/falcon/regression/FeedInstanceListingTest.java
a6639ed 
>   falcon-regression/merlin/src/test/java/org/apache/falcon/regression/FeedLateRerunTest.java
2c8346d 
>   falcon-regression/merlin/src/test/java/org/apache/falcon/regression/FeedReplicationTest.java
eb8c4fe 
>   falcon-regression/merlin/src/test/java/org/apache/falcon/regression/NewRetryTest.java
13a9776 
>   falcon-regression/merlin/src/test/java/org/apache/falcon/regression/NoOutputProcessTest.java
59a701d 
>   falcon-regression/merlin/src/test/java/org/apache/falcon/regression/ProcessFrequencyTest.java
8cf2862 
>   falcon-regression/merlin/src/test/java/org/apache/falcon/regression/ProcessInstanceColoMixedTest.java
48cb59b 
>   falcon-regression/merlin/src/test/java/org/apache/falcon/regression/ProcessInstanceKillsTest.java
34dfce3 
>   falcon-regression/merlin/src/test/java/org/apache/falcon/regression/ProcessInstanceResumeTest.java
f558cc5 
>   falcon-regression/merlin/src/test/java/org/apache/falcon/regression/ProcessInstanceRunningTest.java
c9334eb 
>   falcon-regression/merlin/src/test/java/org/apache/falcon/regression/ProcessInstanceStatusTest.java
58936a7 
>   falcon-regression/merlin/src/test/java/org/apache/falcon/regression/ProcessInstanceSuspendTest.java
26348bd 
>   falcon-regression/merlin/src/test/java/org/apache/falcon/regression/ProcessLateRerunTest.java
b41cf05 
>   falcon-regression/merlin/src/test/java/org/apache/falcon/regression/ProcessSLATest.java
cd7eba4 
>   falcon-regression/merlin/src/test/java/org/apache/falcon/regression/ValidateAPIPrismAndServerTest.java
9886d76 
>   falcon-regression/merlin/src/test/java/org/apache/falcon/regression/entity/EntitiesPatternSearchTest.java
f9fcf8d 
>   falcon-regression/merlin/src/test/java/org/apache/falcon/regression/entity/ListEntitiesTest.java
13b3b88 
>   falcon-regression/merlin/src/test/java/org/apache/falcon/regression/hcat/HCatProcessTest.java
202298e 
>   falcon-regression/merlin/src/test/java/org/apache/falcon/regression/prism/NewPrismProcessUpdateTest.java
4466c13 
>   falcon-regression/merlin/src/test/java/org/apache/falcon/regression/prism/OptionalInputTest.java
c9e373e 
>   falcon-regression/merlin/src/test/java/org/apache/falcon/regression/prism/PrismFeedReplicationPartitionExpTest.java
97d4e67 
>   falcon-regression/merlin/src/test/java/org/apache/falcon/regression/prism/PrismFeedReplicationUpdateTest.java
e1a96f3 
>   falcon-regression/merlin/src/test/java/org/apache/falcon/regression/prism/PrismFeedUpdateTest.java
483c281 
>   falcon-regression/merlin/src/test/java/org/apache/falcon/regression/prism/PrismProcessDeleteTest.java
f1ff8fe 
>   falcon-regression/merlin/src/test/java/org/apache/falcon/regression/prism/PrismProcessScheduleTest.java
03f380d 
>   falcon-regression/merlin/src/test/java/org/apache/falcon/regression/prism/PrismProcessSnSTest.java
dfb405f 
>   falcon-regression/merlin/src/test/java/org/apache/falcon/regression/prism/PrismSubmitTest.java
7bc4b5b 
>   falcon-regression/merlin/src/test/java/org/apache/falcon/regression/prism/ProcessPartitionExpVariableTest.java
272ac3b 
>   falcon-regression/merlin/src/test/java/org/apache/falcon/regression/prism/RescheduleKilledProcessTest.java
1d65d12 
>   falcon-regression/merlin/src/test/java/org/apache/falcon/regression/prism/RescheduleProcessInFinalStatesTest.java
7e4422b 
>   falcon-regression/merlin/src/test/java/org/apache/falcon/regression/prism/UpdateAtSpecificTimeTest.java
0cc0d6e 
>   falcon-regression/merlin/src/test/java/org/apache/falcon/regression/ui/ProcessUITest.java
4c54409 
> 
> Diff: https://reviews.apache.org/r/32814/diff/
> 
> 
> Testing
> -------
> 
> tested
> 
> 
> Thanks,
> 
> Ruslan Ostafiychuk
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message