> On March 12, 2014, 8:43 p.m., Terence Yim wrote:
> > twill-examples/pom.xml, line 63
> > <https://reviews.apache.org/r/19151/diff/1/?file=517670#file517670line63>
> >
> >     What the profile is for?

Removed the profiles.


> On March 12, 2014, 8:43 p.m., Terence Yim wrote:
> > twill-examples/src/main/java/org/apache/twill/yarn/HelloWorld.java, line 41
> > <https://reviews.apache.org/r/19151/diff/1/?file=517671#file517671line41>
> >
> >     If there is only one runnable, this app is not needed.

Removed the app.

My thinking was that it would be easier for new developers to figure out how to 
configure a TwillApplication and how it relates to a TwillRunnable. However, I 
do think that the Hello World example should be as simple as possible, but 
there's still value in including a more explicit Hello World example.


> On March 12, 2014, 8:43 p.m., Terence Yim wrote:
> > twill-examples/src/main/java/org/apache/twill/yarn/HelloWorld.java, line 71
> > <https://reviews.apache.org/r/19151/diff/1/?file=517671#file517671line71>
> >
> >     Ideally there is a try-finally block to stop the twillRunner when main 
> > exit.

Added a shutdown hook.


> On March 12, 2014, 8:43 p.m., Terence Yim wrote:
> > twill-ext/src/main/java/org/apache/twill/ext/BundledJarApplication.java, 
> > line 206
> > <https://reviews.apache.org/r/19151/diff/1/?file=517675#file517675line206>
> >
> >     What is this for?

Was for communication between the BundledJarRunnable and the actual class that 
is being run, but I didn't implement that yet. I removed it for now.


> On March 12, 2014, 8:43 p.m., Terence Yim wrote:
> > twill-ext/src/main/java/org/apache/twill/ext/BundledJarApplication.java, 
> > line 27
> > <https://reviews.apache.org/r/19151/diff/1/?file=517675#file517675line27>
> >
> >     What is it trying to do? When the bundled jar path has to be in 
> > classpath?

It's loading the bundled jar from within the jar that's being used to run the 
Twill application.

I think instead of having to include the bundled jar within the Twill 
application jar, we can just have it outside of the application jar as you 
mentioned in a previous comment.


> On March 12, 2014, 8:43 p.m., Terence Yim wrote:
> > twill-examples/src/main/java/org/apache/twill/yarn/JarRunnerExample.java, 
> > line 40
> > <https://reviews.apache.org/r/19151/diff/1/?file=517672#file517672line40>
> >
> >     Wouldn't it better to have the example main takes a bundle jar file 
> > from the argument?
> >     
> >     Also, we don't want to provide bundled.jar binary in the source, as 
> > this complicated licensing. However, you could have another project that 
> > contains the source to create a bundle jar.

Yes, I agree that external bundle jar would be better.

Also, I'll create another module, yes. Probably under twill-examples.


> On March 12, 2014, 8:43 p.m., Terence Yim wrote:
> > twill-ext/src/main/java/org/apache/twill/ext/BundledJarApplication.java, 
> > line 64
> > <https://reviews.apache.org/r/19151/diff/1/?file=517675#file517675line64>
> >
> >     I think it's better to have the runnable wrapper passes runtime 
> > argument to the main method of the target class rather than using 
> > configuration time configs as arguments provided to main method.

I'll create a class extending BundledJarApplication that takes the bundled jar 
and various configuration configured as runtime arguments.


- Jiahua


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


On March 12, 2014, 8:14 p.m., Jiahua Wang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19151/
> -----------------------------------------------------------
> 
> (Updated March 12, 2014, 8:14 p.m.)
> 
> 
> Review request for Twill.
> 
> 
> Repository: twill
> 
> 
> Description
> -------
> 
> Implement a TwillApplication that runs bundled jars. This is to allow user 
> programs to run without having to worry about dependency conflicts with 
> Twill. For example, Presto uses Guava 16 whereas Twill uses Guava 13, so they 
> can't run in the same class loader.
> 
> See JarRunnerExample (in twill-examples) for example usage of 
> BundledJarApplication (in twill-ext).
> 
> 
> Diffs
> -----
> 
>   pom.xml 8e93850 
>   twill-examples/.gitignore PRE-CREATION 
>   twill-examples/pom.xml PRE-CREATION 
>   twill-examples/src/main/java/org/apache/twill/yarn/HelloWorld.java 
> PRE-CREATION 
>   twill-examples/src/main/java/org/apache/twill/yarn/JarRunnerExample.java 
> PRE-CREATION 
>   twill-examples/src/main/resources/org/apache/twill/yarn/bundled.jar.keep 
> PRE-CREATION 
>   twill-ext/pom.xml PRE-CREATION 
>   twill-ext/src/main/java/org/apache/twill/ext/BundledJarApplication.java 
> PRE-CREATION 
>   twill-ext/src/main/java/org/apache/twill/ext/BundledJarRunnable.java 
> PRE-CREATION 
>   twill-ext/src/main/java/org/apache/twill/ext/BundledJarRunner.java 
> PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/19151/diff/
> 
> 
> Testing
> -------
> 
> Tested JarRunnerExample on a single node cluster. Also tested a Twill 
> application that starts up multiple runnables that run Presto services.
> 
> 
> Thanks,
> 
> Jiahua Wang
> 
>

Reply via email to