[GitHub] thrift issue #1491: THRIFT-4489: Add UDS support for nodejs thrift client

2018-03-20 Thread jeking3
Github user jeking3 commented on the issue: https://github.com/apache/thrift/pull/1491 Looks good, will merge in the morning. ---

[GitHub] thrift issue #1491: THRIFT-4489: Add UDS support for nodejs thrift client

2018-03-20 Thread danielhtshih
Github user danielhtshih commented on the issue: https://github.com/apache/thrift/pull/1491 @jeking3 here is the single commit https://github.com/danielhtshih/thrift/commit/9c560daf52fcd89c212451f0793973af17f6cc55 from THRIFT-4489-squashed Thanks. ---

[GitHub] thrift issue #1491: THRIFT-4489: Add UDS support for nodejs thrift client

2018-03-20 Thread jeking3
Github user jeking3 commented on the issue: https://github.com/apache/thrift/pull/1491 This squash was not done properly. It includes all of the master commits in it. You want to start with an updated master: git checkout master git pull Then you want to

[GitHub] thrift issue #1491: THRIFT-4489: Add UDS support for nodejs thrift client

2018-03-20 Thread danielhtshih
Github user danielhtshih commented on the issue: https://github.com/apache/thrift/pull/1491 @jeking3 please use this single commit https://github.com/apache/thrift/pull/1491/commits/041f91dd6042b98c5bbd0846f5e6ed7fb6fd4bf1 Thanks. ---

[GitHub] thrift issue #1491: THRIFT-4489: Add UDS support for nodejs thrift client

2018-03-19 Thread jeking3
Github user jeking3 commented on the issue: https://github.com/apache/thrift/pull/1491 Okay so, here's the deal. This pull request has 9 commits and one of them is a merge. If you want your name on the commit tag you will need to squash and resubmit with a single commit and then I c

[GitHub] thrift issue #1491: THRIFT-4489: Add UDS support for nodejs thrift client

2018-03-19 Thread jeking3
Github user jeking3 commented on the issue: https://github.com/apache/thrift/pull/1491 I get an error that may be unrelated when I run cross tests on --server nodejs locally. This one test failes sporadically and I don't know if it could be related to these changes; the client has to

[GitHub] thrift issue #1491: THRIFT-4489: Add UDS support for nodejs thrift client

2018-03-19 Thread jeking3
Github user jeking3 commented on the issue: https://github.com/apache/thrift/pull/1491 I am running a cross test on this locally; if it passes I will merge it, so don't take any action. ---

[GitHub] thrift issue #1491: THRIFT-4489: Add UDS support for nodejs thrift client

2018-03-19 Thread jeking3
Github user jeking3 commented on the issue: https://github.com/apache/thrift/pull/1491 Perhaps; my PR for THRIFT-4515 improves the stability and coverage of crosstest by adding more controlled server shutdown, and it passed tests (I've run it many times on my local fork as well). I j

[GitHub] thrift issue #1491: THRIFT-4489: Add UDS support for nodejs thrift client

2018-03-18 Thread danielhtshih
Github user danielhtshih commented on the issue: https://github.com/apache/thrift/pull/1491 I believe the unexpected failures from the latest run is not related to my changes for nodejs. Maybe it needs another merge from master branch? ``` ===

[GitHub] thrift issue #1491: THRIFT-4489: Add UDS support for nodejs thrift client

2018-03-14 Thread jeking3
Github user jeking3 commented on the issue: https://github.com/apache/thrift/pull/1491 Please rebase on master one more time, most of the build issues are gone now. ---

[GitHub] thrift issue #1491: THRIFT-4489: Add UDS support for nodejs thrift client

2018-03-06 Thread jeking3
Github user jeking3 commented on the issue: https://github.com/apache/thrift/pull/1491 If you haven't seen it I recommend reading `build/docker/README.md` and use the `ubuntu-artful` image to run a cross test with `build/docker/scripts/cross-test.sh` inside the docker container. Doin

[GitHub] thrift issue #1491: THRIFT-4489: Add UDS support for nodejs thrift client

2018-03-06 Thread jeking3
Github user jeking3 commented on the issue: https://github.com/apache/thrift/pull/1491 The presence of "--domain-socket" to the client or server implies uds. This needs to be fixed, and domain needs to be moved from haskell to nodejs, and the cross tests that are failing (many) in th

[GitHub] thrift issue #1491: THRIFT-4489: Add UDS support for nodejs thrift client

2018-03-05 Thread jeking3
Github user jeking3 commented on the issue: https://github.com/apache/thrift/pull/1491 Where does the "--type uds" option get passed into the python test client or server in order to convince it to use domain sockets? In other test suites like cpp, only "--domain_socket" is required