[GitHub] thrift pull request: THRIFT-2878 - Go validation support of requir...
Github user cvlchinet commented on the pull request: https://github.com/apache/thrift/pull/304#issuecomment-69471686 Rebased and changed to use OptionalRequiredTest.thrift. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] thrift pull request: THRIFT-2844: Nodejs support broken when runni...
Github user cvlchinet commented on the pull request: https://github.com/apache/thrift/pull/251#issuecomment-65298557 Usually you should be the one, with closing rights. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] thrift pull request: THRIFT-2866 - Fix readability in Go generator...
Github user cvlchinet commented on the pull request: https://github.com/apache/thrift/pull/295#issuecomment-65059473 I did that. I thought I had all the dependencies installed. But now i figured that I need clang-format right? ok I am going to work on that. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] thrift pull request: THRIFT-2866 - Fix readability in Go generator...
Github user cvlchinet commented on the pull request: https://github.com/apache/thrift/pull/295#issuecomment-65095045 Wow ok now it works and yes it reverts everything. I'm going to change this, so it works with clang-format. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] thrift pull request: THRIFT-2866 - Fix readability in Go generator...
Github user cvlchinet commented on the pull request: https://github.com/apache/thrift/pull/295#issuecomment-65110610 Thanks hcorg. I changed it now and ran the code with code-format. Rebased on current 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 your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] thrift pull request: THRIFT-2865 - Test case for Go: SeqId out of ...
Github user cvlchinet closed the pull request at: https://github.com/apache/thrift/pull/294 --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] thrift pull request: THRIFT-2865 - Test case for Go: SeqId out of ...
Github user cvlchinet commented on the pull request: https://github.com/apache/thrift/pull/294#issuecomment-65127998 I closed this PR and will provide a more comprehensive PR that cover more cases. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] thrift pull request: Enhance error handling for the Go client
GitHub user cvlchinet opened a pull request: https://github.com/apache/thrift/pull/297 Enhance error handling for the Go client This PR enhances the Go client error handling by the following: - Check if method name is correct - if not return thrift.WRONG_METHOD_NAME - Check if MessageType is thrift.REPLY or EXCEPTION - if not return thrift.INVALID_MESSAGE_TYPE_EXCEPTION - Checking the sequence id is done before checking the message type Includes test cases for every error case. You can merge this pull request into a Git repository by running: $ git pull https://github.com/cvlchinet/thrift go-enhance-client-eh2 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/thrift/pull/297.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 #297 commit 851923612feafc802ead03565316d0d57c3b1242 Author: Chi Vinh Le c...@chinet.info Date: 2014-12-01T19:14:31Z Enhance error handling for the Go client --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] thrift pull request: THRIFT-2865 - Test case for Go: SeqId out of ...
Github user cvlchinet commented on the pull request: https://github.com/apache/thrift/pull/294#issuecomment-65130129 See https://github.com/apache/thrift/pull/297 --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] thrift pull request: Test case for Go: SeqId out of sequence
GitHub user cvlchinet opened a pull request: https://github.com/apache/thrift/pull/294 Test case for Go: SeqId out of sequence This adds a test case to the Go library to test the error handling behavior when returned sequence id was out of order. You can merge this pull request into a Git repository by running: $ git pull https://github.com/cvlchinet/thrift go-test-seqid Alternatively you can review and apply these changes as the patch at: https://github.com/apache/thrift/pull/294.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 #294 commit 6af0732d48ff02b085bcc66057b603fdbb5eedbb Author: Chi Vinh Le c...@chinet.info Date: 2014-11-29T13:07:18Z SeqId out of sequence error handling test case --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] thrift pull request: Fix readability in Go generator
GitHub user cvlchinet opened a pull request: https://github.com/apache/thrift/pull/295 Fix readability in Go generator Because of auto-formatting the source code from https://issues.apache.org/jira/browse/THRIFT-2729 the source writing part of the Go generator is cluttered and barely readable. This PR sets line breaks in appropriate positions to make it a lot more readable and maintainable. You can merge this pull request into a Git repository by running: $ git pull https://github.com/cvlchinet/thrift go-fix-readability Alternatively you can review and apply these changes as the patch at: https://github.com/apache/thrift/pull/295.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 #295 commit 4fed6698a9df36c99dbb5468835d1cf9467a10ad Author: Chi Vinh Le c...@chinet.info Date: 2014-11-29T13:39:10Z Increase readability in go generator --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] thrift pull request: THRIFT-2854 Go Struct writer and reader loose...
GitHub user cvlchinet opened a pull request: https://github.com/apache/thrift/pull/291 THRIFT-2854 Go Struct writer and reader looses important error information This PR fixes https://issues.apache.org/jira/browse/THRIFT-2854. The included test will cover the complete client error handling code except the check for the Sequence ID. (unless I miss something) I will submit a follow up PR to test the error handling for the Sequence ID too once this PR has been merged. The covered code makes up the majority of the server error handling code too. And yes.. it was not fun too code the test.. :P You can merge this pull request into a Git repository by running: $ git pull https://github.com/cvlchinet/thrift go-struct-error Alternatively you can review and apply these changes as the patch at: https://github.com/apache/thrift/pull/291.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 #291 commit 18d2d2b58a1dd3ed256fe189bc9b8a5a77f5bcc5 Author: Chi Vinh Le c...@chinet.info Date: 2014-11-26T23:19:17Z Fixes error reporting in go generator --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] thrift pull request: Remove strange public Peek() from GO transpor...
GitHub user cvlchinet opened a pull request: https://github.com/apache/thrift/pull/283 Remove strange public Peek() from GO transports I've been seeing this public Peek() function in the GO library for a while, but still cannot figure out any sense to it. If there are useless, we can remove them right? This PR removes the public Peek() from the implemented transports. All tests still pass. You can merge this pull request into a Git repository by running: $ git pull https://github.com/cvlchinet/thrift go-peek Alternatively you can review and apply these changes as the patch at: https://github.com/apache/thrift/pull/283.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 #283 commit ac64c3395beaa2fbc6576fabfd3835d90b6737ff Author: Chi Vinh Le c...@chinet.info Date: 2014-11-24T18:44:16Z Remove public Peek fom transports --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] thrift pull request: Better open/close behavior for StreamTranspor...
GitHub user cvlchinet opened a pull request: https://github.com/apache/thrift/pull/285 Better open/close behavior for StreamTransport. I use StreamTransport a lot to test functionality locally that otherwise uses a Socket transport. Current implementation of IsOpen returns always true, even though after a close, the whole transport is not usable. This PR implements IsOpen, Open and Close that reflects the internal state and behaves more similar to other transports like TSocket. Includes a test case. You can merge this pull request into a Git repository by running: $ git pull https://github.com/cvlchinet/thrift go-closable Alternatively you can review and apply these changes as the patch at: https://github.com/apache/thrift/pull/285.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 #285 commit 1592475aa369a35e93a45f09c154ae2574e4cbd5 Author: Chi Vinh Le c...@chinet.info Date: 2014-11-24T21:13:20Z Better open/close behavior for StreamTransport. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] thrift pull request: THRIFT-2853: Remove comments that doesn't app...
GitHub user cvlchinet opened a pull request: https://github.com/apache/thrift/pull/286 THRIFT-2853: Remove comments that doesn't apply anymore because of THRIFT-2852 THRIFT-2852 changed behavior of iostream_transport.go which made some comments invalid. This PR removes the remaining comments. You can merge this pull request into a Git repository by running: $ git pull https://github.com/cvlchinet/thrift go-comment Alternatively you can review and apply these changes as the patch at: https://github.com/apache/thrift/pull/286.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 #286 commit 11774ee72d962533f1ea8585eddbba558d57eb3e Author: Chi Vinh Le c...@chinet.info Date: 2014-11-24T22:07:28Z Remove comments that doesn't apply anymore. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] thrift pull request: THRIFT-2844: Nodejs support broken when runni...
Github user cvlchinet commented on the pull request: https://github.com/apache/thrift/pull/251#issuecomment-63784793 Ok thanks for clarifying :-) --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] thrift pull request: Fixes THRIFT-2817 and THRIFT-2839
Github user cvlchinet commented on the pull request: https://github.com/apache/thrift/pull/273#issuecomment-63554886 Commited. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] thrift pull request: THRIFT-2831 - Removes dead code in web_server...
Github user cvlchinet commented on the pull request: https://github.com/apache/thrift/pull/270#issuecomment-63554942 Commited. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] thrift pull request: Fixes THRIFT-2817 and THRIFT-2839
Github user cvlchinet closed the pull request at: https://github.com/apache/thrift/pull/273 --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] thrift pull request: THRIFT-2831 - Removes dead code in web_server...
Github user cvlchinet closed the pull request at: https://github.com/apache/thrift/pull/270 --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] thrift pull request: add support for running under Browserify
Github user cvlchinet commented on the pull request: https://github.com/apache/thrift/pull/251#issuecomment-63556192 Did you double check that data really comes as an Uint8Array and not a plain ArrayBuffer? And to get more attention and to follow their contribution rules, you should file an issue in JIRA: https://issues.apache.org/jira/browse/THRIFT/ --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] thrift pull request: Update simple_json_protocol.go
Github user cvlchinet commented on the pull request: https://github.com/apache/thrift/pull/134#issuecomment-63557119 This was fixed in https://github.com/apache/thrift/pull/273. Can be closed. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] thrift pull request: Fixes THRIFT-2817 and THRIFT-2839
GitHub user cvlchinet opened a pull request: https://github.com/apache/thrift/pull/273 Fixes THRIFT-2817 and THRIFT-2839 Current implementation of json protocols uses buffer peeks extensively. But in certain situations this causes the buffer to read beyond the json message, which stalls the whole connection. (THRIFT-2817) To test this fix I ported the integration tests from the nodejs thrift library to go. Those are comprehensive integration tests that tests the whole thrift stack. The tests revealed another problem in TFramedTransport. When calling TFramedTransport.Read, it should return remaining bytes, but it doesn't when passing a []byte that is bigger than the current frame_size. (THRIFT-2839) This PR fixes THRIFT-2817 and THRIFT-2839. You can merge this pull request into a Git repository by running: $ git pull https://github.com/cvlchinet/thrift go-fix-json Alternatively you can review and apply these changes as the patch at: https://github.com/apache/thrift/pull/273.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 #273 commit 550bb2d92cd7eb256a96d435cbc1f69437fe902b Author: Chi Vinh Le c...@chinet.info Date: 2014-11-17T22:19:04Z Smarter buffer peeking for json protocols commit 7f16c1dda2405029bf64e99312852d074d44 Author: Chi Vinh Le c...@chinet.info Date: 2014-11-17T22:19:42Z Fix bug in TFramedTransport commit 7710d32ee10136365f0cbe65c28e6565298f6b60 Author: Chi Vinh Le c...@chinet.info Date: 2014-11-17T22:20:25Z Add comprehensive integration tests for the whole stack --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] thrift pull request: THRIFT-2817 - TSimpleJSONProtocol reads beyon...
Github user cvlchinet closed the pull request at: https://github.com/apache/thrift/pull/264 --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] thrift pull request: THRIFT-2817 - TSimpleJSONProtocol reads beyon...
Github user cvlchinet commented on the pull request: https://github.com/apache/thrift/pull/264#issuecomment-63389816 New PR. See: https://github.com/apache/thrift/pull/273 --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] thrift pull request: THRIFT-2819 - Add WebsSocket client to node.j...
Github user cvlchinet closed the pull request at: https://github.com/apache/thrift/pull/265 --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] thrift pull request: THRIFT-2831 - Removes dead code in web_server...
GitHub user cvlchinet opened a pull request: https://github.com/apache/thrift/pull/270 THRIFT-2831 - Removes dead code in web_server.js introduced in THRIFT-2819 The commit https://issues.apache.org/jira/browse/THRIFT-2819 introduced dead code in web_server.js. Sorry it was introduced by me. The web_server.js was working totally fine even in binary cases. My code actually does nothing. This pull request will remove my changes. What was I thinking.. ;) You can merge this pull request into a Git repository by running: $ git pull https://github.com/cvlchinet/thrift master Alternatively you can review and apply these changes as the patch at: https://github.com/apache/thrift/pull/270.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 #270 commit dfe4653441496d4880183daae94eb32f933dac87 Author: Chi Vinh Le c...@chinet.info Date: 2014-11-16T22:23:50Z Undo changes in web_server.. it was working perfectly --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] thrift pull request: THRIFT-2819 - Add WebsSocket client to node.j...
Github user cvlchinet commented on the pull request: https://github.com/apache/thrift/pull/265#issuecomment-63179959 I added binary support to client and to the web server. Now all integration tests pass. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] thrift pull request: THRIFT-2817 - TSimpleJSONProtocol reads beyon...
Github user cvlchinet commented on the pull request: https://github.com/apache/thrift/pull/264#issuecomment-63180271 Any build files to create the dependencies? Or do I have to do it by hand? --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] thrift pull request: Remove trailing commas of arrays
GitHub user cvlchinet opened a pull request: https://github.com/apache/thrift/pull/263 Remove trailing commas of arrays One of the obstacles to allow thrift to be able to run in IE8 You can merge this pull request into a Git repository by running: $ git pull https://github.com/cvlchinet/thrift patch-1 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/thrift/pull/263.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 #263 commit b2fab3e5599dded8b03227ca36076dace3936999 Author: cvlchinet c...@chinet.info Date: 2014-11-13T16:30:46Z Remove trailing commas of arrays One of the obstacles to allow thrift to be able to run in IE8 --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] thrift pull request: Fix TSimpleJSONProtocol reading beyond messag...
GitHub user cvlchinet opened a pull request: https://github.com/apache/thrift/pull/264 Fix TSimpleJSONProtocol reading beyond message TSimpleJSONProtocol should stop reading after whole message was read. This is a more simple fix than https://github.com/apache/thrift/pull/134 You can merge this pull request into a Git repository by running: $ git pull https://github.com/cvlchinet/thrift patch-2 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/thrift/pull/264.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 #264 commit 5281e9d3e788da94c7c30fc664ea007e0ddaeb71 Author: cvlchinet c...@chinet.info Date: 2014-11-13T16:37:50Z Fix TSimpleJSONProtocol reading beyond message TSimpleJSONProtocol should stop reading after whole message was read. This is a more simple fix than https://github.com/apache/thrift/pull/134 --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] thrift pull request: THRIFT-2817 - TSimpleJSONProtocol reads beyon...
Github user cvlchinet commented on the pull request: https://github.com/apache/thrift/pull/264#issuecomment-62960161 Alright, working on it. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] thrift pull request: node.js WebSocket client
GitHub user cvlchinet opened a pull request: https://github.com/apache/thrift/pull/265 node.js WebSocket client I needed a WebSocket client for node.js and implemented it. I am sharing my code with the community: This PR adds a WebSocket client to the node.js library. It is a combination of http_connection.js and the WebSocket implementation from js. Adds dependency to https://github.com/einaros/ws Includes integration tests. It only works with TJSONProtocol and TBufferedTransport because sending raw binary data on a WebSocket currently causes an UTF8 error. Maybe it is possible to fix? You can merge this pull request into a Git repository by running: $ git pull https://github.com/cvlchinet/thrift master Alternatively you can review and apply these changes as the patch at: https://github.com/apache/thrift/pull/265.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 #265 commit 29a22a41e1de218697cca94b07ff6445c702c7f0 Author: Chi Vinh Le c...@chinet.info Date: 2014-11-13T21:03:53Z node.js WebSocket client --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] thrift pull request: THRIFT-2817 - TSimpleJSONProtocol reads beyon...
Github user cvlchinet commented on the pull request: https://github.com/apache/thrift/pull/264#issuecomment-62982101 Ok I checked the options to write a test. It is quite complicated, because this bug only occurs when using real sockets. So when using a Buffer for the tests it won't work. I don't see a straightforward to write a mock either. An integration test, would catch it immediately. I see the lib has some kind of integration tests? right? I couldn't find out yet, how they work though. Is there a integration test concept? --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] thrift pull request: Wrap errors in iostream_transport.go
GitHub user cvlchinet opened a pull request: https://github.com/apache/thrift/pull/246 Wrap errors in iostream_transport.go When I used the StreamTransport to do unit tests I noticed that the EOF TTransportException is not correctly thrown. I quickly found out that the errors in iostream_transport.go where not wrapped with NewTTransportExceptionFromError. Fixed that quickly and it works greatly! Hope this saves others precious time when developing with thrift and go :-) You can merge this pull request into a Git repository by running: $ git pull https://github.com/cvlchinet/thrift master Alternatively you can review and apply these changes as the patch at: https://github.com/apache/thrift/pull/246.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 #246 commit c00fa154faa337db47a07c2c49d13d51c487d77a Author: cvlchinet c...@chinet.info Date: 2014-10-11T22:57:23Z Wrap errors in iostream_transport.go Wrap errors in iostream_transport.go using NewTTransportExceptionFromError --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] thrift pull request: Wrap errors in iostream_transport.go
Github user cvlchinet commented on the pull request: https://github.com/apache/thrift/pull/246#issuecomment-58768209 The precommit seems to fail with node related stuff. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
