You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@twill.apache.org by Jiahua Wang <ji...@gmail.com> on 2014/03/12 20:32:35 UTC

Review Request 19151: TWILL-57: TwillApplication that runs bundled jars

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

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 
  twill-yarn/src/main/java/org/apache/twill/yarn/YarnTwillPreparer.java 8c96629 

Diff: https://reviews.apache.org/r/19151/diff/


Testing
-------

Tested JarRunnerExample on a single node cluster.


Thanks,

Jiahua Wang


Re: Review Request 19151: TWILL-57: TwillApplication that runs bundled jars

Posted by Jiahua Wang <ji...@gmail.com>.

> On March 12, 2014, 9:36 p.m., Terence Yim wrote:
> > twill-ext/src/main/java/org/apache/twill/ext/BundledJarRunner.java, line 53
> > <https://reviews.apache.org/r/19151/diff/2/?file=517694#file517694line53>
> >
> >     Why need to copy before unJar?

I was copying from within the jar, so I thought I needed to copy to the local filesystem first.

Anyway, I'm having the bundle jar exist outside of the Twill application jar, so now this is no longer needed.


> On March 12, 2014, 9:36 p.m., Terence Yim wrote:
> > twill-ext/src/main/java/org/apache/twill/ext/BundledJarRunner.java, line 104
> > <https://reviews.apache.org/r/19151/diff/2/?file=517694#file517694line104>
> >
> >     Why not just always require the file be exists? Isn't that when this Runnable is executed with file being localized by Twill?

Removed this method.


- Jiahua


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


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
> 
>


Re: Review Request 19151: TWILL-57: TwillApplication that runs bundled jars

Posted by Terence Yim <ch...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19151/#review36991
-----------------------------------------------------------



twill-ext/src/main/java/org/apache/twill/ext/BundledJarRunnable.java
<https://reviews.apache.org/r/19151/#comment68244>

    It would be better if the mainArgs comes from the TwillContext.getArguments() as I mentioned above.



twill-ext/src/main/java/org/apache/twill/ext/BundledJarRunner.java
<https://reviews.apache.org/r/19151/#comment68230>

    Take a File rather than String.



twill-ext/src/main/java/org/apache/twill/ext/BundledJarRunner.java
<https://reviews.apache.org/r/19151/#comment68242>

    Normally no need to final local variables unless you want to use it in inner class. 



twill-ext/src/main/java/org/apache/twill/ext/BundledJarRunner.java
<https://reviews.apache.org/r/19151/#comment68229>

    delete on exit doesn't work on directory.



twill-ext/src/main/java/org/apache/twill/ext/BundledJarRunner.java
<https://reviews.apache.org/r/19151/#comment68233>

    Why need to copy before unJar?



twill-ext/src/main/java/org/apache/twill/ext/BundledJarRunner.java
<https://reviews.apache.org/r/19151/#comment68243>

    It might be better to allow user to specify what is the directory inside the bundle jar file contains .jar and what is the directory contains .class



twill-ext/src/main/java/org/apache/twill/ext/BundledJarRunner.java
<https://reviews.apache.org/r/19151/#comment68231>

    Why not just always require the file be exists? Isn't that when this Runnable is executed with file being localized by Twill?



twill-ext/src/main/java/org/apache/twill/ext/BundledJarRunner.java
<https://reviews.apache.org/r/19151/#comment68234>

    Why using full qualifier vs using import?



twill-ext/src/main/java/org/apache/twill/ext/BundledJarRunner.java
<https://reviews.apache.org/r/19151/#comment68236>

    Besides, this is not the right way of using InputSupplier, as it normally expect a new InputStream is created when getInput() is called, as the caller would close the stream.



twill-ext/src/main/java/org/apache/twill/ext/BundledJarRunner.java
<https://reviews.apache.org/r/19151/#comment68238>

    So empty directory are getting skipped? I think it's better to just unjar everything.



twill-ext/src/main/java/org/apache/twill/ext/BundledJarRunner.java
<https://reviews.apache.org/r/19151/#comment68239>

    Use try-finally to always close stream.
    
    OutputStream os = ...
    try {
      InputStream is = ...
      try {
        // copying logic.
    
      } finally {
        is.close();
      }
    } finally {
      os.close();
    }



twill-ext/src/main/java/org/apache/twill/ext/BundledJarRunner.java
<https://reviews.apache.org/r/19151/#comment68240>

    Naming is not good. Better call it something like getJarURLs


- Terence Yim


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
> 
>


Re: Review Request 19151: TWILL-57: TwillApplication that runs bundled jars

Posted by Jiahua Wang <ji...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19151/
-----------------------------------------------------------

(Updated March 17, 2014, 8:59 p.m.)


Review request for Twill.


Changes
-------

Style fixes.


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 (updated)
-----

  pom.xml 8e93850 
  twill-api/src/main/java/org/apache/twill/api/LocalFile.java bcc3e13 
  twill-api/src/main/java/org/apache/twill/internal/DefaultResourceSpecification.java 2998165 
  twill-core/src/main/java/org/apache/twill/internal/json/ResourceSpecificationCodec.java 5f7d7ae 
  twill-examples/echo/pom.xml PRE-CREATION 
  twill-examples/echo/src/main/java/echo/EchoMain.java PRE-CREATION 
  twill-examples/pom.xml PRE-CREATION 
  twill-examples/yarn/pom.xml PRE-CREATION 
  twill-examples/yarn/src/main/java/org/apache/twill/example/yarn/BundledJarExample.java PRE-CREATION 
  twill-examples/yarn/src/main/java/org/apache/twill/example/yarn/ConfigDrivenBundledJarExample.java PRE-CREATION 
  twill-examples/yarn/src/main/java/org/apache/twill/example/yarn/HelloWorld.java 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 
  twill-ext/src/main/java/org/apache/twill/ext/ConfigDrivenBundledJarApplication.java PRE-CREATION 
  twill-ext/src/main/java/org/apache/twill/ext/RunnableConfig.java PRE-CREATION 
  twill-ext/src/test/java/org/apache/twill/ext/BundledJarRunnableTest.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


Re: Review Request 19151: TWILL-57: TwillApplication that runs bundled jars

Posted by Jiahua Wang <ji...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19151/
-----------------------------------------------------------

(Updated March 15, 2014, 8:43 p.m.)


Review request for Twill.


Changes
-------

Apache license.


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 (updated)
-----

  pom.xml 8e93850 
  twill-api/src/main/java/org/apache/twill/api/LocalFile.java bcc3e13 
  twill-api/src/main/java/org/apache/twill/internal/DefaultResourceSpecification.java 2998165 
  twill-core/src/main/java/org/apache/twill/internal/json/ResourceSpecificationCodec.java 5f7d7ae 
  twill-examples/echo/pom.xml PRE-CREATION 
  twill-examples/echo/src/main/java/echo/EchoMain.java PRE-CREATION 
  twill-examples/pom.xml PRE-CREATION 
  twill-examples/yarn/pom.xml PRE-CREATION 
  twill-examples/yarn/src/main/java/org/apache/twill/example/yarn/BundledJarExample.java PRE-CREATION 
  twill-examples/yarn/src/main/java/org/apache/twill/example/yarn/ConfigDrivenBundledJarExample.java PRE-CREATION 
  twill-examples/yarn/src/main/java/org/apache/twill/example/yarn/HelloWorld.java 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 
  twill-ext/src/main/java/org/apache/twill/ext/ConfigDrivenBundledJarApplication.java PRE-CREATION 
  twill-ext/src/main/java/org/apache/twill/ext/RunnableConfig.java PRE-CREATION 
  twill-ext/src/test/java/org/apache/twill/ext/BundledJarRunnableTest.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


Re: Review Request 19151: TWILL-57: TwillApplication that runs bundled jars

Posted by Jiahua Wang <ji...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19151/
-----------------------------------------------------------

(Updated March 15, 2014, 8:39 p.m.)


Review request for Twill.


Changes
-------

Add feature to BundledJarRunnable to automatically save field values when transferring runnable to the container


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 (updated)
-----

  pom.xml 8e93850 
  twill-api/src/main/java/org/apache/twill/api/LocalFile.java bcc3e13 
  twill-api/src/main/java/org/apache/twill/internal/DefaultResourceSpecification.java 2998165 
  twill-core/src/main/java/org/apache/twill/internal/json/ResourceSpecificationCodec.java 5f7d7ae 
  twill-examples/echo/pom.xml PRE-CREATION 
  twill-examples/echo/src/main/java/echo/EchoMain.java PRE-CREATION 
  twill-examples/pom.xml PRE-CREATION 
  twill-examples/yarn/pom.xml PRE-CREATION 
  twill-examples/yarn/src/main/java/org/apache/twill/example/yarn/BundledJarExample.java PRE-CREATION 
  twill-examples/yarn/src/main/java/org/apache/twill/example/yarn/ConfigDrivenBundledJarExample.java PRE-CREATION 
  twill-examples/yarn/src/main/java/org/apache/twill/example/yarn/HelloWorld.java 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 
  twill-ext/src/main/java/org/apache/twill/ext/ConfigDrivenBundledJarApplication.java PRE-CREATION 
  twill-ext/src/main/java/org/apache/twill/ext/RunnableConfig.java PRE-CREATION 
  twill-ext/src/test/java/org/apache/twill/ext/BundledJarRunnableTest.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


Re: Review Request 19151: TWILL-57: TwillApplication that runs bundled jars

Posted by Jiahua Wang <ji...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19151/
-----------------------------------------------------------

(Updated March 14, 2014, 12:18 a.m.)


Review request for Twill.


Changes
-------

Changed parent class loader of class loader used in BundledJarRunner to default to the system class loader.
Fixed minor issues.


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 (updated)
-----

  pom.xml 8e93850 
  twill-api/src/main/java/org/apache/twill/api/LocalFile.java bcc3e13 
  twill-api/src/main/java/org/apache/twill/internal/DefaultResourceSpecification.java 2998165 
  twill-core/src/main/java/org/apache/twill/internal/json/ResourceSpecificationCodec.java 5f7d7ae 
  twill-examples/echo/pom.xml PRE-CREATION 
  twill-examples/echo/src/main/java/echo/EchoMain.java PRE-CREATION 
  twill-examples/pom.xml PRE-CREATION 
  twill-examples/yarn/pom.xml PRE-CREATION 
  twill-examples/yarn/src/main/java/org/apache/twill/example/yarn/BundledJarExample.java PRE-CREATION 
  twill-examples/yarn/src/main/java/org/apache/twill/example/yarn/ConfigDrivenBundledJarExample.java PRE-CREATION 
  twill-examples/yarn/src/main/java/org/apache/twill/example/yarn/HelloWorld.java 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 
  twill-ext/src/main/java/org/apache/twill/ext/ConfigDrivenBundledJarApplication.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


Re: Review Request 19151: TWILL-57: TwillApplication that runs bundled jars

Posted by Jiahua Wang <ji...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19151/
-----------------------------------------------------------

(Updated March 13, 2014, 10:47 p.m.)


Review request for Twill.


Changes
-------

Responded to comments.
Restructured twill-examples to not be a child of twill-parent and have submodules only instead of also having its own src.


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 (updated)
-----

  pom.xml 8e93850 
  twill-api/src/main/java/org/apache/twill/api/LocalFile.java bcc3e13 
  twill-api/src/main/java/org/apache/twill/internal/DefaultResourceSpecification.java 2998165 
  twill-core/src/main/java/org/apache/twill/internal/json/ResourceSpecificationCodec.java 5f7d7ae 
  twill-examples/echo/pom.xml PRE-CREATION 
  twill-examples/echo/src/main/java/echo/EchoMain.java PRE-CREATION 
  twill-examples/pom.xml PRE-CREATION 
  twill-examples/yarn/pom.xml PRE-CREATION 
  twill-examples/yarn/src/main/java/org/apache/twill/example/yarn/BundledJarExample.java PRE-CREATION 
  twill-examples/yarn/src/main/java/org/apache/twill/example/yarn/ConfigDrivenBundledJarExample.java PRE-CREATION 
  twill-examples/yarn/src/main/java/org/apache/twill/example/yarn/HelloWorld.java 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 
  twill-ext/src/main/java/org/apache/twill/ext/ConfigDrivenBundledJarApplication.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


Re: Review Request 19151: TWILL-57: TwillApplication that runs bundled jars

Posted by Jiahua Wang <ji...@gmail.com>.

> On March 13, 2014, 8:55 p.m., Terence Yim wrote:
> > twill-ext/src/main/java/org/apache/twill/ext/BundledJarRunner.java, line 107
> > <https://reviews.apache.org/r/19151/diff/3/?file=517926#file517926line107>
> >
> >     Why you want to get the parent of system classloader? I thought you want to use system classloader as the parent of the URLClassLoader.
> 
> Jiahua Wang wrote:
>     I'm using the parent of system classloader b/c system classloader has the Twill classes and dependencies (including Guava 13), so loading the Presto stuff into a child classloader of the system classloader would cause Guava conflicts.

Defaulted parent of classloader to be the system classloader.


- Jiahua


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


On March 14, 2014, 12:18 a.m., Jiahua Wang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19151/
> -----------------------------------------------------------
> 
> (Updated March 14, 2014, 12:18 a.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-api/src/main/java/org/apache/twill/api/LocalFile.java bcc3e13 
>   twill-api/src/main/java/org/apache/twill/internal/DefaultResourceSpecification.java 2998165 
>   twill-core/src/main/java/org/apache/twill/internal/json/ResourceSpecificationCodec.java 5f7d7ae 
>   twill-examples/echo/pom.xml PRE-CREATION 
>   twill-examples/echo/src/main/java/echo/EchoMain.java PRE-CREATION 
>   twill-examples/pom.xml PRE-CREATION 
>   twill-examples/yarn/pom.xml PRE-CREATION 
>   twill-examples/yarn/src/main/java/org/apache/twill/example/yarn/BundledJarExample.java PRE-CREATION 
>   twill-examples/yarn/src/main/java/org/apache/twill/example/yarn/ConfigDrivenBundledJarExample.java PRE-CREATION 
>   twill-examples/yarn/src/main/java/org/apache/twill/example/yarn/HelloWorld.java 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 
>   twill-ext/src/main/java/org/apache/twill/ext/ConfigDrivenBundledJarApplication.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
> 
>


Re: Review Request 19151: TWILL-57: TwillApplication that runs bundled jars

Posted by Jiahua Wang <ji...@gmail.com>.

> On March 13, 2014, 8:55 p.m., Terence Yim wrote:
> > twill-ext/src/main/java/org/apache/twill/ext/BundledJarRunner.java, line 107
> > <https://reviews.apache.org/r/19151/diff/3/?file=517926#file517926line107>
> >
> >     Why you want to get the parent of system classloader? I thought you want to use system classloader as the parent of the URLClassLoader.

I'm using the parent of system classloader b/c system classloader has the Twill classes and dependencies (including Guava 13), so loading the Presto stuff into a child classloader of the system classloader would cause Guava conflicts.


- Jiahua


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


On March 13, 2014, 10:47 p.m., Jiahua Wang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19151/
> -----------------------------------------------------------
> 
> (Updated March 13, 2014, 10:47 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-api/src/main/java/org/apache/twill/api/LocalFile.java bcc3e13 
>   twill-api/src/main/java/org/apache/twill/internal/DefaultResourceSpecification.java 2998165 
>   twill-core/src/main/java/org/apache/twill/internal/json/ResourceSpecificationCodec.java 5f7d7ae 
>   twill-examples/echo/pom.xml PRE-CREATION 
>   twill-examples/echo/src/main/java/echo/EchoMain.java PRE-CREATION 
>   twill-examples/pom.xml PRE-CREATION 
>   twill-examples/yarn/pom.xml PRE-CREATION 
>   twill-examples/yarn/src/main/java/org/apache/twill/example/yarn/BundledJarExample.java PRE-CREATION 
>   twill-examples/yarn/src/main/java/org/apache/twill/example/yarn/ConfigDrivenBundledJarExample.java PRE-CREATION 
>   twill-examples/yarn/src/main/java/org/apache/twill/example/yarn/HelloWorld.java 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 
>   twill-ext/src/main/java/org/apache/twill/ext/ConfigDrivenBundledJarApplication.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
> 
>


Re: Review Request 19151: TWILL-57: TwillApplication that runs bundled jars

Posted by Terence Yim <ch...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19151/#review37106
-----------------------------------------------------------



twill-ext/src/main/java/org/apache/twill/ext/BundledJarRunner.java
<https://reviews.apache.org/r/19151/#comment68413>

    Check the preconditions before anything.



twill-ext/src/main/java/org/apache/twill/ext/BundledJarRunner.java
<https://reviews.apache.org/r/19151/#comment68414>

    Why you want to get the parent of system classloader? I thought you want to use system classloader as the parent of the URLClassLoader.



twill-ext/src/main/java/org/apache/twill/ext/BundledJarRunner.java
<https://reviews.apache.org/r/19151/#comment68415>

    If there is error, isn't that we want to propagate so that the container would exit with failure rather than success?
    
    Also you might want to move the class loading into the init() method of TwillRunnable and only the invocation to run() method so that if failure happened in loading class, the container would exit with initialization error so that AM won't keep retrying.



twill-ext/src/main/java/org/apache/twill/ext/BundledJarRunner.java
<https://reviews.apache.org/r/19151/#comment68417>

    Move this into the try block, then in the finally you don't need to check for null. This makes the code cleaner.



twill-ext/src/main/java/org/apache/twill/ext/BundledJarRunner.java
<https://reviews.apache.org/r/19151/#comment68419>

    The while loop could be simplified to
    
    ByteStreams.copy(is, os);



twill-ext/src/main/java/org/apache/twill/ext/BundledJarRunner.java
<https://reviews.apache.org/r/19151/#comment68420>

    Just make unJar throw IOException and will make this method much less clutter.
    
    OutputStream os = new ...;
    try {
      InputStream is = jarFile.get...;
      try {
        ByteStreams.copy(is, os);
      } finally {
        is.close();
      }
    } finally {
      os.close();
    }



twill-ext/src/main/java/org/apache/twill/ext/BundledJarRunner.java
<https://reviews.apache.org/r/19151/#comment68421>

    Url should be all caps URL.



twill-ext/src/main/java/org/apache/twill/ext/RuntimeBundledJarApplication.java
<https://reviews.apache.org/r/19151/#comment68425>

    Should it be called something like ConfigDrivenBundledJarApp? (Not necessarily the best name, but should be clearer than Runtime).



twill-ext/src/main/java/org/apache/twill/ext/RuntimeBundledJarApplication.java
<https://reviews.apache.org/r/19151/#comment68423>

    There is a org.apache.twill.internal.json.ResourceSpecificationCodec. Is that not good enough?


- Terence Yim


On March 13, 2014, 1:15 a.m., Jiahua Wang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19151/
> -----------------------------------------------------------
> 
> (Updated March 13, 2014, 1:15 a.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-api/src/main/java/org/apache/twill/api/LocalFile.java bcc3e13 
>   twill-api/src/main/java/org/apache/twill/internal/DefaultResourceSpecification.java 2998165 
>   twill-examples/echo/pom.xml PRE-CREATION 
>   twill-examples/echo/src/main/java/echo/EchoMain.java PRE-CREATION 
>   twill-examples/pom.xml PRE-CREATION 
>   twill-examples/src/main/java/org/apache/twill/yarn/BundledJarExample.java PRE-CREATION 
>   twill-examples/src/main/java/org/apache/twill/yarn/HelloWorld.java PRE-CREATION 
>   twill-examples/src/main/java/org/apache/twill/yarn/RuntimeBundledJarExample.java 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 
>   twill-ext/src/main/java/org/apache/twill/ext/RuntimeBundledJarApplication.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
> 
>


Re: Review Request 19151: TWILL-57: TwillApplication that runs bundled jars

Posted by Jiahua Wang <ji...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19151/
-----------------------------------------------------------

(Updated March 13, 2014, 1:15 a.m.)


Review request for Twill.


Changes
-------

Responded to comments.
Changed BundledJarApplication to run an external jar instead of an embedded jar (within Twill application jar).
Added RuntimeBundledJarApplication, for configuring a BundledJarApplication using an external JSON file.


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 (updated)
-----

  pom.xml 8e93850 
  twill-api/src/main/java/org/apache/twill/api/LocalFile.java bcc3e13 
  twill-api/src/main/java/org/apache/twill/internal/DefaultResourceSpecification.java 2998165 
  twill-examples/echo/pom.xml PRE-CREATION 
  twill-examples/echo/src/main/java/echo/EchoMain.java PRE-CREATION 
  twill-examples/pom.xml PRE-CREATION 
  twill-examples/src/main/java/org/apache/twill/yarn/BundledJarExample.java PRE-CREATION 
  twill-examples/src/main/java/org/apache/twill/yarn/HelloWorld.java PRE-CREATION 
  twill-examples/src/main/java/org/apache/twill/yarn/RuntimeBundledJarExample.java 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 
  twill-ext/src/main/java/org/apache/twill/ext/RuntimeBundledJarApplication.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


Re: Review Request 19151: TWILL-57: TwillApplication that runs bundled jars

Posted by Jiahua Wang <ji...@gmail.com>.

> 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
> 
>


Re: Review Request 19151: TWILL-57: TwillApplication that runs bundled jars

Posted by Terence Yim <ch...@gmail.com>.

> On March 12, 2014, 8:43 p.m., Terence Yim wrote:
> >

All files need to carry the Apache License header.


- Terence


-----------------------------------------------------------
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
> 
>


Re: Review Request 19151: TWILL-57: TwillApplication that runs bundled jars

Posted by Jiahua Wang <ji...@gmail.com>.

> On March 12, 2014, 8:43 p.m., Terence Yim wrote:
> > pom.xml, line 140
> > <https://reviews.apache.org/r/19151/diff/1/?file=517668#file517668line140>
> >
> >     Should example built as a sub-module or should it be a separate maven project that depends on twill?

Changed to a separate maven project that depends on twill.


- 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
> 
>


Re: Review Request 19151: TWILL-57: TwillApplication that runs bundled jars

Posted by Terence Yim <ch...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19151/#review36967
-----------------------------------------------------------



pom.xml
<https://reviews.apache.org/r/19151/#comment68167>

    Should example built as a sub-module or should it be a separate maven project that depends on twill?



twill-examples/.gitignore
<https://reviews.apache.org/r/19151/#comment68168>

    You can add a configuration to the shade plugin to disable generation of the dependency-reduced-pom.xml
    
    <createDependencyReducedPom>false</createDependencyReducedPom>



twill-examples/pom.xml
<https://reviews.apache.org/r/19151/#comment68169>

    What the profile is for?



twill-examples/src/main/java/org/apache/twill/yarn/HelloWorld.java
<https://reviews.apache.org/r/19151/#comment68170>

    Missing javadoc.



twill-examples/src/main/java/org/apache/twill/yarn/HelloWorld.java
<https://reviews.apache.org/r/19151/#comment68171>

    Missing Javadoc.



twill-examples/src/main/java/org/apache/twill/yarn/HelloWorld.java
<https://reviews.apache.org/r/19151/#comment68172>

    If there is only one runnable, this app is not needed.



twill-examples/src/main/java/org/apache/twill/yarn/HelloWorld.java
<https://reviews.apache.org/r/19151/#comment68178>

    Wrong alignment.



twill-examples/src/main/java/org/apache/twill/yarn/HelloWorld.java
<https://reviews.apache.org/r/19151/#comment68179>

    Ideally there is a try-finally block to stop the twillRunner when main exit.



twill-examples/src/main/java/org/apache/twill/yarn/JarRunnerExample.java
<https://reviews.apache.org/r/19151/#comment68173>

    Missing javadoc.



twill-examples/src/main/java/org/apache/twill/yarn/JarRunnerExample.java
<https://reviews.apache.org/r/19151/#comment68177>

    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.



twill-ext/pom.xml
<https://reviews.apache.org/r/19151/#comment68181>

    Version should be inherited from parent pom, hence shouldn't be specified here.



twill-ext/src/main/java/org/apache/twill/ext/BundledJarApplication.java
<https://reviews.apache.org/r/19151/#comment68182>

    Missing javadoc.



twill-ext/src/main/java/org/apache/twill/ext/BundledJarApplication.java
<https://reviews.apache.org/r/19151/#comment68183>

    What is it trying to do? When the bundled jar path has to be in classpath?



twill-ext/src/main/java/org/apache/twill/ext/BundledJarApplication.java
<https://reviews.apache.org/r/19151/#comment68210>

    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.



twill-ext/src/main/java/org/apache/twill/ext/BundledJarApplication.java
<https://reviews.apache.org/r/19151/#comment68208>

    Child class should be able to specify resource spec per runnable and 128MB seems too low as default anyway.



twill-ext/src/main/java/org/apache/twill/ext/BundledJarApplication.java
<https://reviews.apache.org/r/19151/#comment68196>

    Use Preconditions.checkArgument



twill-ext/src/main/java/org/apache/twill/ext/BundledJarApplication.java
<https://reviews.apache.org/r/19151/#comment68207>

    I think it's better to use TwillRunnableSpecification and ResourceSpecification instead of creating new spec.



twill-ext/src/main/java/org/apache/twill/ext/BundledJarApplication.java
<https://reviews.apache.org/r/19151/#comment68209>

    What is this for?


- Terence Yim


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
> 
>


Re: Review Request 19151: TWILL-57: TwillApplication that runs bundled jars

Posted by Jiahua Wang <ji...@gmail.com>.
-----------------------------------------------------------
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.


Changes
-------

Added getResourceSpecification to InstanceSpecification, to allow user programs more flexibility. Also fixed the JarRunnerExample.


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 (updated)
-----

  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


Re: Review Request 19151: TWILL-57: TwillApplication that runs bundled jars

Posted by Jiahua Wang <ji...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19151/
-----------------------------------------------------------

(Updated March 12, 2014, 7:35 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 
  twill-yarn/src/main/java/org/apache/twill/yarn/YarnTwillPreparer.java 8c96629 

Diff: https://reviews.apache.org/r/19151/diff/


Testing (updated)
-------

Tested JarRunnerExample on a single node cluster. Also tested a Twill application that starts up multiple runnables that run Presto services.


Thanks,

Jiahua Wang