You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@falcon.apache.org by Narayan Periwal <na...@inmobi.com> on 2015/10/28 02:56:04 UTC
Re: Review Request 39657: FALCON-1528: Instance Management Api in
Falcon Unit
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39657/
-----------------------------------------------------------
(Updated Oct. 28, 2015, 1:56 a.m.)
Review request for Falcon.
Summary (updated)
-----------------
FALCON-1528: Instance Management Api in Falcon Unit
Bugs: FALCON-1528
https://issues.apache.org/jira/browse/FALCON-1528
Repository: falcon-git
Description
-------
Instance Management Api's should be supported in Falcon Unit.
Diffs (updated)
-----
client/src/main/java/org/apache/falcon/client/AbstractFalconClient.java 91d5324
client/src/main/java/org/apache/falcon/client/FalconClient.java 27510f6
oozie/src/main/java/org/apache/oozie/client/LocalOozieClientCoordProxy.java PRE-CREATION
oozie/src/main/java/org/apache/oozie/client/LocalProxyOozieClient.java c2100d1
prism/src/main/java/org/apache/falcon/resource/AbstractInstanceManager.java 606f741
unit/src/main/java/org/apache/falcon/unit/FalconUnitClient.java b5afae3
unit/src/main/java/org/apache/falcon/unit/LocalInstanceManager.java 1503b28
unit/src/test/java/org/apache/falcon/unit/FalconUnitTestBase.java ac478f4
unit/src/test/java/org/apache/falcon/unit/TestFalconUnit.java 8cdbd88
unit/src/test/java/org/apache/falcon/unit/examples/JavaSleepExample.java PRE-CREATION
unit/src/test/resources/sleepWorkflow.xml PRE-CREATION
Diff: https://reviews.apache.org/r/39657/diff/
Testing
-------
Done.
Thanks,
Narayan Periwal
Re: Review Request 39657: FALCON-1528: Instance Management Api in
Falcon Unit
Posted by pavan kumar kolamuri <pa...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39657/#review104257
-----------------------------------------------------------
Ship it!
Ship It!
- pavan kumar kolamuri
On Oct. 28, 2015, 1:56 a.m., Narayan Periwal wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39657/
> -----------------------------------------------------------
>
> (Updated Oct. 28, 2015, 1:56 a.m.)
>
>
> Review request for Falcon.
>
>
> Bugs: FALCON-1528
> https://issues.apache.org/jira/browse/FALCON-1528
>
>
> Repository: falcon-git
>
>
> Description
> -------
>
> Instance Management Api's should be supported in Falcon Unit.
>
>
> Diffs
> -----
>
> client/src/main/java/org/apache/falcon/client/AbstractFalconClient.java 91d5324
> client/src/main/java/org/apache/falcon/client/FalconClient.java 27510f6
> oozie/src/main/java/org/apache/oozie/client/LocalOozieClientCoordProxy.java PRE-CREATION
> oozie/src/main/java/org/apache/oozie/client/LocalProxyOozieClient.java c2100d1
> prism/src/main/java/org/apache/falcon/resource/AbstractInstanceManager.java 606f741
> unit/src/main/java/org/apache/falcon/unit/FalconUnitClient.java b5afae3
> unit/src/main/java/org/apache/falcon/unit/LocalInstanceManager.java 1503b28
> unit/src/test/java/org/apache/falcon/unit/FalconUnitTestBase.java ac478f4
> unit/src/test/java/org/apache/falcon/unit/TestFalconUnit.java 8cdbd88
> unit/src/test/java/org/apache/falcon/unit/examples/JavaSleepExample.java PRE-CREATION
> unit/src/test/resources/sleepWorkflow.xml PRE-CREATION
>
> Diff: https://reviews.apache.org/r/39657/diff/
>
>
> Testing
> -------
>
> Done.
>
>
> Thanks,
>
> Narayan Periwal
>
>
Re: Review Request 39657: FALCON-1528: Instance Management Api in
Falcon Unit
Posted by Narayan Periwal <na...@inmobi.com>.
> On Oct. 29, 2015, 10:48 a.m., Pallavi Rao wrote:
> > unit/src/main/java/org/apache/falcon/unit/FalconUnitClient.java, line 250
> > <https://reviews.apache.org/r/39657/diff/3/?file=1110427#file1110427line250>
> >
> > For this and some other methods, I don't understand why we are creating an input stream and then extracting properties from the stream. Rather than, just creating the properties directly.
Agreed. Will create the properties directly.
> On Oct. 29, 2015, 10:48 a.m., Pallavi Rao wrote:
> > unit/src/test/java/org/apache/falcon/unit/TestFalconUnit.java, line 238
> > <https://reviews.apache.org/r/39657/diff/3/?file=1110430#file1110430line238>
> >
> > The tests will be more modular and readable if we can have one test per API. I understand there is an overhead of setup for every test. If the overhead is high, you can even have a separate test class and have the setup in @BeforeClass.
Yes, combining all the tests make it difficult to read and understand. I have broken the test into 3 different tests.
> On Oct. 29, 2015, 10:48 a.m., Pallavi Rao wrote:
> > unit/src/test/java/org/apache/falcon/unit/examples/JavaSleepExample.java, line 29
> > <https://reviews.apache.org/r/39657/diff/3/?file=1110431#file1110431line29>
> >
> > Isn't 40s a little high for UTs? This could even be a Dummy Java action, which just does a sys out and exits. Really need a sleep here?
We need a sleep here, so that we can get the running instance for testing the suspend functionality. We dont wait for the process to complete. So, 40 is not a overhead.
- Narayan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39657/#review104413
-----------------------------------------------------------
On Oct. 28, 2015, 1:56 a.m., Narayan Periwal wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39657/
> -----------------------------------------------------------
>
> (Updated Oct. 28, 2015, 1:56 a.m.)
>
>
> Review request for Falcon.
>
>
> Bugs: FALCON-1528
> https://issues.apache.org/jira/browse/FALCON-1528
>
>
> Repository: falcon-git
>
>
> Description
> -------
>
> Instance Management Api's should be supported in Falcon Unit.
>
>
> Diffs
> -----
>
> client/src/main/java/org/apache/falcon/client/AbstractFalconClient.java 91d5324
> client/src/main/java/org/apache/falcon/client/FalconClient.java 27510f6
> oozie/src/main/java/org/apache/oozie/client/LocalOozieClientCoordProxy.java PRE-CREATION
> oozie/src/main/java/org/apache/oozie/client/LocalProxyOozieClient.java c2100d1
> prism/src/main/java/org/apache/falcon/resource/AbstractInstanceManager.java 606f741
> unit/src/main/java/org/apache/falcon/unit/FalconUnitClient.java b5afae3
> unit/src/main/java/org/apache/falcon/unit/LocalInstanceManager.java 1503b28
> unit/src/test/java/org/apache/falcon/unit/FalconUnitTestBase.java ac478f4
> unit/src/test/java/org/apache/falcon/unit/TestFalconUnit.java 8cdbd88
> unit/src/test/java/org/apache/falcon/unit/examples/JavaSleepExample.java PRE-CREATION
> unit/src/test/resources/sleepWorkflow.xml PRE-CREATION
>
> Diff: https://reviews.apache.org/r/39657/diff/
>
>
> Testing
> -------
>
> Done.
>
>
> Thanks,
>
> Narayan Periwal
>
>
Re: Review Request 39657: FALCON-1528: Instance Management Api in
Falcon Unit
Posted by Pallavi Rao <pa...@inmobi.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39657/#review104413
-----------------------------------------------------------
unit/src/main/java/org/apache/falcon/unit/FalconUnitClient.java (line 249)
<https://reviews.apache.org/r/39657/#comment162682>
For this and some other methods, I don't understand why we are creating an input stream and then extracting properties from the stream. Rather than, just creating the properties directly.
unit/src/test/java/org/apache/falcon/unit/TestFalconUnit.java (line 237)
<https://reviews.apache.org/r/39657/#comment162681>
The tests will be more modular and readable if we can have one test per API. I understand there is an overhead of setup for every test. If the overhead is high, you can even have a separate test class and have the setup in @BeforeClass.
unit/src/test/java/org/apache/falcon/unit/examples/JavaSleepExample.java (line 29)
<https://reviews.apache.org/r/39657/#comment162683>
Isn't 40s a little high for UTs? This could even be a Dummy Java action, which just does a sys out and exits. Really need a sleep here?
- Pallavi Rao
On Oct. 28, 2015, 1:56 a.m., Narayan Periwal wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39657/
> -----------------------------------------------------------
>
> (Updated Oct. 28, 2015, 1:56 a.m.)
>
>
> Review request for Falcon.
>
>
> Bugs: FALCON-1528
> https://issues.apache.org/jira/browse/FALCON-1528
>
>
> Repository: falcon-git
>
>
> Description
> -------
>
> Instance Management Api's should be supported in Falcon Unit.
>
>
> Diffs
> -----
>
> client/src/main/java/org/apache/falcon/client/AbstractFalconClient.java 91d5324
> client/src/main/java/org/apache/falcon/client/FalconClient.java 27510f6
> oozie/src/main/java/org/apache/oozie/client/LocalOozieClientCoordProxy.java PRE-CREATION
> oozie/src/main/java/org/apache/oozie/client/LocalProxyOozieClient.java c2100d1
> prism/src/main/java/org/apache/falcon/resource/AbstractInstanceManager.java 606f741
> unit/src/main/java/org/apache/falcon/unit/FalconUnitClient.java b5afae3
> unit/src/main/java/org/apache/falcon/unit/LocalInstanceManager.java 1503b28
> unit/src/test/java/org/apache/falcon/unit/FalconUnitTestBase.java ac478f4
> unit/src/test/java/org/apache/falcon/unit/TestFalconUnit.java 8cdbd88
> unit/src/test/java/org/apache/falcon/unit/examples/JavaSleepExample.java PRE-CREATION
> unit/src/test/resources/sleepWorkflow.xml PRE-CREATION
>
> Diff: https://reviews.apache.org/r/39657/diff/
>
>
> Testing
> -------
>
> Done.
>
>
> Thanks,
>
> Narayan Periwal
>
>
Re: Review Request 39657: FALCON-1528: Instance Management Api in
Falcon Unit
Posted by Narayan Periwal <na...@inmobi.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39657/
-----------------------------------------------------------
(Updated Oct. 30, 2015, 5:10 a.m.)
Review request for Falcon.
Bugs: FALCON-1528
https://issues.apache.org/jira/browse/FALCON-1528
Repository: falcon-git
Description
-------
Instance Management Api's should be supported in Falcon Unit.
Diffs (updated)
-----
client/src/main/java/org/apache/falcon/client/AbstractFalconClient.java 91d5324
client/src/main/java/org/apache/falcon/client/FalconClient.java 27510f6
oozie/src/main/java/org/apache/oozie/client/LocalOozieClientCoordProxy.java PRE-CREATION
oozie/src/main/java/org/apache/oozie/client/LocalProxyOozieClient.java c2100d1
prism/src/main/java/org/apache/falcon/resource/AbstractInstanceManager.java 606f741
unit/src/main/java/org/apache/falcon/unit/FalconUnitClient.java b5afae3
unit/src/main/java/org/apache/falcon/unit/LocalInstanceManager.java 1503b28
unit/src/test/java/org/apache/falcon/unit/FalconUnitTestBase.java ac478f4
unit/src/test/java/org/apache/falcon/unit/TestFalconUnit.java 8cdbd88
unit/src/test/java/org/apache/falcon/unit/examples/JavaSleepExample.java PRE-CREATION
unit/src/test/resources/sleepWorkflow.xml PRE-CREATION
Diff: https://reviews.apache.org/r/39657/diff/
Testing
-------
Done.
Thanks,
Narayan Periwal
Re: Review Request 39657: FALCON-1528: Instance Management Api in
Falcon Unit
Posted by Pallavi Rao <pa...@inmobi.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39657/#review104525
-----------------------------------------------------------
Ship it!
Assuming you will address the minor comment.
unit/src/main/java/org/apache/falcon/unit/FalconUnitClient.java (line 376)
<https://reviews.apache.org/r/39657/#comment162769>
Better to use StringUtils.isNotEmpty()?
unit/src/main/java/org/apache/falcon/unit/FalconUnitClient.java (line 379)
<https://reviews.apache.org/r/39657/#comment162770>
Better to use StringUtils.isNotEmpty()?
- Pallavi Rao
On Oct. 29, 2015, 4:46 p.m., Narayan Periwal wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39657/
> -----------------------------------------------------------
>
> (Updated Oct. 29, 2015, 4:46 p.m.)
>
>
> Review request for Falcon.
>
>
> Bugs: FALCON-1528
> https://issues.apache.org/jira/browse/FALCON-1528
>
>
> Repository: falcon-git
>
>
> Description
> -------
>
> Instance Management Api's should be supported in Falcon Unit.
>
>
> Diffs
> -----
>
> client/src/main/java/org/apache/falcon/client/AbstractFalconClient.java 91d5324
> client/src/main/java/org/apache/falcon/client/FalconClient.java 27510f6
> oozie/src/main/java/org/apache/oozie/client/LocalOozieClientCoordProxy.java PRE-CREATION
> oozie/src/main/java/org/apache/oozie/client/LocalProxyOozieClient.java c2100d1
> prism/src/main/java/org/apache/falcon/resource/AbstractInstanceManager.java 606f741
> unit/src/main/java/org/apache/falcon/unit/FalconUnitClient.java b5afae3
> unit/src/main/java/org/apache/falcon/unit/LocalInstanceManager.java 1503b28
> unit/src/test/java/org/apache/falcon/unit/FalconUnitTestBase.java ac478f4
> unit/src/test/java/org/apache/falcon/unit/TestFalconUnit.java 8cdbd88
> unit/src/test/java/org/apache/falcon/unit/examples/JavaSleepExample.java PRE-CREATION
> unit/src/test/resources/sleepWorkflow.xml PRE-CREATION
>
> Diff: https://reviews.apache.org/r/39657/diff/
>
>
> Testing
> -------
>
> Done.
>
>
> Thanks,
>
> Narayan Periwal
>
>
Re: Review Request 39657: FALCON-1528: Instance Management Api in
Falcon Unit
Posted by Narayan Periwal <na...@inmobi.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39657/
-----------------------------------------------------------
(Updated Oct. 29, 2015, 4:46 p.m.)
Review request for Falcon.
Bugs: FALCON-1528
https://issues.apache.org/jira/browse/FALCON-1528
Repository: falcon-git
Description
-------
Instance Management Api's should be supported in Falcon Unit.
Diffs (updated)
-----
client/src/main/java/org/apache/falcon/client/AbstractFalconClient.java 91d5324
client/src/main/java/org/apache/falcon/client/FalconClient.java 27510f6
oozie/src/main/java/org/apache/oozie/client/LocalOozieClientCoordProxy.java PRE-CREATION
oozie/src/main/java/org/apache/oozie/client/LocalProxyOozieClient.java c2100d1
prism/src/main/java/org/apache/falcon/resource/AbstractInstanceManager.java 606f741
unit/src/main/java/org/apache/falcon/unit/FalconUnitClient.java b5afae3
unit/src/main/java/org/apache/falcon/unit/LocalInstanceManager.java 1503b28
unit/src/test/java/org/apache/falcon/unit/FalconUnitTestBase.java ac478f4
unit/src/test/java/org/apache/falcon/unit/TestFalconUnit.java 8cdbd88
unit/src/test/java/org/apache/falcon/unit/examples/JavaSleepExample.java PRE-CREATION
unit/src/test/resources/sleepWorkflow.xml PRE-CREATION
Diff: https://reviews.apache.org/r/39657/diff/
Testing
-------
Done.
Thanks,
Narayan Periwal