[GitHub] twill pull request #80: (TWILL-264) Fix Discoverable.hashCode implementation

2019-01-25 Thread anew
Github user anew commented on a diff in the pull request: https://github.com/apache/twill/pull/80#discussion_r251089608 --- Diff: twill-discovery-api/src/test/java/org/apache/twill/discovery/DiscoverableTest.java --- @@ -0,0 +1,36 @@ +package org.apache.twill.discovery

[GitHub] twill pull request #67: (TWILL-61) Fix to allow higher attempts to relaunch ...

2018-03-14 Thread anew
Github user anew commented on a diff in the pull request: https://github.com/apache/twill/pull/67#discussion_r174643542 --- Diff: twill-zookeeper/src/main/java/org/apache/twill/zookeeper/ZKOperations.java --- @@ -281,6 +286,73 @@ public void onFailure(Throwable t

[GitHub] twill pull request #67: (TWILL-61) Fix to allow higher attempts to relaunch ...

2018-03-14 Thread anew
Github user anew commented on a diff in the pull request: https://github.com/apache/twill/pull/67#discussion_r174643070 --- Diff: twill-yarn/src/main/java/org/apache/twill/internal/appmaster/ApplicationMasterMain.java --- @@ -170,45 +167,7 @@ private ApplicationKafkaService

[GitHub] twill pull request #67: (TWILL-61) Fix to allow higher attempts to relaunch ...

2018-03-14 Thread anew
Github user anew commented on a diff in the pull request: https://github.com/apache/twill/pull/67#discussion_r174639182 --- Diff: twill-core/src/main/java/org/apache/twill/internal/AbstractTwillService.java --- @@ -216,11 +217,52 @@ protected final void shutDown() throws Exception

[GitHub] twill pull request #67: (TWILL-61) Fix to allow higher attempts to relaunch ...

2018-03-14 Thread anew
Github user anew commented on a diff in the pull request: https://github.com/apache/twill/pull/67#discussion_r174638923 --- Diff: twill-core/src/main/java/org/apache/twill/internal/AbstractTwillService.java --- @@ -216,11 +217,52 @@ protected final void shutDown() throws Exception

[GitHub] twill pull request #67: (TWILL-61) Fix to allow higher attempts to relaunch ...

2018-03-14 Thread anew
Github user anew commented on a diff in the pull request: https://github.com/apache/twill/pull/67#discussion_r174580690 --- Diff: twill-core/src/main/java/org/apache/twill/internal/kafka/client/SimpleKafkaPublisher.java --- @@ -159,30 +159,37 @@ public void changed(BrokerService

[GitHub] twill pull request #67: (TWILL-61) Fix to allow higher attempts to relaunch ...

2018-03-14 Thread anew
Github user anew commented on a diff in the pull request: https://github.com/apache/twill/pull/67#discussion_r174571435 --- Diff: twill-core/src/main/java/org/apache/twill/internal/AbstractTwillService.java --- @@ -216,11 +217,52 @@ protected final void shutDown() throws Exception

[GitHub] twill pull request #67: (TWILL-61) Fix to allow higher attempts to relaunch ...

2018-03-14 Thread anew
Github user anew commented on a diff in the pull request: https://github.com/apache/twill/pull/67#discussion_r174571807 --- Diff: twill-core/src/main/java/org/apache/twill/internal/AbstractTwillService.java --- @@ -216,11 +217,52 @@ protected final void shutDown() throws Exception

[GitHub] twill pull request #67: (TWILL-61) Fix to allow higher attempts to relaunch ...

2018-03-14 Thread anew
Github user anew commented on a diff in the pull request: https://github.com/apache/twill/pull/67#discussion_r174583711 --- Diff: twill-yarn/src/main/java/org/apache/twill/internal/appmaster/ApplicationMasterMain.java --- @@ -165,9 +168,47 @@ private ApplicationKafkaService

[GitHub] twill pull request #67: (TWILL-61) Fix to allow higher attempts to relaunch ...

2018-03-14 Thread anew
Github user anew commented on a diff in the pull request: https://github.com/apache/twill/pull/67#discussion_r174573795 --- Diff: twill-core/src/main/java/org/apache/twill/internal/AbstractTwillService.java --- @@ -216,11 +217,52 @@ protected final void shutDown() throws Exception

[GitHub] twill issue #59: (TWILL-241) Added per runnable configuration and jvm option...

2017-08-07 Thread anew
Github user anew commented on the issue: https://github.com/apache/twill/pull/59 LGTM --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature

[GitHub] twill pull request #59: (TWILL-241) Added per runnable configuration and jvm...

2017-08-07 Thread anew
Github user anew commented on a diff in the pull request: https://github.com/apache/twill/pull/59#discussion_r131708600 --- Diff: twill-core/src/main/java/org/apache/twill/internal/TwillRuntimeSpecification.java --- @@ -171,9 +168,27 @@ public String getKafkaZKConnect

[GitHub] twill pull request #59: (TWILL-241) Added per runnable configuration and jvm...

2017-08-07 Thread anew
Github user anew commented on a diff in the pull request: https://github.com/apache/twill/pull/59#discussion_r131707378 --- Diff: twill-core/src/main/java/org/apache/twill/internal/TwillRuntimeSpecification.java --- @@ -171,9 +168,27 @@ public String getKafkaZKConnect

[GitHub] twill pull request #59: (TWILL-241) Added per runnable configuration and jvm...

2017-08-04 Thread anew
Github user anew commented on a diff in the pull request: https://github.com/apache/twill/pull/59#discussion_r131511104 --- Diff: twill-yarn/src/test/java/org/apache/twill/yarn/ContainerSizeTestRun.java --- @@ -67,20 +69,37 @@ public void testContainerSize() throws

[GitHub] twill pull request #59: (TWILL-241) Added per runnable configuration and jvm...

2017-08-04 Thread anew
Github user anew commented on a diff in the pull request: https://github.com/apache/twill/pull/59#discussion_r131510954 --- Diff: twill-yarn/src/main/java/org/apache/twill/yarn/YarnTwillPreparer.java --- @@ -183,6 +184,13 @@ public TwillPreparer withConfiguration(Map<Str

[GitHub] twill issue #43: (TWILL-194) Acquires KMS delegation token correctly

2017-03-27 Thread anew
Github user anew commented on the issue: https://github.com/apache/twill/pull/43 LGTM --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature

[GitHub] twill pull request #38: (TWILL-119) Make YarnAppClient supports HA RM

2017-03-17 Thread anew
Github user anew commented on a diff in the pull request: https://github.com/apache/twill/pull/38#discussion_r106763328 --- Diff: twill-yarn/src/main/hadoop23/org/apache/twill/internal/yarn/Hadoop23YarnAppClient.java --- @@ -0,0 +1,111 @@ +/* + * Licensed to the Apache

[GitHub] twill pull request #38: (TWILL-119) Make YarnAppClient supports HA RM

2017-03-17 Thread anew
Github user anew commented on a diff in the pull request: https://github.com/apache/twill/pull/38#discussion_r106763336 --- Diff: twill-yarn/src/main/hadoop23/org/apache/twill/internal/yarn/Hadoop23YarnAppClient.java --- @@ -0,0 +1,111 @@ +/* + * Licensed to the Apache

[GitHub] twill pull request #27: Fix location permission test for older Hadoop versio...

2017-01-27 Thread anew
GitHub user anew opened a pull request: https://github.com/apache/twill/pull/27 Fix location permission test for older Hadoop versions In older Hadoop versions (2.0), files are always created with permissions that do not have execute bits. In later Hadoop versions, execute

[GitHub] twill issue #26: (TWILL-208) add Location.mkdirs(String permissions)

2017-01-26 Thread anew
Github user anew commented on the issue: https://github.com/apache/twill/pull/26 Squashing commits now. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so

[GitHub] twill pull request #26: (TWILL-208) add Location.mkdirs(String permissions)

2017-01-25 Thread anew
Github user anew commented on a diff in the pull request: https://github.com/apache/twill/pull/26#discussion_r97929702 --- Diff: twill-yarn/src/test/java/org/apache/twill/filesystem/HDFSLocationTest.java --- @@ -55,11 +55,19 @@ protected LocationFactory createLocationFactory

[GitHub] twill pull request #26: (TWILL-208) add Location.mkdirs(String permissions)

2017-01-25 Thread anew
Github user anew commented on a diff in the pull request: https://github.com/apache/twill/pull/26#discussion_r97880251 --- Diff: twill-yarn/src/test/java/org/apache/twill/filesystem/HDFSLocationTest.java --- @@ -55,11 +55,19 @@ protected LocationFactory createLocationFactory

[GitHub] twill pull request #26: (TWILL-208) add Location.mkdirs(String permissions)

2017-01-25 Thread anew
Github user anew commented on a diff in the pull request: https://github.com/apache/twill/pull/26#discussion_r97875317 --- Diff: twill-yarn/src/test/java/org/apache/twill/filesystem/HDFSLocationTest.java --- @@ -47,4 +52,14 @@ public static void finish() { protected

[GitHub] twill pull request #26: (TWILL-208) add Location.mkdirs(String permissions)

2017-01-25 Thread anew
Github user anew commented on a diff in the pull request: https://github.com/apache/twill/pull/26#discussion_r97874790 --- Diff: twill-common/src/main/java/org/apache/twill/filesystem/Location.java --- @@ -207,6 +207,21 @@ boolean mkdirs() throws IOException

[GitHub] twill pull request #26: (TWILL-208) add Location.mkdirs(String permissions)

2017-01-24 Thread anew
Github user anew commented on a diff in the pull request: https://github.com/apache/twill/pull/26#discussion_r97689959 --- Diff: twill-common/src/main/java/org/apache/twill/filesystem/LocalLocation.java --- @@ -251,6 +251,34 @@ public boolean mkdirs() throws IOException

[GitHub] twill pull request #26: (TWILL-208) add Location.mkdirs(String permissions)

2017-01-24 Thread anew
Github user anew commented on a diff in the pull request: https://github.com/apache/twill/pull/26#discussion_r97689935 --- Diff: twill-common/src/main/java/org/apache/twill/filesystem/LocalLocation.java --- @@ -98,8 +98,8 @@ public OutputStream getOutputStream() throws IOException

[GitHub] twill pull request #26: (TWILL-208) add Location.mkdirs(String permissions)

2017-01-24 Thread anew
Github user anew commented on a diff in the pull request: https://github.com/apache/twill/pull/26#discussion_r97689666 --- Diff: twill-common/src/main/java/org/apache/twill/filesystem/Location.java --- @@ -207,6 +207,21 @@ boolean mkdirs() throws IOException

[GitHub] twill pull request #26: (TWILL-208) add Location.mkdirs(String permissions)

2017-01-24 Thread anew
GitHub user anew opened a pull request: https://github.com/apache/twill/pull/26 (TWILL-208) add Location.mkdirs(String permissions) - adds mkdirs() that takes permissions - makes sure getOutputStream(permisssions) also uses that when creating the directories - add test

[GitHub] twill issue #14: TWILL-138 Runtime log level change

2016-12-03 Thread anew
Github user anew commented on the issue: https://github.com/apache/twill/pull/14 OK, my concerns are addressed and it LGTM. Please wait for @chtyim to do another pass before this gets merged. --- If your project is set up for it, you can reply to this email and have your reply

[GitHub] twill pull request #14: TWILL-138 Runtime log level change

2016-12-03 Thread anew
Github user anew commented on a diff in the pull request: https://github.com/apache/twill/pull/14#discussion_r90768827 --- Diff: twill-yarn/src/main/java/org/apache/twill/internal/container/TwillContainerService.java --- @@ -51,29 +63,36 @@ private final ClassLoader

[GitHub] twill pull request #14: TWILL-138 Runtime log level change

2016-12-03 Thread anew
Github user anew commented on a diff in the pull request: https://github.com/apache/twill/pull/14#discussion_r90767247 --- Diff: twill-yarn/src/main/java/org/apache/twill/internal/container/TwillContainerService.java --- @@ -51,29 +63,36 @@ private final ClassLoader

[GitHub] twill pull request #14: TWILL-138 Runtime log level change

2016-12-03 Thread anew
Github user anew commented on a diff in the pull request: https://github.com/apache/twill/pull/14#discussion_r90767079 --- Diff: twill-api/src/main/java/org/apache/twill/api/TwillController.java --- @@ -89,4 +90,44 @@ * @return A {@link Future} that will be completed when

[GitHub] twill pull request #14: TWILL-138 Runtime log level change

2016-12-03 Thread anew
Github user anew commented on a diff in the pull request: https://github.com/apache/twill/pull/14#discussion_r90767209 --- Diff: twill-api/src/main/java/org/apache/twill/api/TwillController.java --- @@ -89,4 +90,45 @@ * @return A {@link Future} that will be completed when

[GitHub] twill pull request #14: TWILL-138 Runtime log level change

2016-11-02 Thread anew
Github user anew commented on a diff in the pull request: https://github.com/apache/twill/pull/14#discussion_r86287618 --- Diff: twill-core/src/main/java/org/apache/twill/internal/TwillRuntimeSpecification.java --- @@ -89,8 +91,7 @@ public String getRmSchedulerAddr

[GitHub] twill pull request #14: TWILL-138 Runtime log level change

2016-11-02 Thread anew
Github user anew commented on a diff in the pull request: https://github.com/apache/twill/pull/14#discussion_r86287397 --- Diff: twill-yarn/src/test/java/org/apache/twill/yarn/LogLevelChangeTestRun.java --- @@ -0,0 +1,249 @@ +/* + * Licensed to the Apache Software

[GitHub] twill pull request #14: TWILL-138 Runtime log level change

2016-11-02 Thread anew
Github user anew commented on a diff in the pull request: https://github.com/apache/twill/pull/14#discussion_r86287245 --- Diff: twill-yarn/src/main/java/org/apache/twill/yarn/YarnTwillPreparer.java --- @@ -296,8 +298,42 @@ public TwillPreparer addSecureStore(SecureStore

[GitHub] twill pull request #14: TWILL-138 Runtime log level change

2016-11-02 Thread anew
Github user anew commented on a diff in the pull request: https://github.com/apache/twill/pull/14#discussion_r86286972 --- Diff: twill-yarn/src/main/java/org/apache/twill/internal/appmaster/RunningContainers.java --- @@ -475,6 +496,11 @@ private void sendMessage(final String

[GitHub] twill pull request #14: TWILL-138 Runtime log level change

2016-11-02 Thread anew
Github user anew commented on a diff in the pull request: https://github.com/apache/twill/pull/14#discussion_r86275201 --- Diff: twill-core/src/main/java/org/apache/twill/internal/DefaultResourceReport.java --- @@ -158,4 +174,31 @@ public String toString