You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@bigtop.apache.org by Hari Shreedharan <hs...@cloudera.com> on 2013/01/30 21:35:36 UTC
Review Request: BIGTOP-835. The shell exec method must have variants which
have timeout and can run in background
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9166/
-----------------------------------------------------------
Review request for bigtop and Roman Shaposhnik.
Description
-------
Added new methods which can execute a script for a specified timeout and can also execute in the background.
This addresses bug BIGTOP-835.
https://issues.apache.org/jira/browse/BIGTOP-835
Diffs
-----
bigtop-test-framework/src/main/groovy/org/apache/bigtop/itest/shell/Shell.groovy ae3da68
bigtop-test-framework/src/test/groovy/org/apache/bigtop/itest/shell/ShellTest.groovy 1571e10
Diff: https://reviews.apache.org/r/9166/diff/
Testing
-------
Added unit tests for new functionality. Current unit tests pass
Thanks,
Hari Shreedharan
Re: Review Request: BIGTOP-835. The shell exec method must have variants
which have timeout and can run in background
Posted by Hari Shreedharan <hs...@cloudera.com>.
> On Jan. 31, 2013, 12:54 a.m., Mark Grover wrote:
> > bigtop-test-framework/src/main/groovy/org/apache/bigtop/itest/shell/Shell.groovy, line 71
> > <https://reviews.apache.org/r/9166/diff/1/?file=253572#file253572line71>
> >
> > Hi Hari, this is when java/groovy starts to get hairy for me so I wrote a test in groovy. Here is how it looks:
> > private void test(int x, Object... args){
> > System.out.println("with x")
> > }
> >
> > private void test(Object... args){
> > System.out.println("without x")
> > }
> > test(1,2,3)
> > test('a','b','c')
> > test("abc")
> > test(1)
> > test(1, 'a')
> >
> > The output is:
> > with x
> > without x
> > without x
> > with x
> > with x
> >
> > Conclusion: If the first argument of the call is int, the method with 1st int argument gets called (i.e. the signature you are adding) directly, bypassing the existing method that just takes Object...
> >
> > Do you see any concerns here?
> >
> > 1. Do you think those concerns may be resolved by making the newly signature private?
> > 2. Even better, do you think there is a way to accomodate the additional timeout as a part of the existing arguments parameter without changing the method signature?
>
> Hari Shreedharan wrote:
> Makes sense. I see one way to solve this, by adding an enum which is used only for this one purpose.
This is the problem with varargs. I missed this case, excellent catch Mark!
- Hari
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9166/#review15912
-----------------------------------------------------------
On Jan. 30, 2013, 8:35 p.m., Hari Shreedharan wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9166/
> -----------------------------------------------------------
>
> (Updated Jan. 30, 2013, 8:35 p.m.)
>
>
> Review request for bigtop and Roman Shaposhnik.
>
>
> Description
> -------
>
> Added new methods which can execute a script for a specified timeout and can also execute in the background.
>
>
> This addresses bug BIGTOP-835.
> https://issues.apache.org/jira/browse/BIGTOP-835
>
>
> Diffs
> -----
>
> bigtop-test-framework/src/main/groovy/org/apache/bigtop/itest/shell/Shell.groovy ae3da68
> bigtop-test-framework/src/test/groovy/org/apache/bigtop/itest/shell/ShellTest.groovy 1571e10
>
> Diff: https://reviews.apache.org/r/9166/diff/
>
>
> Testing
> -------
>
> Added unit tests for new functionality. Current unit tests pass
>
>
> Thanks,
>
> Hari Shreedharan
>
>
Re: Review Request: BIGTOP-835. The shell exec method must have variants
which have timeout and can run in background
Posted by Hari Shreedharan <hs...@cloudera.com>.
> On Jan. 31, 2013, 12:54 a.m., Mark Grover wrote:
> > bigtop-test-framework/src/main/groovy/org/apache/bigtop/itest/shell/Shell.groovy, line 71
> > <https://reviews.apache.org/r/9166/diff/1/?file=253572#file253572line71>
> >
> > Hi Hari, this is when java/groovy starts to get hairy for me so I wrote a test in groovy. Here is how it looks:
> > private void test(int x, Object... args){
> > System.out.println("with x")
> > }
> >
> > private void test(Object... args){
> > System.out.println("without x")
> > }
> > test(1,2,3)
> > test('a','b','c')
> > test("abc")
> > test(1)
> > test(1, 'a')
> >
> > The output is:
> > with x
> > without x
> > without x
> > with x
> > with x
> >
> > Conclusion: If the first argument of the call is int, the method with 1st int argument gets called (i.e. the signature you are adding) directly, bypassing the existing method that just takes Object...
> >
> > Do you see any concerns here?
> >
> > 1. Do you think those concerns may be resolved by making the newly signature private?
> > 2. Even better, do you think there is a way to accomodate the additional timeout as a part of the existing arguments parameter without changing the method signature?
Makes sense. I see one way to solve this, by adding an enum which is used only for this one purpose.
- Hari
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9166/#review15912
-----------------------------------------------------------
On Jan. 30, 2013, 8:35 p.m., Hari Shreedharan wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9166/
> -----------------------------------------------------------
>
> (Updated Jan. 30, 2013, 8:35 p.m.)
>
>
> Review request for bigtop and Roman Shaposhnik.
>
>
> Description
> -------
>
> Added new methods which can execute a script for a specified timeout and can also execute in the background.
>
>
> This addresses bug BIGTOP-835.
> https://issues.apache.org/jira/browse/BIGTOP-835
>
>
> Diffs
> -----
>
> bigtop-test-framework/src/main/groovy/org/apache/bigtop/itest/shell/Shell.groovy ae3da68
> bigtop-test-framework/src/test/groovy/org/apache/bigtop/itest/shell/ShellTest.groovy 1571e10
>
> Diff: https://reviews.apache.org/r/9166/diff/
>
>
> Testing
> -------
>
> Added unit tests for new functionality. Current unit tests pass
>
>
> Thanks,
>
> Hari Shreedharan
>
>
Re: Review Request: BIGTOP-835. The shell exec method must have
variants which have timeout and can run in background
Posted by Hari Shreedharan <hs...@cloudera.com>.
HI Wing Yew,
I modified the code to make sure that backward compatibility is not messed up, by removing the polymorphism. So this should no longer be a concern.
Hari
--
Hari Shreedharan
On Wednesday, January 30, 2013 at 5:57 PM, Wing Yew Poon wrote:
> I don't see any concern as long as
>
> shell.exec("cd /some/dir", "tar xvf some.tar", "hadoop fs -put
> contents contents");
>
> calls the exec without the int; while
>
> shell.exec(10000, "/some/dir/some.sh (http://some.sh)");
>
> calls the exec with the int.
> - Wing Yew
>
> On Wed, Jan 30, 2013 at 4:54 PM, Mark Grover
> <grover.markgrover@gmail.com (mailto:grover.markgrover@gmail.com)> wrote:
> >
> > -----------------------------------------------------------
> > This is an automatically generated e-mail. To reply, visit:
> > https://reviews.apache.org/r/9166/#review15912
> > -----------------------------------------------------------
> >
> >
> >
> > bigtop-test-framework/src/main/groovy/org/apache/bigtop/itest/shell/Shell.groovy
> > <https://reviews.apache.org/r/9166/#comment34193>
> >
> > Hi Hari, this is when java/groovy starts to get hairy for me so I wrote a test in groovy. Here is how it looks:
> > private void test(int x, Object... args){
> > System.out.println("with x")
> > }
> >
> > private void test(Object... args){
> > System.out.println("without x")
> > }
> > test(1,2,3)
> > test('a','b','c')
> > test("abc")
> > test(1)
> > test(1, 'a')
> >
> > The output is:
> > with x
> > without x
> > without x
> > with x
> > with x
> >
> > Conclusion: If the first argument of the call is int, the method with 1st int argument gets called (i.e. the signature you are adding) directly, bypassing the existing method that just takes Object...
> >
> > Do you see any concerns here?
> >
> > 1. Do you think those concerns may be resolved by making the newly signature private?
> > 2. Even better, do you think there is a way to accomodate the additional timeout as a part of the existing arguments parameter without changing the method signature?
> >
> >
> > - Mark Grover
> >
> >
> > On Jan. 30, 2013, 8:35 p.m., Hari Shreedharan wrote:
> > >
> > > -----------------------------------------------------------
> > > This is an automatically generated e-mail. To reply, visit:
> > > https://reviews.apache.org/r/9166/
> > > -----------------------------------------------------------
> > >
> > > (Updated Jan. 30, 2013, 8:35 p.m.)
> > >
> > >
> > > Review request for bigtop and Roman Shaposhnik.
> > >
> > >
> > > Description
> > > -------
> > >
> > > Added new methods which can execute a script for a specified timeout and can also execute in the background.
> > >
> > >
> > > This addresses bug BIGTOP-835.
> > > https://issues.apache.org/jira/browse/BIGTOP-835
> > >
> > >
> > > Diffs
> > > -----
> > >
> > > bigtop-test-framework/src/main/groovy/org/apache/bigtop/itest/shell/Shell.groovy ae3da68
> > > bigtop-test-framework/src/test/groovy/org/apache/bigtop/itest/shell/ShellTest.groovy 1571e10
> > >
> > > Diff: https://reviews.apache.org/r/9166/diff/
> > >
> > >
> > > Testing
> > > -------
> > >
> > > Added unit tests for new functionality. Current unit tests pass
> > >
> > >
> > > Thanks,
> > >
> > > Hari Shreedharan
Re: Review Request: BIGTOP-835. The shell exec method must have
variants which have timeout and can run in background
Posted by Wing Yew Poon <wy...@cloudera.com>.
I don't see any concern as long as
shell.exec("cd /some/dir", "tar xvf some.tar", "hadoop fs -put
contents contents");
calls the exec without the int; while
shell.exec(10000, "/some/dir/some.sh");
calls the exec with the int.
- Wing Yew
On Wed, Jan 30, 2013 at 4:54 PM, Mark Grover
<gr...@gmail.com> wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9166/#review15912
> -----------------------------------------------------------
>
>
>
> bigtop-test-framework/src/main/groovy/org/apache/bigtop/itest/shell/Shell.groovy
> <https://reviews.apache.org/r/9166/#comment34193>
>
> Hi Hari, this is when java/groovy starts to get hairy for me so I wrote a test in groovy. Here is how it looks:
> private void test(int x, Object... args){
> System.out.println("with x")
> }
>
> private void test(Object... args){
> System.out.println("without x")
> }
> test(1,2,3)
> test('a','b','c')
> test("abc")
> test(1)
> test(1, 'a')
>
> The output is:
> with x
> without x
> without x
> with x
> with x
>
> Conclusion: If the first argument of the call is int, the method with 1st int argument gets called (i.e. the signature you are adding) directly, bypassing the existing method that just takes Object...
>
> Do you see any concerns here?
>
> 1. Do you think those concerns may be resolved by making the newly signature private?
> 2. Even better, do you think there is a way to accomodate the additional timeout as a part of the existing arguments parameter without changing the method signature?
>
>
> - Mark Grover
>
>
> On Jan. 30, 2013, 8:35 p.m., Hari Shreedharan wrote:
>>
>> -----------------------------------------------------------
>> This is an automatically generated e-mail. To reply, visit:
>> https://reviews.apache.org/r/9166/
>> -----------------------------------------------------------
>>
>> (Updated Jan. 30, 2013, 8:35 p.m.)
>>
>>
>> Review request for bigtop and Roman Shaposhnik.
>>
>>
>> Description
>> -------
>>
>> Added new methods which can execute a script for a specified timeout and can also execute in the background.
>>
>>
>> This addresses bug BIGTOP-835.
>> https://issues.apache.org/jira/browse/BIGTOP-835
>>
>>
>> Diffs
>> -----
>>
>> bigtop-test-framework/src/main/groovy/org/apache/bigtop/itest/shell/Shell.groovy ae3da68
>> bigtop-test-framework/src/test/groovy/org/apache/bigtop/itest/shell/ShellTest.groovy 1571e10
>>
>> Diff: https://reviews.apache.org/r/9166/diff/
>>
>>
>> Testing
>> -------
>>
>> Added unit tests for new functionality. Current unit tests pass
>>
>>
>> Thanks,
>>
>> Hari Shreedharan
>>
>>
>
Re: Review Request: BIGTOP-835. The shell exec method must have variants
which have timeout and can run in background
Posted by Mark Grover <gr...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9166/#review15912
-----------------------------------------------------------
bigtop-test-framework/src/main/groovy/org/apache/bigtop/itest/shell/Shell.groovy
<https://reviews.apache.org/r/9166/#comment34193>
Hi Hari, this is when java/groovy starts to get hairy for me so I wrote a test in groovy. Here is how it looks:
private void test(int x, Object... args){
System.out.println("with x")
}
private void test(Object... args){
System.out.println("without x")
}
test(1,2,3)
test('a','b','c')
test("abc")
test(1)
test(1, 'a')
The output is:
with x
without x
without x
with x
with x
Conclusion: If the first argument of the call is int, the method with 1st int argument gets called (i.e. the signature you are adding) directly, bypassing the existing method that just takes Object...
Do you see any concerns here?
1. Do you think those concerns may be resolved by making the newly signature private?
2. Even better, do you think there is a way to accomodate the additional timeout as a part of the existing arguments parameter without changing the method signature?
- Mark Grover
On Jan. 30, 2013, 8:35 p.m., Hari Shreedharan wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9166/
> -----------------------------------------------------------
>
> (Updated Jan. 30, 2013, 8:35 p.m.)
>
>
> Review request for bigtop and Roman Shaposhnik.
>
>
> Description
> -------
>
> Added new methods which can execute a script for a specified timeout and can also execute in the background.
>
>
> This addresses bug BIGTOP-835.
> https://issues.apache.org/jira/browse/BIGTOP-835
>
>
> Diffs
> -----
>
> bigtop-test-framework/src/main/groovy/org/apache/bigtop/itest/shell/Shell.groovy ae3da68
> bigtop-test-framework/src/test/groovy/org/apache/bigtop/itest/shell/ShellTest.groovy 1571e10
>
> Diff: https://reviews.apache.org/r/9166/diff/
>
>
> Testing
> -------
>
> Added unit tests for new functionality. Current unit tests pass
>
>
> Thanks,
>
> Hari Shreedharan
>
>
Re: Review Request: BIGTOP-835. The shell exec method must have variants
which have timeout and can run in background
Posted by Mark Grover <gr...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9166/#review15890
-----------------------------------------------------------
bigtop-test-framework/src/main/groovy/org/apache/bigtop/itest/shell/Shell.groovy
<https://reviews.apache.org/r/9166/#comment34163>
It's worth noting that this is a backwards incompatible change. Any code that presently calls exec() and hasn't been updated will now break because now it's first parameter being sent is being treated as a time out argument.
- Mark Grover
On Jan. 30, 2013, 8:35 p.m., Hari Shreedharan wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9166/
> -----------------------------------------------------------
>
> (Updated Jan. 30, 2013, 8:35 p.m.)
>
>
> Review request for bigtop and Roman Shaposhnik.
>
>
> Description
> -------
>
> Added new methods which can execute a script for a specified timeout and can also execute in the background.
>
>
> This addresses bug BIGTOP-835.
> https://issues.apache.org/jira/browse/BIGTOP-835
>
>
> Diffs
> -----
>
> bigtop-test-framework/src/main/groovy/org/apache/bigtop/itest/shell/Shell.groovy ae3da68
> bigtop-test-framework/src/test/groovy/org/apache/bigtop/itest/shell/ShellTest.groovy 1571e10
>
> Diff: https://reviews.apache.org/r/9166/diff/
>
>
> Testing
> -------
>
> Added unit tests for new functionality. Current unit tests pass
>
>
> Thanks,
>
> Hari Shreedharan
>
>
Re: Review Request: BIGTOP-835. The shell exec method must have variants
which have timeout and can run in background
Posted by Hari Shreedharan <hs...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9166/#review15891
-----------------------------------------------------------
bigtop-test-framework/src/main/groovy/org/apache/bigtop/itest/shell/Shell.groovy
<https://reviews.apache.org/r/9166/#comment34164>
Mark,
There is a version of exec below which calls this one:
Shell exec(Object... args) {
return exec(-1, args)
}
So this is not an incompatible change, and there will be no behavior change
- Hari Shreedharan
On Jan. 30, 2013, 8:35 p.m., Hari Shreedharan wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9166/
> -----------------------------------------------------------
>
> (Updated Jan. 30, 2013, 8:35 p.m.)
>
>
> Review request for bigtop and Roman Shaposhnik.
>
>
> Description
> -------
>
> Added new methods which can execute a script for a specified timeout and can also execute in the background.
>
>
> This addresses bug BIGTOP-835.
> https://issues.apache.org/jira/browse/BIGTOP-835
>
>
> Diffs
> -----
>
> bigtop-test-framework/src/main/groovy/org/apache/bigtop/itest/shell/Shell.groovy ae3da68
> bigtop-test-framework/src/test/groovy/org/apache/bigtop/itest/shell/ShellTest.groovy 1571e10
>
> Diff: https://reviews.apache.org/r/9166/diff/
>
>
> Testing
> -------
>
> Added unit tests for new functionality. Current unit tests pass
>
>
> Thanks,
>
> Hari Shreedharan
>
>
Re: Review Request: BIGTOP-835. The shell exec method must have variants
which have timeout and can run in background
Posted by Hari Shreedharan <hs...@cloudera.com>.
> On Jan. 31, 2013, 2:03 a.m., Mark Grover wrote:
> > bigtop-test-framework/src/test/groovy/org/apache/bigtop/itest/shell/ShellTest.groovy, line 54
> > <https://reviews.apache.org/r/9166/diff/2/?file=253628#file253628line54>
> >
> > Wouldn't a better test to be have something that never finishes (like an infinite loop) and have a timeout on that? Test would succeed asserting if it finished (possibly within a certain time).
>
> Hari Shreedharan wrote:
> Agreed, but it is not really required - as long as it is predictable.
An infinite loop will make this test more complex, since we will need to exec in a separate thread (to make sure the test eventually completes). But I get your point, I will increase the sleep time to 30 seconds - scheduling won't delay things for that long anyway.
- Hari
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9166/#review15917
-----------------------------------------------------------
On Jan. 31, 2013, 1:37 a.m., Hari Shreedharan wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9166/
> -----------------------------------------------------------
>
> (Updated Jan. 31, 2013, 1:37 a.m.)
>
>
> Review request for bigtop and Roman Shaposhnik.
>
>
> Description
> -------
>
> Added new methods which can execute a script for a specified timeout and can also execute in the background.
>
>
> This addresses bug BIGTOP-835.
> https://issues.apache.org/jira/browse/BIGTOP-835
>
>
> Diffs
> -----
>
> bigtop-test-framework/src/main/groovy/org/apache/bigtop/itest/shell/Shell.groovy ae3da68
> bigtop-test-framework/src/test/groovy/org/apache/bigtop/itest/shell/ShellTest.groovy 1571e10
>
> Diff: https://reviews.apache.org/r/9166/diff/
>
>
> Testing
> -------
>
> Added unit tests for new functionality. Current unit tests pass
>
>
> Thanks,
>
> Hari Shreedharan
>
>
Re: Review Request: BIGTOP-835. The shell exec method must have variants
which have timeout and can run in background
Posted by Hari Shreedharan <hs...@cloudera.com>.
> On Jan. 31, 2013, 2:03 a.m., Mark Grover wrote:
> > bigtop-test-framework/src/test/groovy/org/apache/bigtop/itest/shell/ShellTest.groovy, line 54
> > <https://reviews.apache.org/r/9166/diff/2/?file=253628#file253628line54>
> >
> > Wouldn't a better test to be have something that never finishes (like an infinite loop) and have a timeout on that? Test would succeed asserting if it finished (possibly within a certain time).
Agreed, but it is not really required - as long as it is predictable.
> On Jan. 31, 2013, 2:03 a.m., Mark Grover wrote:
> > bigtop-test-framework/src/test/groovy/org/apache/bigtop/itest/shell/ShellTest.groovy, line 58
> > <https://reviews.apache.org/r/9166/diff/2/?file=253628#file253628line58>
> >
> > Why 4000? Could you please add a comment stating so?
Just to ensure it is less than 5 seconds (which is what the process sleeps for). Accounting for scheduling irregularities, it is safe to make sure it didnt take the full 5 seconds.
- Hari
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9166/#review15917
-----------------------------------------------------------
On Jan. 31, 2013, 1:37 a.m., Hari Shreedharan wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9166/
> -----------------------------------------------------------
>
> (Updated Jan. 31, 2013, 1:37 a.m.)
>
>
> Review request for bigtop and Roman Shaposhnik.
>
>
> Description
> -------
>
> Added new methods which can execute a script for a specified timeout and can also execute in the background.
>
>
> This addresses bug BIGTOP-835.
> https://issues.apache.org/jira/browse/BIGTOP-835
>
>
> Diffs
> -----
>
> bigtop-test-framework/src/main/groovy/org/apache/bigtop/itest/shell/Shell.groovy ae3da68
> bigtop-test-framework/src/test/groovy/org/apache/bigtop/itest/shell/ShellTest.groovy 1571e10
>
> Diff: https://reviews.apache.org/r/9166/diff/
>
>
> Testing
> -------
>
> Added unit tests for new functionality. Current unit tests pass
>
>
> Thanks,
>
> Hari Shreedharan
>
>
Re: Review Request: BIGTOP-835. The shell exec method must have variants
which have timeout and can run in background
Posted by Mark Grover <gr...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9166/#review15917
-----------------------------------------------------------
bigtop-test-framework/src/main/groovy/org/apache/bigtop/itest/shell/Shell.groovy
<https://reviews.apache.org/r/9166/#comment34197>
NItpick: If all you want is an empty list, could you use Collections.EMPTY_LIST?
bigtop-test-framework/src/main/groovy/org/apache/bigtop/itest/shell/Shell.groovy
<https://reviews.apache.org/r/9166/#comment34198>
If you do decide to change out to be Collections.EMPTY_LIST, could you please change this to be the same as well for consistency?
bigtop-test-framework/src/test/groovy/org/apache/bigtop/itest/shell/ShellTest.groovy
<https://reviews.apache.org/r/9166/#comment34201>
Wouldn't a better test to be have something that never finishes (like an infinite loop) and have a timeout on that? Test would succeed asserting if it finished (possibly within a certain time).
bigtop-test-framework/src/test/groovy/org/apache/bigtop/itest/shell/ShellTest.groovy
<https://reviews.apache.org/r/9166/#comment34199>
Why 4000? Could you please add a comment stating so?
bigtop-test-framework/src/test/groovy/org/apache/bigtop/itest/shell/ShellTest.groovy
<https://reviews.apache.org/r/9166/#comment34200>
Same here
- Mark Grover
On Jan. 31, 2013, 1:37 a.m., Hari Shreedharan wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9166/
> -----------------------------------------------------------
>
> (Updated Jan. 31, 2013, 1:37 a.m.)
>
>
> Review request for bigtop and Roman Shaposhnik.
>
>
> Description
> -------
>
> Added new methods which can execute a script for a specified timeout and can also execute in the background.
>
>
> This addresses bug BIGTOP-835.
> https://issues.apache.org/jira/browse/BIGTOP-835
>
>
> Diffs
> -----
>
> bigtop-test-framework/src/main/groovy/org/apache/bigtop/itest/shell/Shell.groovy ae3da68
> bigtop-test-framework/src/test/groovy/org/apache/bigtop/itest/shell/ShellTest.groovy 1571e10
>
> Diff: https://reviews.apache.org/r/9166/diff/
>
>
> Testing
> -------
>
> Added unit tests for new functionality. Current unit tests pass
>
>
> Thanks,
>
> Hari Shreedharan
>
>
Re: Review Request: BIGTOP-835. The shell exec method must have variants
which have timeout and can run in background
Posted by Hari Shreedharan <hs...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9166/
-----------------------------------------------------------
(Updated Jan. 31, 2013, 10:16 p.m.)
Review request for bigtop and Roman Shaposhnik.
Changes
-------
Rename execInBackground to fork
Description
-------
Added new methods which can execute a script for a specified timeout and can also execute in the background.
This addresses bug BIGTOP-835.
https://issues.apache.org/jira/browse/BIGTOP-835
Diffs (updated)
-----
bigtop-test-framework/src/main/groovy/org/apache/bigtop/itest/shell/Shell.groovy ae3da68
bigtop-test-framework/src/test/groovy/org/apache/bigtop/itest/shell/ShellTest.groovy 1571e10
Diff: https://reviews.apache.org/r/9166/diff/
Testing
-------
Added unit tests for new functionality. Current unit tests pass
Thanks,
Hari Shreedharan
Re: Review Request: BIGTOP-835. The shell exec method must have variants
which have timeout and can run in background
Posted by Hari Shreedharan <hs...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9166/
-----------------------------------------------------------
(Updated Jan. 31, 2013, 3:49 a.m.)
Review request for bigtop and Roman Shaposhnik.
Changes
-------
Minor unit test updates and use Collections.EMPTY_LIST instead of ArrayList of size 0.
Description
-------
Added new methods which can execute a script for a specified timeout and can also execute in the background.
This addresses bug BIGTOP-835.
https://issues.apache.org/jira/browse/BIGTOP-835
Diffs (updated)
-----
bigtop-test-framework/src/main/groovy/org/apache/bigtop/itest/shell/Shell.groovy ae3da68
bigtop-test-framework/src/test/groovy/org/apache/bigtop/itest/shell/ShellTest.groovy 1571e10
Diff: https://reviews.apache.org/r/9166/diff/
Testing
-------
Added unit tests for new functionality. Current unit tests pass
Thanks,
Hari Shreedharan
Re: Review Request: BIGTOP-835. The shell exec method must have variants
which have timeout and can run in background
Posted by Hari Shreedharan <hs...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9166/
-----------------------------------------------------------
(Updated Jan. 31, 2013, 1:37 a.m.)
Review request for bigtop and Roman Shaposhnik.
Changes
-------
Fix the backward compatibility issues by getting rid of polymorphism, since var args can wreak havoc on polymorphic methods.
Description
-------
Added new methods which can execute a script for a specified timeout and can also execute in the background.
This addresses bug BIGTOP-835.
https://issues.apache.org/jira/browse/BIGTOP-835
Diffs (updated)
-----
bigtop-test-framework/src/main/groovy/org/apache/bigtop/itest/shell/Shell.groovy ae3da68
bigtop-test-framework/src/test/groovy/org/apache/bigtop/itest/shell/ShellTest.groovy 1571e10
Diff: https://reviews.apache.org/r/9166/diff/
Testing
-------
Added unit tests for new functionality. Current unit tests pass
Thanks,
Hari Shreedharan