falcon-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Murali Ramasami" <murali.m...@gmail.com>
Subject Re: Review Request 41749: [FALCON-1701] HiveDr, ClusterSetupTest, MirrorSummaryTest fixes provided by Murali Ramasami
Date Mon, 04 Jan 2016 15:30:48 GMT


> On Dec. 29, 2015, 7:23 p.m., Ajay Yadava wrote:
> > falcon-regression/merlin/src/main/java/org/apache/falcon/regression/ui/search/ClusterWizardPage.java,
line 278
> > <https://reviews.apache.org/r/41749/diff/1/?file=1176963#file1176963line278>
> >
> >     nit: I think you meant "Assertion for Working Location"

Yes. It is "Assertion for Working Location"


> On Dec. 29, 2015, 7:23 p.m., Ajay Yadava wrote:
> > falcon-regression/merlin/src/main/java/org/apache/falcon/regression/ui/search/ClusterWizardPage.java,
line 284
> > <https://reviews.apache.org/r/41749/diff/1/?file=1176963#file1176963line284>
> >
> >     The purpose of the function seems to validate the error message for the element.
If that is the case then will some other name like "checkErrorMessageByElement" be more descriptive?

Thanks Ajay. Make sense. Will change it to "checkErrorMessageByElement"


> On Dec. 29, 2015, 7:23 p.m., Ajay Yadava wrote:
> > falcon-regression/merlin/src/main/java/org/apache/falcon/regression/ui/search/ClusterWizardPage.java,
line 317
> > <https://reviews.apache.org/r/41749/diff/1/?file=1176963#file1176963line317>
> >
> >     nit: remove the commented code please.

Removed the Commented Code.


> On Dec. 29, 2015, 7:23 p.m., Ajay Yadava wrote:
> > falcon-regression/merlin/src/main/java/org/apache/falcon/regression/ui/search/ClusterWizardPage.java,
line 398
> > <https://reviews.apache.org/r/41749/diff/1/?file=1176963#file1176963line398>
> >
> >     nit: remove dead code please.

Removed the Commented Code.


> On Dec. 29, 2015, 7:23 p.m., Ajay Yadava wrote:
> > falcon-regression/merlin/src/test/java/org/apache/falcon/regression/searchUI/ClusterSetupTest.java,
line 269
> > <https://reviews.apache.org/r/41749/diff/1/?file=1176965#file1176965line269>
> >
> >     The description is very usefual but the BUG ID looks like reference to a private
or internal issue tracking system. Can you please refer Apache JIRA instead or edit the comment?

Removed the internal BUG ID from the patch.


> On Dec. 29, 2015, 7:23 p.m., Ajay Yadava wrote:
> > falcon-regression/merlin/src/test/java/org/apache/falcon/regression/searchUI/ClusterSetupTest.java,
line 288
> > <https://reviews.apache.org/r/41749/diff/1/?file=1176965#file1176965line288>
> >
> >     The description is very usefual but the BUG ID looks like reference to a private
or internal issue tracking system. Can you please refer Apache JIRA instead or edit the comment?

Removed the internal BUG ID from the patch.


> On Dec. 29, 2015, 7:23 p.m., Ajay Yadava wrote:
> > falcon-regression/merlin/src/test/resources/HdfsRecipe/hive-disaster-recovery-template.xml,
line 42
> > <https://reviews.apache.org/r/41749/diff/1/?file=1176967#file1176967line42>
> >
> >     Is the notification functionality tested somewhere in these tests or is it added
for future use cases?

As part of the email notification feauture, we have added that for future use case and yet
to be tested.


> On Dec. 29, 2015, 7:23 p.m., Ajay Yadava wrote:
> > falcon-regression/merlin/src/test/java/org/apache/falcon/regression/hcat/HCatFeedOperationsTest.java,
line 160
> > <https://reviews.apache.org/r/41749/diff/1/?file=1176964#file1176964line160>
> >
> >     Why is this test being disabled?

Dev commented that "This test is invalid - If the table does not exist on target cluster there
will be a failure."


- Murali


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


On Dec. 28, 2015, 7:37 p.m., Paul Isaychuk wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41749/
> -----------------------------------------------------------
> 
> (Updated Dec. 28, 2015, 7:37 p.m.)
> 
> 
> Review request for Falcon.
> 
> 
> Bugs: FALCON-1701
>     https://issues.apache.org/jira/browse/FALCON-1701
> 
> 
> Repository: falcon-git
> 
> 
> Description
> -------
> 
> HiveDr, ClusterSetupTest, MirrorSummaryTest fixes provided by Murali Ramasami
> 
> 
> Diffs
> -----
> 
>   falcon-regression/merlin/src/main/java/org/apache/falcon/regression/ui/search/ClusterWizardPage.java
bcada4a 
>   falcon-regression/merlin/src/test/java/org/apache/falcon/regression/hcat/HCatFeedOperationsTest.java
27417bd 
>   falcon-regression/merlin/src/test/java/org/apache/falcon/regression/searchUI/ClusterSetupTest.java
5efa5b2 
>   falcon-regression/merlin/src/test/java/org/apache/falcon/regression/searchUI/MirrorSummaryTest.java
989e4b3 
>   falcon-regression/merlin/src/test/resources/HdfsRecipe/hive-disaster-recovery-template.xml
c644b99 
>   falcon-regression/merlin/src/test/resources/HdfsRecipe/hive-disaster-recovery-workflow.xml
aa4d5b0 
>   falcon-regression/merlin/src/test/resources/HdfsRecipe/hive-disaster-recovery.properties
99f748d 
>   falcon-regression/merlin/src/test/resources/HiveDrRecipe/hive-disaster-recovery-template.xml
3afbef0 
>   falcon-regression/merlin/src/test/resources/HiveDrRecipe/hive-disaster-recovery-workflow.xml
c441998 
>   falcon-regression/merlin/src/test/resources/HiveDrRecipe/hive-disaster-recovery.properties
de7f7f9 
>   falcon-regression/merlin/src/test/resources/HiveDrSecureRecipe/hive-disaster-recovery-secure-template.xml
3afbef0 
>   falcon-regression/merlin/src/test/resources/HiveDrSecureRecipe/hive-disaster-recovery-secure-workflow.xml
7362c2e 
>   falcon-regression/merlin/src/test/resources/HiveDrSecureRecipe/hive-disaster-recovery-secure.properties
ff2611f 
> 
> Diff: https://reviews.apache.org/r/41749/diff/
> 
> 
> Testing
> -------
> 
> tested on nightly runs
> 
> 
> Thanks,
> 
> Paul Isaychuk
> 
>


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