[GitHub] twill pull request #2: TWILL-190 Wait for Twill runnables to stop when resta...

Github user poornachandra commented on a diff in the pull request: https://github.com/apache/twill/pull/2#discussion_r75228168 --- Diff: twill-yarn/src/main/java/org/apache/twill/internal/appmaster/RunningContainers.java --- @@ -222,14 +231,17 @@ private void removeInstanceById

[GitHub] twill pull request #2: TWILL-190 Wait for Twill runnables to stop when resta...

Github user poornachandra commented on a diff in the pull request: https://github.com/apache/twill/pull/2#discussion_r75257373 --- Diff: twill-yarn/src/main/java/org/apache/twill/internal/appmaster/RunningContainers.java --- @@ -222,14 +231,17 @@ private void removeInstanceById

[GitHub] twill issue #2: TWILL-190 Wait for Twill runnables to stop when restarting t...

Github user poornachandra commented on the issue: https://github.com/apache/twill/pull/2 @chtyim All the tests pass now, please review. --- 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

[GitHub] twill pull request #2: TWILL-190 Wait for Twill runnables to stop when resta...

Github user poornachandra commented on a diff in the pull request: https://github.com/apache/twill/pull/2#discussion_r75439711 --- Diff: twill-core/src/main/java/org/apache/twill/internal/TwillContainerLauncher.java --- @@ -153,21 +155,29 @@ public TwillContainerController start

[GitHub] twill pull request #2: TWILL-190 Wait for Twill runnables to stop when resta...

Github user poornachandra commented on a diff in the pull request: https://github.com/apache/twill/pull/2#discussion_r75523615 --- Diff: twill-core/src/main/java/org/apache/twill/internal/TwillContainerLauncher.java --- @@ -177,7 +187,22 @@ protected void doStartUp

[GitHub] twill issue #2: TWILL-190 Wait for Twill runnables to stop when restarting t...

Github user poornachandra commented on the issue: https://github.com/apache/twill/pull/2 @hsaputra The PR is ready for review. I would appreciate a review of the overall approach and structure. I'm just adding code to better handle exceptions or adding more test cases now.

[GitHub] twill pull request #2: TWILL-190 Wait for Twill runnables to stop when resta...

Github user poornachandra commented on a diff in the pull request: https://github.com/apache/twill/pull/2#discussion_r75572116 --- Diff: twill-yarn/src/main/java/org/apache/twill/internal/appmaster/ApplicationMasterService.java --- @@ -846,24 +875,31 @@ private boolean

[GitHub] twill pull request #2: TWILL-190 Wait for Twill runnables to stop when resta...

Github user poornachandra commented on a diff in the pull request: https://github.com/apache/twill/pull/2#discussion_r75572272 --- Diff: twill-core/src/main/java/org/apache/twill/internal/TwillContainerLauncher.java --- @@ -177,7 +188,24 @@ protected void doStartUp

[GitHub] twill pull request #2: TWILL-190 Wait for Twill runnables to stop when resta...

Github user poornachandra commented on a diff in the pull request: https://github.com/apache/twill/pull/2#discussion_r75572275 --- Diff: twill-core/src/main/java/org/apache/twill/internal/TwillContainerLauncher.java --- @@ -225,5 +257,35 @@ public ContainerLiveNodeData

[GitHub] twill issue #2: TWILL-190 Wait for Twill runnables to stop when restarting t...

Github user poornachandra commented on the issue: https://github.com/apache/twill/pull/2 @chtyim Except for moving the kill loop to `ProcessController.cancel()` method, I have addressed all other comments. --- If your project is set up for it, you can reply to this email and have

[GitHub] twill pull request #4: TWILL-190 Wait for Twill runnables to stop when resta...

GitHub user poornachandra opened a pull request: https://github.com/apache/twill/pull/4 TWILL-190 Wait for Twill runnables to stop when restarting them Also kill non-responding Twill runnables after a timeout. JIRA - https://issues.apache.org/jira/browse/TWILL-190

[GitHub] twill issue #5: Fix the LocationTest unit tests

Github user poornachandra commented on the issue: https://github.com/apache/twill/pull/5 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

[GitHub] twill issue #11: Fix a ClassCastException in YarnUtils.addDelegationTokens.

Github user poornachandra commented on the issue: https://github.com/apache/twill/pull/11 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

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

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

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

Github user poornachandra commented on a diff in the pull request: https://github.com/apache/twill/pull/26#discussion_r97683616 --- 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)

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

[GitHub] twill pull request #23: (TWILL-181) allow setting the maximum number of retr...

Github user poornachandra commented on a diff in the pull request: https://github.com/apache/twill/pull/23#discussion_r97944230 --- Diff: twill-yarn/src/main/java/org/apache/twill/internal/appmaster/RunningContainers.java --- @@ -113,9 +117,11 @@ public Integer apply(BitSet input

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

Github user poornachandra commented on a diff in the pull request: https://github.com/apache/twill/pull/26#discussion_r98070266 --- 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 #23: (TWILL-181) allow setting the maximum number of retr...

Github user poornachandra commented on a diff in the pull request: https://github.com/apache/twill/pull/23#discussion_r98075925 --- Diff: twill-yarn/src/main/java/org/apache/twill/internal/appmaster/RunningContainers.java --- @@ -113,9 +117,11 @@ public Integer apply(BitSet input

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

Github user poornachandra commented on the issue: https://github.com/apache/twill/pull/26 Just one comment on javadoc, please fix and merge 👍 --- 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

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

Github user poornachandra commented on the issue: https://github.com/apache/twill/pull/26 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

[GitHub] twill pull request #23: (TWILL-181) allow setting the maximum number of retr...

Github user poornachandra commented on a diff in the pull request: https://github.com/apache/twill/pull/23#discussion_r98088342 --- Diff: twill-yarn/src/main/java/org/apache/twill/internal/appmaster/RunningContainers.java --- @@ -113,9 +117,11 @@ public Integer apply(BitSet input

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

Github user poornachandra commented on the issue: https://github.com/apache/twill/pull/26 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

[GitHub] twill pull request #23: (TWILL-181) allow setting the maximum number of retr...

Github user poornachandra commented on a diff in the pull request: https://github.com/apache/twill/pull/23#discussion_r98337486 --- Diff: twill-yarn/src/main/java/org/apache/twill/internal/appmaster/RunningContainers.java --- @@ -113,9 +117,11 @@ public Integer apply(BitSet input

[GitHub] twill issue #29: (TWILL-211) use ALLOCATE_ONE_INSTANCE_AT_A_TIME for retries...

Github user poornachandra commented on the issue: https://github.com/apache/twill/pull/29 LGTM from me too --- 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