[GitHub] twill pull request #65: (TWILL-254) Update to use ContainerId.fromString

2018-02-04 Thread cbaenziger
Github user cbaenziger commented on a diff in the pull request: https://github.com/apache/twill/pull/65#discussion_r165862673 --- Diff: pom.xml --- @@ -680,9 +680,9 @@ -hadoop-2.5 +hadoop-2.6

[GitHub] twill pull request #65: (TWILL-254) Update to use ContainerId.fromString

2018-02-04 Thread cbaenziger
Github user cbaenziger commented on a diff in the pull request: https://github.com/apache/twill/pull/65#discussion_r165862857 --- Diff: twill-yarn/src/main/hadoop20/org/apache/twill/internal/yarn/Hadoop20YarnAMClient.java --- @@ -71,6 +73,11 @@ public Hadoop20YarnAMClient

[GitHub] twill issue #65: (TWILL-254) Update to use ContainerId.fromString

2018-02-04 Thread cbaenziger
Github user cbaenziger commented on the issue: https://github.com/apache/twill/pull/65 I added a few omissions and corrected the parent `pom.xml` from my original submission. Now the new code is actually being used and I have updated the default target to Hadoop 2.6 and removed

[GitHub] twill pull request #65: (TWILL-254) Update to use ContainerId.fromString

2018-02-04 Thread cbaenziger
Github user cbaenziger commented on a diff in the pull request: https://github.com/apache/twill/pull/65#discussion_r165862926 --- Diff: twill-yarn/src/main/hadoop22/org/apache/twill/internal/yarn/Hadoop22YarnAMClient.java --- @@ -26,7 +26,7 @@ /** * Wrapper class

[GitHub] twill issue #65: (TWILL-254) Update to use ContainerId.fromString

2018-02-04 Thread cbaenziger
Github user cbaenziger commented on the issue: https://github.com/apache/twill/pull/65 Thank you for the super fast code review @serranom! I think all comments have been addressed and the code should be complete now. Please let me know if you see anything else necessary. It is great

[GitHub] twill pull request #65: (TWILL-254) Update to use ContainerId.fromString

2018-02-03 Thread cbaenziger
GitHub user cbaenziger opened a pull request: https://github.com/apache/twill/pull/65 (TWILL-254) Update to use ContainerId.fromString This moves away from the deprecated `ConverterUitls.toContainerId` and updates the build to use Hadoop 2.7.2 to avoid folks grabbing master

[GitHub] twill issue #65: (TWILL-254) Update to use ContainerId.fromString

2018-02-05 Thread cbaenziger
Github user cbaenziger commented on the issue: https://github.com/apache/twill/pull/65 Hi @chtyim, thank you for the review. Could you please expand on what you see concerning in the `pom.xml`? For reference, I need to provide a 2.6 target as it is the first version to provide

[GitHub] twill pull request #65: (TWILL-254) Update to use ContainerId.fromString

2018-02-05 Thread cbaenziger
Github user cbaenziger commented on a diff in the pull request: https://github.com/apache/twill/pull/65#discussion_r166133525 --- Diff: pom.xml --- @@ -680,9 +680,9 @@ -hadoop-2.5 +hadoop-2.6