You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@twill.apache.org by chtyim <gi...@git.apache.org> on 2017/10/10 20:26:48 UTC

[GitHub] twill pull request #62: (TWILL-248) Upgrade to use Netty-4.1

GitHub user chtyim opened a pull request:

    https://github.com/apache/twill/pull/62

    (TWILL-248) Upgrade to use Netty-4.1

    - Also enable ResourceReportClient to use HTTP compression

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/chtyim/twill feature/TWILL-248-upgrade-netty-4.1

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/twill/pull/62.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #62
    
----
commit 321e7ba0116ccb7ba1c9b814ff60ad0dfd4ac3e5
Author: Terence Yim <ch...@apache.org>
Date:   2017-10-10T20:26:11Z

    (TWILL-248) Upgrade to use Netty-4.1
    
    - Also enable ResourceReportClient to use HTTP compression

----


---

[GitHub] twill pull request #62: (TWILL-248) Upgrade to use Netty-4.1

Posted by chtyim <gi...@git.apache.org>.
Github user chtyim commented on a diff in the pull request:

    https://github.com/apache/twill/pull/62#discussion_r143854276
  
    --- Diff: twill-yarn/src/main/java/org/apache/twill/yarn/ResourceReportClient.java ---
    @@ -52,12 +54,16 @@
       public ResourceReport get() {
         for (URL url : resourceUrls) {
           try {
    -        Reader reader = new BufferedReader(new InputStreamReader(url.openStream(), Charsets.UTF_8));
    -        try {
    +        HttpURLConnection urlConn = (HttpURLConnection) url.openConnection();
    --- End diff --
    
    Not entirely. This is to enable compression when fetching the report.


---

[GitHub] twill pull request #62: (TWILL-248) Upgrade to use Netty-4.1

Posted by hsaputra <gi...@git.apache.org>.
Github user hsaputra commented on a diff in the pull request:

    https://github.com/apache/twill/pull/62#discussion_r143848112
  
    --- Diff: twill-yarn/src/main/java/org/apache/twill/yarn/ResourceReportClient.java ---
    @@ -52,12 +54,16 @@
       public ResourceReport get() {
         for (URL url : resourceUrls) {
           try {
    -        Reader reader = new BufferedReader(new InputStreamReader(url.openStream(), Charsets.UTF_8));
    -        try {
    +        HttpURLConnection urlConn = (HttpURLConnection) url.openConnection();
    --- End diff --
    
    Is this change related to Netty upgrade?


---

[GitHub] twill pull request #62: (TWILL-248) Upgrade to use Netty-4.1

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/twill/pull/62


---

[GitHub] twill issue #62: (TWILL-248) Upgrade to use Netty-4.1

Posted by hsaputra <gi...@git.apache.org>.
Github user hsaputra commented on the issue:

    https://github.com/apache/twill/pull/62
  
    One question about change form using BufferedReader,  but looks good.
    
    If passes tests +1


---