You are viewing a plain text version of this content. The canonical link for it is here.

Posted to dev@thrift.apache.org by trotterdylan <gi...@git.apache.org> on 2017/11/07 17:56:46 UTC

[GitHub] thrift pull request #1411: Fix remote client for HTTP transport

GitHub user trotterdylan opened a pull request:

    https://github.com/apache/thrift/pull/1411

    Fix remote client for HTTP transport

    This change fixes two issues:
    
    - Assign parsedUrl to the variable in the outer scope instead of creating a new one. Previously the outer parsedUrl was never assigned and was therefore always empty.
    - Create a POST client using NewTHttpPostClient instead of NewTHttpClient since the latter has no requestBuffer to write to the request body.

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

    $ git pull https://github.com/trotterdylan/thrift patch-1

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

    https://github.com/apache/thrift/pull/1411.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 #1411
    
----
commit acbc50f5a3ab559cdb0937986709388350282d87
Author: Dylan Trotter <tr...@users.noreply.github.com>
Date:   2017-11-07T17:56:17Z

    Fix remote client for HTTP transport
    
    This change fixes two issues:
    
    - Assign parsedUrl to the variable in the outer scope instead of creating a new one. Previously the outer parsedUrl was never assigned and was therefore always empty.
    - Create a POST client using NewTHttpPostClient instead of NewTHttpClient since the latter has no requestBuffer to write to the request body.

----


---

[GitHub] thrift issue #1411: Fix remote client for HTTP transport

Posted by dcelasun <gi...@git.apache.org>.

Github user dcelasun commented on the issue:

    https://github.com/apache/thrift/pull/1411
  
    @jeking3 LGTM


---

[GitHub] thrift issue #1411: Fix remote client for HTTP transport

Posted by jeking3 <gi...@git.apache.org>.

Github user jeking3 commented on the issue:

    https://github.com/apache/thrift/pull/1411
  
    @dcelasun can you approve if these changes look good to you?


---

[GitHub] thrift issue #1411: Fix remote client for HTTP transport

Posted by dcelasun <gi...@git.apache.org>.

Github user dcelasun commented on the issue:

    https://github.com/apache/thrift/pull/1411
  
    Please revert the `NewTHttpClient` part of this PR, since `THttpPostClient` is deprecated and is just an alias for `THttpClient` since 0dd82358.


---

[GitHub] thrift pull request #1411: Fix remote client for HTTP transport

Posted by asfgit <gi...@git.apache.org>.

Github user asfgit closed the pull request at:

    https://github.com/apache/thrift/pull/1411


---

[GitHub] thrift issue #1411: Fix remote client for HTTP transport

Posted by trotterdylan <gi...@git.apache.org>.

Github user trotterdylan commented on the issue:

    https://github.com/apache/thrift/pull/1411
  
    Done. Updated the commit message accordingly.


---

[GitHub] thrift issue #1411: Fix remote client for HTTP transport

Posted by jeking3 <gi...@git.apache.org>.

Github user jeking3 commented on the issue:

    https://github.com/apache/thrift/pull/1411
  
    He @trotterdylan please review https://thrift.apache.org/docs/HowToContribute as we need a Jira ticket for this change.


---

[GitHub] thrift issue #1411: Fix remote client for HTTP transport

Posted by trotterdylan <gi...@git.apache.org>.

Github user trotterdylan commented on the issue:

    https://github.com/apache/thrift/pull/1411
  
    Done! https://issues.apache.org/jira/browse/THRIFT-4385


---

[GitHub] thrift issue #1411: Fix remote client for HTTP transport

Posted by jeking3 <gi...@git.apache.org>.

Github user jeking3 commented on the issue:

    https://github.com/apache/thrift/pull/1411
  
    Thanks - once the CI builds pass I will re-review.


---