You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@thrift.apache.org by vh <gi...@git.apache.org> on 2016/08/02 13:55:07 UTC
[GitHub] thrift pull request #1058: Node.js: Set/unset client seqid for json_protocol...
GitHub user vh opened a pull request:
https://github.com/apache/thrift/pull/1058
Node.js: Set/unset client seqid for json_protocol and compact_protocol
Like binary_protocol does.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/vh/thrift master
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/thrift/pull/1058.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 #1058
----
commit bfa8f2db023491add792252d179ce55bbbf433d8
Author: Vadim Homchik <ho...@gmail.com>
Date: 2016-08-02T13:47:47Z
record client seqid as in the binary_protocol to find callback
commit a5ff4e1f8edcc1b249d9b8000229fffc23384174
Author: Vadim Homchik <ho...@gmail.com>
Date: 2016-08-02T13:48:57Z
unset seqid on writeMessageEnd
----
---
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 issue #1058: Node.js: Set/unset client seqid for json_protocol and co...
Posted by jeking3 <gi...@git.apache.org>.
Github user jeking3 commented on the issue:
https://github.com/apache/thrift/pull/1058
This change needs a Jira ticket; please see https://thrift.apache.org/docs/HowToContribute. Also if you can rebase on master and push that will kick a new CI build; CI needs to pass (or failures easily explained) to merge.
---
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 issue #1058: Node.js: Set/unset client seqid for json_protocol and co...
Posted by jeking3 <gi...@git.apache.org>.
Github user jeking3 commented on the issue:
https://github.com/apache/thrift/pull/1058
In general I want to make sure all clients send a seqid and all servers put that seqid in the result. Without this we won't be able to have clients that can have multiple outstanding requests in the future.
---
[GitHub] thrift issue #1058: Node.js: Set/unset client seqid for json_protocol and co...
Posted by jeking3 <gi...@git.apache.org>.
Github user jeking3 commented on the issue:
https://github.com/apache/thrift/pull/1058
@bananer is this worth pulling forward if it passes a build?
---
[GitHub] thrift issue #1058: Node.js: Set/unset client seqid for json_protocol and co...
Posted by jeking3 <gi...@git.apache.org>.
Github user jeking3 commented on the issue:
https://github.com/apache/thrift/pull/1058
This change needs a Jira ticket; please see https://thrift.apache.org/docs/HowToContribute. Also if you can rebase on master and push that will kick a new CI build; CI needs to pass (or failures easily explained) to merge.
---
[GitHub] thrift issue #1058: Node.js: Set/unset client seqid for json_protocol and co...
Posted by bananer <gi...@git.apache.org>.
Github user bananer commented on the issue:
https://github.com/apache/thrift/pull/1058
@jeking3 To be honest, I don't know why this is useful/necessary. I think there is no need for a seqid because the connection between request and response is handled through callbacks automatically.
---