[GitHub] thrift issue #1191: Thrift 3706 - Implement multiplexor.

2017-02-14 Thread jeking3
Github user jeking3 commented on the issue: https://github.com/apache/thrift/pull/1191 What testing have you done to verify this code (I have to ask because there is no multiplexed protocol testing being done in crosstest currently)? --- If your project is set up for it, you can repl

[GitHub] thrift issue #1191: Thrift 3706 - Implement multiplexor.

2017-02-14 Thread jeking3
Github user jeking3 commented on the issue: https://github.com/apache/thrift/pull/1191 In the future please format your pull request title to start with THRIFT-3706 so that the bots link the pull request to the open Jira item. Thanks! --- If your project is set up for it, you can r

[GitHub] thrift issue #1191: Thrift 3706 - Implement multiplexor.

2017-02-14 Thread gadLinux
Github user gadLinux commented on the issue: https://github.com/apache/thrift/pull/1191 Hi Jeking3, You are right! I forgot this time to do the proper naming of the merge request. Yesterday I was thinking about the cross test. I will try to add a cross test for Java vs

[GitHub] thrift issue #1191: THRIFT-3706 - Implement multiplexor.

2017-02-15 Thread gadLinux
Github user gadLinux commented on the issue: https://github.com/apache/thrift/pull/1191 @jeking3 Please explain how this can happen. Just did a rebase and merged my changes that only adds few files (I think two or three). If master is so controlled that no build failures can go in (yo

[GitHub] thrift issue #1191: THRIFT-3706 - Implement multiplexor.

2017-02-15 Thread gadLinux
Github user gadLinux commented on the issue: https://github.com/apache/thrift/pull/1191 I added some cross tests against Java. Maybe others can add the rest when the protocol is ready for the other languages. --- If your project is set up for it, you can reply to this email and have

[GitHub] thrift issue #1191: THRIFT-3706 - Implement multiplexor.

2017-02-15 Thread gadLinux
Github user gadLinux commented on the issue: https://github.com/apache/thrift/pull/1191 What do I have to do to be a contributor? --- 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

[GitHub] thrift issue #1191: THRIFT-3706 - Implement multiplexor.

2017-02-15 Thread jeking3
Github user jeking3 commented on the issue: https://github.com/apache/thrift/pull/1191 I'm a committer on the project which means I have write access to the apache repository for the project. Most changes come in like this, and one of the committers need to merge it into master. --

[GitHub] thrift issue #1191: THRIFT-3706 - Implement multiplexor.

2017-02-16 Thread gadLinux
Github user gadLinux commented on the issue: https://github.com/apache/thrift/pull/1191 I must say that like the SSL patch this is already tested in production. Java vs c_glib. We have to add support for the Server c_glib and Client Java. But I suppose this time it will work without p

[GitHub] thrift issue #1191: THRIFT-3706 - Implement multiplexor.

2017-02-18 Thread jeking3
Github user jeking3 commented on the issue: https://github.com/apache/thrift/pull/1191 I added "multiplexed" support to make cross in Travis CI if you rebase on master. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If

[GitHub] thrift issue #1191: THRIFT-3706 - Implement multiplexor.

2017-02-18 Thread gadLinux
Github user gadLinux commented on the issue: https://github.com/apache/thrift/pull/1191 Hi Jeking3, Why don't you merge it and add a new issue for others to add multiplexing protocol? --- If your project is set up for it, you can reply to this email and have your reply appear on

[GitHub] thrift issue #1191: THRIFT-3706 - Implement multiplexor.

2017-02-18 Thread jeking3
Github user jeking3 commented on the issue: https://github.com/apache/thrift/pull/1191 I'm not going to merge it until it passes make cross with multiplexed protocol. There's nothing in the pull request and the build system that proves it works and will keep working. I am working on

[GitHub] thrift issue #1191: THRIFT-3706 - Implement multiplexor.

2017-02-20 Thread gadLinux
Github user gadLinux commented on the issue: https://github.com/apache/thrift/pull/1191 Did you see that the test is working against Java? If you try a connection with and without multiplexed in one of the ends the connection doesn't work. The only way to see if the data is sent corre

[GitHub] thrift issue #1191: THRIFT-3706 - Implement multiplexor.

2017-02-20 Thread jeking3
Github user jeking3 commented on the issue: https://github.com/apache/thrift/pull/1191 Discussing with folks who created crosstest what we should do. My preference is to have a single "multiplexed" protocol test that uses the binary protocol. That will be the most widely used combo,

[GitHub] thrift issue #1191: THRIFT-3706 - Implement multiplexor.

2017-02-20 Thread jeking3
Github user jeking3 commented on the issue: https://github.com/apache/thrift/pull/1191 Well I was hoping the multiplexed implementation was compatible with older clients using binary protocol but it is not. I'm going to send you a PR that changes the Java Server and the c_glib client

[GitHub] thrift issue #1191: THRIFT-3706 - Implement multiplexor.

2017-02-20 Thread jeking3
Github user jeking3 commented on the issue: https://github.com/apache/thrift/pull/1191 @gadLinux for some reason I cannot send you a PR for this one like I had before, possibly because I rebased on master. If you cherry-pick b2e4e1464b74733ccf78afd68abbe94b92aec96a from my repository

[GitHub] thrift issue #1191: THRIFT-3706 - Implement multiplexor.

2017-02-20 Thread gadLinux
Github user gadLinux commented on the issue: https://github.com/apache/thrift/pull/1191 I cannot do it since this commit is in thrift main outside master. So I cannot see it. You cannot merge because this branch is still not merged so it's still in my fork. You can merge and then appl

[GitHub] thrift issue #1191: THRIFT-3706 - Implement multiplexor.

2017-02-20 Thread jeking3
Github user jeking3 commented on the issue: https://github.com/apache/thrift/pull/1191 Weird, I'm not exactly sure what happened here, but I submitted a new PR rebased against master incorporating your changes here and I added one more commit - see PR #1199. If that passes I'll squas

[GitHub] thrift issue #1191: THRIFT-3706 - Implement multiplexor.

2017-10-15 Thread gadLinux
Github user gadLinux commented on the issue: https://github.com/apache/thrift/pull/1191 Jus great, thank you! ---