You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@thrift.apache.org by hhkbp2 <gi...@git.apache.org> on 2015/07/21 10:30:31 UTC

[GitHub] thrift pull request: Thrift 3251 Add http transport for server to ...

GitHub user hhkbp2 opened a pull request:

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

    Thrift 3251 Add http transport for server to Go lib

    Hi,
    
    This PR is a reopen for previous PR #556, in order to follow our how to contribute guide.
    
    It adds http transport for server to Go lib.
    For more info please refer to Jira
    [https://issues.apache.org/jira/browse/THRIFT-3251](https://issues.apache.org/jira/browse/THRIFT-3251)

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

    $ git pull https://github.com/hhkbp2/thrift THRIFT-3251

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

    https://github.com/apache/thrift/pull/557.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 #557
    
----
commit ebb6d04f1f59818a19525123d32bd64f163063fe
Author: Dylan.Wen <hh...@gmail.com>
Date:   2015-07-07T10:25:14Z

    add http transport implementation for golang server

commit 6c318e50adba846d9d0a0794a0f2d217ecdcb507
Author: Dylan.Wen <hh...@gmail.com>
Date:   2015-07-08T06:20:34Z

    add test for golang http transport

commit b1e4bb7db9af82bd078cfaaf67df27a3fb72e108
Author: Dylan.Wen <hh...@gmail.com>
Date:   2015-07-08T08:38:39Z

    simplify the used interfaces a little bit

----


---
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 is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] thrift pull request: Thrift 3251 Add http transport for server to ...

Posted by dimiro1 <gi...@git.apache.org>.
Github user dimiro1 commented on the pull request:

    https://github.com/apache/thrift/pull/557#issuecomment-170362506
  
    @Jens-G The current test infrastructure for the go lib explicitly assume that the server is a SocketServer /test/go/src/server.go. I think it will be a lot of work write the test to conform with the current structure. In my gist [https://gist.github.com/dimiro1/d38aaf4b7d84c2d5c8f0] I wrote a simple test based on a simple Calculator client. What do you think about this approach? Do have any other idea? I am open to suggestions.


---
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 is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] thrift pull request #557: Thrift 3251 Add http transport for server to Go li...

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

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


---
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 is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] thrift pull request: Thrift 3251 Add http transport for server to ...

Posted by Jens-G <gi...@git.apache.org>.
Github user Jens-G commented on the pull request:

    https://github.com/apache/thrift/pull/557#issuecomment-170364590
  
    All languages are encouraged 1) to implement the standard Thrift test. This test is the basic test to ensure interoperability across languages for all features implemented. So if we have a http server, that thing should be testable. The [command line syntax is documented](http://thrift.apache.org/test/#test-executable-specification) and works the same way for all languages.
    
    So yes, it would be good to have this integrated into the ThriftTest server and controlled via the ```--transport http(s)``` parameter. The client already supports it, [the server throws an error right now](https://github.com/apache/thrift/blob/master/test/go/src/common/server.go#L92).
    ____
    1) one could also say "required" and it would not be wrong 


---
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 is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] thrift pull request: Thrift 3251 Add http transport for server to ...

Posted by Jens-G <gi...@git.apache.org>.
Github user Jens-G commented on the pull request:

    https://github.com/apache/thrift/pull/557#issuecomment-170335304
  
    @dimiro1  could you please make a PR out of it?
    http://thrift.apache.org/docs/HowToContribute



---
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 is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] thrift pull request: Thrift 3251 Add http transport for server to ...

Posted by dimiro1 <gi...@git.apache.org>.
Github user dimiro1 commented on the pull request:

    https://github.com/apache/thrift/pull/557#issuecomment-170301847
  
    I implemented with a handler func. https://gist.github.com/dimiro1/d38aaf4b7d84c2d5c8f0


---
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 is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---