[jira] [Commented] (THRIFT-4659) golang race detected when closing listener socket
[ https://issues.apache.org/jira/browse/THRIFT-4659?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16716572#comment-16716572 ] ASF GitHub Bot commented on THRIFT-4659: dcelasun closed pull request #1645: THRIFT-4659 golang race detected when closing listener socket URL: https://github.com/apache/thrift/pull/1645 This is a PR merged from a forked repository. As GitHub hides the original diff on merge, it is displayed below for the sake of provenance: As this is a foreign pull request (from a fork), the diff is supplied below (as it won't show otherwise due to GitHub magic): diff --git a/lib/go/thrift/server_socket.go b/lib/go/thrift/server_socket.go index 80313c4be5..7dd24ae364 100644 --- a/lib/go/thrift/server_socket.go +++ b/lib/go/thrift/server_socket.go @@ -75,7 +75,9 @@ func (p *TServerSocket) Accept() (TTransport, error) { return nil, errTransportInterrupted } + p.mu.Lock() listener := p.listener + p.mu.Unlock() if listener == nil { return nil, NewTTransportException(NOT_OPEN, "No underlying server socket") } @@ -115,19 +117,20 @@ func (p *TServerSocket) Addr() net.Addr { } func (p *TServerSocket) Close() error { - defer func() { - p.listener = nil - }() + var err error + p.mu.Lock() if p.IsListening() { - return p.listener.Close() + err = p.listener.Close() + p.listener = nil } - return nil + p.mu.Unlock() + return err } func (p *TServerSocket) Interrupt() error { p.mu.Lock() - defer p.mu.Unlock() p.interrupted = true + p.mu.Unlock() p.Close() return nil This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > golang race detected when closing listener socket > - > > Key: THRIFT-4659 > URL: https://issues.apache.org/jira/browse/THRIFT-4659 > Project: Thrift > Issue Type: Bug > Components: Go - Library >Affects Versions: 0.11.0 >Reporter: Jay Gheewala >Assignee: Can Celasun >Priority: Major > Attachments: THRIFT-4659.diff > > > Race condition is deteced for following write/read > > git.apache.org/thrift.git/lib/go/thrift/server_socket.go:119 > git.apache.org/thrift.git/lib/go/thrift.(*TServerSocket).Close() > git.apache.org/thrift.git/lib/go/thrift/server_socket.go:122 > git.apache.org/thrift.git/lib/go/thrift.(*TServerSocket).Interrupt() > > git.apache.org/thrift.git/lib/go/thrift.(*TServerSocket).Accept() > git.apache.org/thrift.git/lib/go/thrift/server_socket.go:78 > git.apache.org/thrift.git/lib/go/thrift.(*TSimpleServer).innerAccept() > git.apache.org/thrift.git/lib/go/thrift/simple_server.go:129 > git.apache.org/thrift.git/lib/go/thrift.(*TSimpleServer).AcceptLoop() -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (THRIFT-4659) golang race detected when closing listener socket
[ https://issues.apache.org/jira/browse/THRIFT-4659?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16715706#comment-16715706 ] ASF GitHub Bot commented on THRIFT-4659: jgheewala opened a new pull request #1645: THRIFT-4659 golang race detected when closing listener socket URL: https://github.com/apache/thrift/pull/1645 Some helpful tips for a successful Apache Thrift PR: * Did you test your changes locally or using CI in your fork? Tested changes locally * Is the Apache Jira THRIFT ticket identifier in the PR title? Yes * Is the Apache Jira THRIFT ticket identifier in the commit message? Yes * Did you squash your changes to a single commit? Yes * Are these changes backwards compatible? (please say so in PR description) * Do you need to update the language-specific README? Client: [go] This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > golang race detected when closing listener socket > - > > Key: THRIFT-4659 > URL: https://issues.apache.org/jira/browse/THRIFT-4659 > Project: Thrift > Issue Type: Bug > Components: Go - Library >Affects Versions: 0.11.0 >Reporter: Jay Gheewala >Assignee: Can Celasun >Priority: Major > Attachments: THRIFT-4659.diff > > > Race condition is deteced for following write/read > > git.apache.org/thrift.git/lib/go/thrift/server_socket.go:119 > git.apache.org/thrift.git/lib/go/thrift.(*TServerSocket).Close() > git.apache.org/thrift.git/lib/go/thrift/server_socket.go:122 > git.apache.org/thrift.git/lib/go/thrift.(*TServerSocket).Interrupt() > > git.apache.org/thrift.git/lib/go/thrift.(*TServerSocket).Accept() > git.apache.org/thrift.git/lib/go/thrift/server_socket.go:78 > git.apache.org/thrift.git/lib/go/thrift.(*TSimpleServer).innerAccept() > git.apache.org/thrift.git/lib/go/thrift/simple_server.go:129 > git.apache.org/thrift.git/lib/go/thrift.(*TSimpleServer).AcceptLoop() -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (THRIFT-4679) Duplicate declaration of InputBufferUnderrunError in lib/nodejs/lib/thrift/json_protocol.js
[ https://issues.apache.org/jira/browse/THRIFT-4679?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16715016#comment-16715016 ] ASF GitHub Bot commented on THRIFT-4679: jeking3 commented on issue #1642: THRIFT-4679: Remove unused variable declaration URL: https://github.com/apache/thrift/pull/1642#issuecomment-445875450 When we complete the 0.12.0 release then we start pushing out to package managers. It's kind of ad-hoc for the project however. It looks like @jfarrell maintains the npmjs updates. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > Duplicate declaration of InputBufferUnderrunError in > lib/nodejs/lib/thrift/json_protocol.js > --- > > Key: THRIFT-4679 > URL: https://issues.apache.org/jira/browse/THRIFT-4679 > Project: Thrift > Issue Type: Bug > Components: JavaScript - Library >Affects Versions: 0.11.0 >Reporter: Griffin Michl >Assignee: James E. King III >Priority: Minor > Labels: easyfix > Fix For: 1.0 > > Original Estimate: 10m > Remaining Estimate: 10m > > InputBufferUnderrunError is unnecessarily declared twice in > `lib/nodejs/lib/thrift/json_protocol.js`. This causes certain compilers to > break when they attempt to compile code that uses the node thrift library. We > should remove the first declaration, as it does not do anything. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (THRIFT-4679) Duplicate declaration of InputBufferUnderrunError in lib/nodejs/lib/thrift/json_protocol.js
[ https://issues.apache.org/jira/browse/THRIFT-4679?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16715038#comment-16715038 ] ASF GitHub Bot commented on THRIFT-4679: griffinmichl commented on issue #1642: THRIFT-4679: Remove unused variable declaration URL: https://github.com/apache/thrift/pull/1642#issuecomment-445881713 Sounds good. I think my company also has some fixes to the browser serializer they want to get in (we use our own right now). I'll try to get that moving along so we can get it into that release as well. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > Duplicate declaration of InputBufferUnderrunError in > lib/nodejs/lib/thrift/json_protocol.js > --- > > Key: THRIFT-4679 > URL: https://issues.apache.org/jira/browse/THRIFT-4679 > Project: Thrift > Issue Type: Bug > Components: JavaScript - Library >Affects Versions: 0.11.0 >Reporter: Griffin Michl >Assignee: James E. King III >Priority: Minor > Labels: easyfix > Fix For: 1.0 > > Original Estimate: 10m > Remaining Estimate: 10m > > InputBufferUnderrunError is unnecessarily declared twice in > `lib/nodejs/lib/thrift/json_protocol.js`. This causes certain compilers to > break when they attempt to compile code that uses the node thrift library. We > should remove the first declaration, as it does not do anything. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (THRIFT-4679) Duplicate declaration of InputBufferUnderrunError in lib/nodejs/lib/thrift/json_protocol.js
[ https://issues.apache.org/jira/browse/THRIFT-4679?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16715002#comment-16715002 ] ASF GitHub Bot commented on THRIFT-4679: griffinmichl commented on issue #1642: THRIFT-4679: Remove unused variable declaration URL: https://github.com/apache/thrift/pull/1642#issuecomment-445871911 Any idea how I can get this merged into the thrift npm package? This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > Duplicate declaration of InputBufferUnderrunError in > lib/nodejs/lib/thrift/json_protocol.js > --- > > Key: THRIFT-4679 > URL: https://issues.apache.org/jira/browse/THRIFT-4679 > Project: Thrift > Issue Type: Bug > Components: JavaScript - Library >Affects Versions: 0.11.0 >Reporter: Griffin Michl >Assignee: James E. King III >Priority: Minor > Labels: easyfix > Fix For: 1.0 > > Original Estimate: 10m > Remaining Estimate: 10m > > InputBufferUnderrunError is unnecessarily declared twice in > `lib/nodejs/lib/thrift/json_protocol.js`. This causes certain compilers to > break when they attempt to compile code that uses the node thrift library. We > should remove the first declaration, as it does not do anything. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (THRIFT-4679) Duplicate declaration of InputBufferUnderrunError in lib/nodejs/lib/thrift/json_protocol.js
[ https://issues.apache.org/jira/browse/THRIFT-4679?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16713291#comment-16713291 ] ASF GitHub Bot commented on THRIFT-4679: jeking3 closed pull request #1642: THRIFT-4679: Remove unused variable declaration URL: https://github.com/apache/thrift/pull/1642 This is a PR merged from a forked repository. As GitHub hides the original diff on merge, it is displayed below for the sake of provenance: As this is a foreign pull request (from a fork), the diff is supplied below (as it won't show otherwise due to GitHub magic): diff --git a/lib/nodejs/lib/thrift/json_protocol.js b/lib/nodejs/lib/thrift/json_protocol.js index d960be9d0c..727a3b2ffb 100644 --- a/lib/nodejs/lib/thrift/json_protocol.js +++ b/lib/nodejs/lib/thrift/json_protocol.js @@ -18,7 +18,6 @@ */ var Int64 = require('node-int64'); -var InputBufferUnderrunError = require('./transport').InputBufferUnderrunError; var Thrift = require('./thrift'); var Type = Thrift.Type; var util = require("util"); This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > Duplicate declaration of InputBufferUnderrunError in > lib/nodejs/lib/thrift/json_protocol.js > --- > > Key: THRIFT-4679 > URL: https://issues.apache.org/jira/browse/THRIFT-4679 > Project: Thrift > Issue Type: Bug > Components: JavaScript - Library >Affects Versions: 0.11.0 >Reporter: Griffin Michl >Priority: Minor > Labels: easyfix > Original Estimate: 10m > Remaining Estimate: 10m > > InputBufferUnderrunError is unnecessarily declared twice in > `lib/nodejs/lib/thrift/json_protocol.js`. This causes certain compilers to > break when they attempt to compile code that uses the node thrift library. We > should remove the first declaration, as it does not do anything. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (THRIFT-4679) Duplicate declaration of InputBufferUnderrunError in lib/nodejs/lib/thrift/json_protocol.js
[ https://issues.apache.org/jira/browse/THRIFT-4679?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16713192#comment-16713192 ] ASF GitHub Bot commented on THRIFT-4679: griffinmichl opened a new pull request #1642: THRIFT-4679: Remove unused variable declaration URL: https://github.com/apache/thrift/pull/1642 Changes are backwards compatible (as this line never did anything) https://issues.apache.org/jira/browse/THRIFT-4679 This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > Duplicate declaration of InputBufferUnderrunError in > lib/nodejs/lib/thrift/json_protocol.js > --- > > Key: THRIFT-4679 > URL: https://issues.apache.org/jira/browse/THRIFT-4679 > Project: Thrift > Issue Type: Bug > Components: JavaScript - Library >Affects Versions: 0.11.0 >Reporter: Griffin Michl >Priority: Minor > Labels: easyfix > Original Estimate: 10m > Remaining Estimate: 10m > > InputBufferUnderrunError is unnecessarily declared twice in > `lib/nodejs/lib/thrift/json_protocol.js`. This causes certain compilers to > break when they attempt to compile code that uses the node thrift library. We > should remove the first declaration, as it does not do anything. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (THRIFT-4676) CL tutorial build fails sporadically
[ https://issues.apache.org/jira/browse/THRIFT-4676?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16711969#comment-16711969 ] ASF GitHub Bot commented on THRIFT-4676: allengeorge commented on issue #1639: THRIFT-4676: Fix intermittent CL build failures URL: https://github.com/apache/thrift/pull/1639#issuecomment-445011984 @uint Will do. Thanks so much for taking a look at this! This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > CL tutorial build fails sporadically > > > Key: THRIFT-4676 > URL: https://issues.apache.org/jira/browse/THRIFT-4676 > Project: Thrift > Issue Type: Task > Components: Build Process >Reporter: Allen George >Assignee: Allen George >Priority: Minor > Fix For: 1.0 > > > Many of the CI failures are caused by the CL tutorial build failing. This > isn't because the CL code is wrong, it's simply because of a race condition > _somewhere_. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (THRIFT-4676) CL tutorial build fails sporadically
[ https://issues.apache.org/jira/browse/THRIFT-4676?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16711918#comment-16711918 ] ASF GitHub Bot commented on THRIFT-4676: uint commented on issue #1639: THRIFT-4676: Fix intermittent CL build failures URL: https://github.com/apache/thrift/pull/1639#issuecomment-444999736 Sorry for taking time. I've looked through it. Looks good to me. Awesome that you cleaned up the makefiles. If you want to rearrange the directory structure like you've mentioned in the e-mails, I'm good with it! I won't be making any changes here. Thanks a lot for working on this. Cheers. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > CL tutorial build fails sporadically > > > Key: THRIFT-4676 > URL: https://issues.apache.org/jira/browse/THRIFT-4676 > Project: Thrift > Issue Type: Task > Components: Build Process >Reporter: Allen George >Assignee: Allen George >Priority: Minor > Fix For: 1.0 > > > Many of the CI failures are caused by the CL tutorial build failing. This > isn't because the CL code is wrong, it's simply because of a race condition > _somewhere_. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (THRIFT-4676) CL tutorial build fails sporadically
[ https://issues.apache.org/jira/browse/THRIFT-4676?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16710638#comment-16710638 ] ASF GitHub Bot commented on THRIFT-4676: jeking3 closed pull request #1639: THRIFT-4676: Fix intermittent CL build failures URL: https://github.com/apache/thrift/pull/1639 This is a PR merged from a forked repository. As GitHub hides the original diff on merge, it is displayed below for the sake of provenance: As this is a foreign pull request (from a fork), the diff is supplied below (as it won't show otherwise due to GitHub magic): diff --git a/.gitignore b/.gitignore index 0e7eedcd86..b7f7b454bb 100644 --- a/.gitignore +++ b/.gitignore @@ -339,8 +339,17 @@ project.lock.json /test/rs/target/ /test/rs/*.iml /test/rs/**/*.iml +/lib/cl/backport-update.zip +/lib/cl/lib +/tutorial/cl/quicklisp.lisp +/tutorial/cl/externals/ +/tutorial/cl/quicklisp/ /tutorial/cl/TutorialClient /tutorial/cl/TutorialServer +/tutorial/cl/backport-update.zip +/tutorial/cl/lib/ +/tutorial/cl/shared-implementation.fasl +/tutorial/cl/tutorial-implementation.fasl /tutorial/cpp/TutorialClient /tutorial/cpp/TutorialServer /tutorial/c_glib/tutorial_client diff --git a/tutorial/cl/Makefile.am b/tutorial/cl/Makefile.am index fb6e83a420..2b2013a3c9 100755 --- a/tutorial/cl/Makefile.am +++ b/tutorial/cl/Makefile.am @@ -15,19 +15,30 @@ # KIND, either express or implied. See the License for the # specific language governing permissions and limitations # under the License. -# + +setup-local-lisp-env: ensure-externals.sh + bash ensure-externals.sh gen-cl: $(top_srcdir)/tutorial/tutorial.thrift $(THRIFT) --gen cl -r $< -TutorialServer: make-tutorial-server.lisp - $(SBCL) --script make-tutorial-server.lisp +ALL_FILE_PREREQS = \ + load-locally.lisp \ + make-tutorial-server.lisp \ + make-tutorial-client.lisp \ + shared-implementation.lisp \ + thrift-tutorial.asd \ + tutorial-implementation.lisp -TutorialClient: make-tutorial-client.lisp +# NOTE: the server and client cannot be built in parallel +# because on loading the make-tutorial-* scripts SBCL will +# attempt to compile their dependencies. Unfortunately, +# because their dependencies are shared, parallel jobs can +# end up overwriting or corrupting the compiled files +all-local: gen-cl setup-local-lisp-env $(ALL_FILE_PREREQS) + $(SBCL) --script make-tutorial-server.lisp $(SBCL) --script make-tutorial-client.lisp -all-local: gen-cl TutorialClient TutorialServer - tutorialserver: all ./TutorialServer @@ -35,9 +46,16 @@ tutorialclient: all ./TutorialClient clean-local: - $(RM) -r gen-* - $(RM) TutorialServer - $(RM) TutorialClient + -$(RM) -r gen-* + -$(RM) -r externals + -$(RM) -r quicklisp + -$(RM) -r lib + -$(RM) quicklisp.lisp + -$(RM) backport-update.zip + -$(RM) shared-implementation.fasl + -$(RM) tutorial-implementation.fasl + -$(RM) TutorialServer + -$(RM) TutorialClient EXTRA_DIST = \ tutorial-implementation.lisp \ diff --git a/tutorial/cl/ensure-externals.sh b/tutorial/cl/ensure-externals.sh new file mode 12 index 00..5ae8c5657c --- /dev/null +++ b/tutorial/cl/ensure-externals.sh @@ -0,0 +1 @@ +../../lib/cl/ensure-externals.sh \ No newline at end of file diff --git a/tutorial/cl/load-locally.lisp b/tutorial/cl/load-locally.lisp new file mode 100644 index 00..b52a0a2698 --- /dev/null +++ b/tutorial/cl/load-locally.lisp @@ -0,0 +1,22 @@ +(in-package #:cl-user) + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. + + Just a script for loading the library itself, using bundled dependencies. + This is an identical copy of the file in lib/cl. + +(require "asdf") + +(load (merge-pathnames "externals/bundle.lisp" *load-truename*)) +(asdf:load-asd (merge-pathnames "lib/de.setf.thrift-backport-update/thrift.asd" *load-truename*)) +(asdf:load-system :thrift) diff --git a/tutorial/cl/make-tutorial-client.lisp b/tutorial/cl/make-tutorial-client.lisp index 59450a2eae..3a6d86134f 100644 --- a/tutorial/cl/make-tutorial-client.lisp +++ b/tutorial/cl/make-tutorial-client.lisp @@ -13,7 +13,7 @@ limitations under the License. (require "asdf") -(load (merge-pathnames
[jira] [Commented] (THRIFT-4656) infinite loop in latest PHP library
[ https://issues.apache.org/jira/browse/THRIFT-4656?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16710636#comment-16710636 ] ASF GitHub Bot commented on THRIFT-4656: jeking3 closed pull request #1618: THRIFT-4656: Fix infinite loop in PHP TCurlClient URL: https://github.com/apache/thrift/pull/1618 This is a PR merged from a forked repository. As GitHub hides the original diff on merge, it is displayed below for the sake of provenance: As this is a foreign pull request (from a fork), the diff is supplied below (as it won't show otherwise due to GitHub magic): diff --git a/lib/php/lib/Transport/TCurlClient.php b/lib/php/lib/Transport/TCurlClient.php index 3d4908d901..482b43b72c 100644 --- a/lib/php/lib/Transport/TCurlClient.php +++ b/lib/php/lib/Transport/TCurlClient.php @@ -169,6 +169,24 @@ public function read($len) } } +/** + * Guarantees that the full amount of data is read. Since TCurlClient gets entire payload at + * once, parent readAll cannot be used. + * + * @return string The data, of exact length + * @throws TTransportException if cannot read data + */ +public function readAll($len) +{ +$data = $this->read($len); + +if (TStringFuncFactory::create()->strlen($data) !== $len) { +throw new TTransportException('TCurlClient could not read '.$len.' bytes'); +} + +return $data; +} + /** * Writes some data into the pending buffer * This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > infinite loop in latest PHP library > --- > > Key: THRIFT-4656 > URL: https://issues.apache.org/jira/browse/THRIFT-4656 > Project: Thrift > Issue Type: Bug > Components: PHP - Library >Reporter: Josip Sokcevic >Priority: Major > Fix For: 0.12.0 > > Attachments: > 0001-THRIFT-4656-Fix-infinite-loop-in-PHP-TCurlClient.patch > > > The latest PHP library can enter into infinite loop state when specific > payload is returned: > HTTP status: 200 > Response body: > > It was introduced with THRIFT-4645, where check was replaced from > {code:java} > !$this->response_{code} > to > {code:java} > $this->response_ === false{code} -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (THRIFT-4676) CL tutorial build fails sporadically
[ https://issues.apache.org/jira/browse/THRIFT-4676?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16710500#comment-16710500 ] ASF GitHub Bot commented on THRIFT-4676: allengeorge commented on issue #1639: THRIFT-4676: Fix intermittent CL build failures URL: https://github.com/apache/thrift/pull/1639#issuecomment-444602420 Yeap. If Tomek would like me to make changes I’m happy to do it in a follow-up PR. The changes as they exist will do the trick for the builds. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > CL tutorial build fails sporadically > > > Key: THRIFT-4676 > URL: https://issues.apache.org/jira/browse/THRIFT-4676 > Project: Thrift > Issue Type: Task > Components: Build Process >Reporter: Allen George >Assignee: Allen George >Priority: Minor > > Many of the CI failures are caused by the CL tutorial build failing. This > isn't because the CL code is wrong, it's simply because of a race condition > _somewhere_. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (THRIFT-4676) CL tutorial build fails sporadically
[ https://issues.apache.org/jira/browse/THRIFT-4676?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16710496#comment-16710496 ] ASF GitHub Bot commented on THRIFT-4676: jeking3 commented on issue #1639: THRIFT-4676: Fix intermittent CL build failures URL: https://github.com/apache/thrift/pull/1639#issuecomment-444600778 Shall I merge? This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > CL tutorial build fails sporadically > > > Key: THRIFT-4676 > URL: https://issues.apache.org/jira/browse/THRIFT-4676 > Project: Thrift > Issue Type: Task > Components: Build Process >Reporter: Allen George >Assignee: Allen George >Priority: Minor > > Many of the CI failures are caused by the CL tutorial build failing. This > isn't because the CL code is wrong, it's simply because of a race condition > _somewhere_. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (THRIFT-4676) CL tutorial build fails sporadically
[ https://issues.apache.org/jira/browse/THRIFT-4676?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16707626#comment-16707626 ] ASF GitHub Bot commented on THRIFT-4676: allengeorge commented on issue #1639: THRIFT-4676: Fix intermittent CL build failures URL: https://github.com/apache/thrift/pull/1639#issuecomment-443815737 This is looking good. I've force-pushed a few times to re-kick the Travis builds and I'm no longer seeing CL-related failures (whereas before at least one or two would pop up). I'm hoping to receive a review from the original CL contributor, but if not I'll submit by EOW. Looking forward to a stable Thrift build! This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > CL tutorial build fails sporadically > > > Key: THRIFT-4676 > URL: https://issues.apache.org/jira/browse/THRIFT-4676 > Project: Thrift > Issue Type: Task > Components: Build Process >Reporter: Allen George >Assignee: Allen George >Priority: Minor > > Many of the CI failures are caused by the CL tutorial build failing. This > isn't because the CL code is wrong, it's simply because of a race condition > _somewhere_. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (THRIFT-4676) CL tutorial build fails sporadically
[ https://issues.apache.org/jira/browse/THRIFT-4676?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16705827#comment-16705827 ] ASF GitHub Bot commented on THRIFT-4676: allengeorge edited a comment on issue #1639: THRIFT-4676: Disable ASDF compiler cache for CL tutorial server/client URL: https://github.com/apache/thrift/pull/1639#issuecomment-443426550 Going to force push a couple of times to kick off the builds and see if the SBCL error pops up. If it doesn't I can tentatively claim victory. The first run was successful, so...let's hope. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > CL tutorial build fails sporadically > > > Key: THRIFT-4676 > URL: https://issues.apache.org/jira/browse/THRIFT-4676 > Project: Thrift > Issue Type: Task > Components: Build Process >Reporter: Allen George >Assignee: Allen George >Priority: Minor > > Many of the CI failures are caused by the CL tutorial build failing. This > isn't because the CL code is wrong, it's simply because of a race condition > _somewhere_. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (THRIFT-4676) CL tutorial build fails sporadically
[ https://issues.apache.org/jira/browse/THRIFT-4676?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16705826#comment-16705826 ] ASF GitHub Bot commented on THRIFT-4676: allengeorge commented on issue #1639: THRIFT-4676: Disable ASDF compiler cache for CL tutorial server/client URL: https://github.com/apache/thrift/pull/1639#issuecomment-443426550 Going to force push a couple of times to kick off the builds and see if the SBCL error pops up. If it doesn't I can tentatively claim victory. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > CL tutorial build fails sporadically > > > Key: THRIFT-4676 > URL: https://issues.apache.org/jira/browse/THRIFT-4676 > Project: Thrift > Issue Type: Task > Components: Build Process >Reporter: Allen George >Assignee: Allen George >Priority: Minor > > Many of the CI failures are caused by the CL tutorial build failing. This > isn't because the CL code is wrong, it's simply because of a race condition > _somewhere_. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (THRIFT-4676) CL tutorial build fails sporadically
[ https://issues.apache.org/jira/browse/THRIFT-4676?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16705306#comment-16705306 ] ASF GitHub Bot commented on THRIFT-4676: allengeorge commented on issue #1639: THRIFT-4676: Prevent CL tutorial server/client being built in parallel URL: https://github.com/apache/thrift/pull/1639#issuecomment-443347279 Second try: it appears that executing the builds sequentially doesn't help because the same path is being used for the compiler cache? I think? So I'll try disabling it. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > CL tutorial build fails sporadically > > > Key: THRIFT-4676 > URL: https://issues.apache.org/jira/browse/THRIFT-4676 > Project: Thrift > Issue Type: Task > Components: Build Process >Reporter: Allen George >Assignee: Allen George >Priority: Minor > > Many of the CI failures are caused by the CL tutorial build failing. This > isn't because the CL code is wrong, it's simply because of a race condition > _somewhere_. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (THRIFT-4676) CL tutorial build fails sporadically
[ https://issues.apache.org/jira/browse/THRIFT-4676?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16705294#comment-16705294 ] ASF GitHub Bot commented on THRIFT-4676: allengeorge commented on issue #1639: THRIFT-4676: Prevent CL tutorial server/client being built in parallel URL: https://github.com/apache/thrift/pull/1639#issuecomment-443343701 Amazing. Now the CL build is failing with another error, specifically that it doesn't have permissions to write an out file... This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > CL tutorial build fails sporadically > > > Key: THRIFT-4676 > URL: https://issues.apache.org/jira/browse/THRIFT-4676 > Project: Thrift > Issue Type: Task > Components: Build Process >Reporter: Allen George >Assignee: Allen George >Priority: Minor > > Many of the CI failures are caused by the CL tutorial build failing. This > isn't because the CL code is wrong, it's simply because of a race condition > _somewhere_. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (THRIFT-4676) CL tutorial build fails sporadically
[ https://issues.apache.org/jira/browse/THRIFT-4676?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16705228#comment-16705228 ] ASF GitHub Bot commented on THRIFT-4676: allengeorge opened a new pull request #1639: THRIFT-4676: Prevent CL tutorial server/client being built in parallel URL: https://github.com/apache/thrift/pull/1639 This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > CL tutorial build fails sporadically > > > Key: THRIFT-4676 > URL: https://issues.apache.org/jira/browse/THRIFT-4676 > Project: Thrift > Issue Type: Task > Components: Build Process >Reporter: Allen George >Assignee: Allen George >Priority: Minor > > Many of the CI failures are caused by the CL tutorial build failing. This > isn't because the CL code is wrong, it's simply because of a race condition > _somewhere_. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (THRIFT-3143) add typescript directory support
[ https://issues.apache.org/jira/browse/THRIFT-3143?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16704409#comment-16704409 ] ASF GitHub Bot commented on THRIFT-3143: mustafa-cosar commented on issue #1630: THRIFT-3143: Add nodets support URL: https://github.com/apache/thrift/pull/1630#issuecomment-443130971 Thanks for merging it. Here is the necessary info (the cross test covers the following cases): - nodets - since: 0.12.0 (next release, I guess) - lang/lib levels: Min: TypeScript 3.1.6 - Low level transports: Socket - Transport wrappers: TLS - Protocols: binary - Servers: Simple This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > add typescript directory support > > > Key: THRIFT-3143 > URL: https://issues.apache.org/jira/browse/THRIFT-3143 > Project: Thrift > Issue Type: New Feature > Components: Node.js - Compiler >Reporter: Kazuki Yasufuku >Assignee: James E. King III >Priority: Major > Fix For: 1.0 > > > Current typescript support is only work for browser, and generated d.ts uses > internal module. > So, it's hard to use for typescript in node.js. > To solve probrem, I make a pull request that generate typescript code > directory. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (THRIFT-3143) add typescript directory support
[ https://issues.apache.org/jira/browse/THRIFT-3143?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16703759#comment-16703759 ] ASF GitHub Bot commented on THRIFT-3143: jeking3 closed pull request #1630: THRIFT-3143: Add nodets support URL: https://github.com/apache/thrift/pull/1630 This is a PR merged from a forked repository. As GitHub hides the original diff on merge, it is displayed below for the sake of provenance: As this is a foreign pull request (from a fork), the diff is supplied below (as it won't show otherwise due to GitHub magic): diff --git a/Makefile.am b/Makefile.am index cdb8bd2f5b..511452dfc8 100755 --- a/Makefile.am +++ b/Makefile.am @@ -54,7 +54,7 @@ empty := space := $(empty) $(empty) comma := , -CROSS_LANGS = @MAYBE_CPP@ @MAYBE_C_GLIB@ @MAYBE_CL@ @MAYBE_D@ @MAYBE_JAVA@ @MAYBE_CSHARP@ @MAYBE_PYTHON@ @MAYBE_PY3@ @MAYBE_RUBY@ @MAYBE_HASKELL@ @MAYBE_PERL@ @MAYBE_PHP@ @MAYBE_GO@ @MAYBE_NODEJS@ @MAYBE_DART@ @MAYBE_ERLANG@ @MAYBE_LUA@ @MAYBE_RS@ @MAYBE_DOTNETCORE@ +CROSS_LANGS = @MAYBE_CPP@ @MAYBE_C_GLIB@ @MAYBE_CL@ @MAYBE_D@ @MAYBE_JAVA@ @MAYBE_CSHARP@ @MAYBE_PYTHON@ @MAYBE_PY3@ @MAYBE_RUBY@ @MAYBE_HASKELL@ @MAYBE_PERL@ @MAYBE_PHP@ @MAYBE_GO@ @MAYBE_NODEJS@ @MAYBE_DART@ @MAYBE_ERLANG@ @MAYBE_LUA@ @MAYBE_RS@ @MAYBE_DOTNETCORE@ @MAYBE_NODETS@ CROSS_LANGS_COMMA_SEPARATED = $(subst $(space),$(comma),$(CROSS_LANGS)) if WITH_PY3 diff --git a/compiler/cpp/src/thrift/generate/t_js_generator.cc b/compiler/cpp/src/thrift/generate/t_js_generator.cc index 840168ef53..61782b9d8b 100644 --- a/compiler/cpp/src/thrift/generate/t_js_generator.cc +++ b/compiler/cpp/src/thrift/generate/t_js_generator.cc @@ -64,7 +64,7 @@ class t_js_generator : public t_oop_generator { bool with_ns_ = false; -for( iter = parsed_options.begin(); iter != parsed_options.end(); ++iter) { +for (iter = parsed_options.begin(); iter != parsed_options.end(); ++iter) { if( iter->first.compare("node") == 0) { gen_node_ = true; } else if( iter->first.compare("jquery") == 0) { @@ -80,10 +80,6 @@ class t_js_generator : public t_oop_generator { } } -if (gen_node_ && gen_ts_) { - throw "Invalid switch: [-gen js:node,ts] options not compatible"; -} - if (gen_es6_ && gen_jquery_) { throw "Invalid switch: [-gen js:es6,jquery] options not compatible"; } @@ -203,6 +199,7 @@ class t_js_generator : public t_oop_generator { */ std::string js_includes(); + std::string ts_includes(); std::string render_includes(); std::string declare_field(t_field* tfield, bool init = false, bool obj = false); std::string function_signature(t_function* tfunction, @@ -285,7 +282,7 @@ class t_js_generator : public t_oop_generator { * TypeScript Definition File helper functions */ - string ts_function_signature(t_function* tfunction, bool include_callback); + string ts_function_signature(t_function* tfunction, bool include_callback, bool optional_callback); string ts_get_type(t_type* type); /** @@ -302,11 +299,11 @@ class t_js_generator : public t_oop_generator { string ts_declare() { return (ts_module_.empty() ? "declare " : ""); } /** - * Returns "?" if the given field is optional. + * Returns "?" if the given field is optional or has a default value. * @param t_field The field to check * @return string */ - string ts_get_req(t_field* field) { return (field->get_req() == t_field::T_OPTIONAL ? "?" : ""); } + string ts_get_req(t_field* field) {return (field->get_req() == t_field::T_OPTIONAL || field->get_value() != NULL ? "?" : ""); } /** * Returns the documentation, if the provided documentable object has one. @@ -415,7 +412,7 @@ void t_js_generator::init_generator() { f_types_ << js_includes() << endl << render_includes() << endl; if (gen_ts_) { -f_types_ts_ << autogen_comment() << endl; +f_types_ts_ << autogen_comment() << ts_includes() << endl; } if (gen_node_) { @@ -457,6 +454,20 @@ string t_js_generator::js_includes() { return ""; } +/** + * Prints standard ts imports + */ +string t_js_generator::ts_includes() { + if (gen_node_) { +return string( +"import thrift = require('thrift');\n" +"import Thrift = thrift.Thrift;\n" +"import Q = thrift.Q;\n"); + } + + return ""; +} + /** * Renders all the imports necessary for including another Thrift program */ @@ -518,8 +529,8 @@ void t_js_generator::generate_enum(t_enum* tenum) { indent_up(); - vector constants = tenum->get_constants(); - vector::iterator c_iter; + vector const& constants = tenum->get_constants(); + vector::const_iterator c_iter; for (c_iter = constants.begin(); c_iter != constants.end(); ++c_iter) { int value = (*c_iter)->get_value(); if (gen_ts_) { @@ -720,6 +731,11 @@ void t_js_generator::generate_js_struct_definition(ostream& out, string prefix =
[jira] [Commented] (THRIFT-3143) add typescript directory support
[ https://issues.apache.org/jira/browse/THRIFT-3143?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16703757#comment-16703757 ] ASF GitHub Bot commented on THRIFT-3143: jeking3 commented on issue #1630: THRIFT-3143: Add nodets support URL: https://github.com/apache/thrift/pull/1630#issuecomment-442981575 Sure, let me know what information needs to go into: https://thrift.apache.org/docs/Languages This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > add typescript directory support > > > Key: THRIFT-3143 > URL: https://issues.apache.org/jira/browse/THRIFT-3143 > Project: Thrift > Issue Type: New Feature > Components: Node.js - Compiler >Reporter: Kazuki Yasufuku >Priority: Major > > Current typescript support is only work for browser, and generated d.ts uses > internal module. > So, it's hard to use for typescript in node.js. > To solve probrem, I make a pull request that generate typescript code > directory. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (THRIFT-3143) add typescript directory support
[ https://issues.apache.org/jira/browse/THRIFT-3143?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=1670#comment-1670 ] ASF GitHub Bot commented on THRIFT-3143: mustafa-cosar commented on issue #1630: THRIFT-3143: Add nodets support URL: https://github.com/apache/thrift/pull/1630#issuecomment-442870437 Hi @jeking3 I have a green build but I had to disable `hs-nodets` test. If it is OK with you, can we merge this PR and then I can take a deeper look into the test with my colleagues? This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > add typescript directory support > > > Key: THRIFT-3143 > URL: https://issues.apache.org/jira/browse/THRIFT-3143 > Project: Thrift > Issue Type: New Feature > Components: Node.js - Compiler >Reporter: Kazuki Yasufuku >Priority: Major > > Current typescript support is only work for browser, and generated d.ts uses > internal module. > So, it's hard to use for typescript in node.js. > To solve probrem, I make a pull request that generate typescript code > directory. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (THRIFT-3143) add typescript directory support
[ https://issues.apache.org/jira/browse/THRIFT-3143?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16701358#comment-16701358 ] ASF GitHub Bot commented on THRIFT-3143: jeking3 edited a comment on issue #1630: THRIFT-3143: Add nodets support URL: https://github.com/apache/thrift/pull/1630#issuecomment-442305147 Do the best you can, and then add the rest to the known failures list. The builds need to pass. If something environmental or unrelated to your changes comes up, I can restart a job. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > add typescript directory support > > > Key: THRIFT-3143 > URL: https://issues.apache.org/jira/browse/THRIFT-3143 > Project: Thrift > Issue Type: New Feature > Components: Node.js - Compiler >Reporter: Kazuki Yasufuku >Priority: Major > > Current typescript support is only work for browser, and generated d.ts uses > internal module. > So, it's hard to use for typescript in node.js. > To solve probrem, I make a pull request that generate typescript code > directory. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (THRIFT-3143) add typescript directory support
[ https://issues.apache.org/jira/browse/THRIFT-3143?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16701355#comment-16701355 ] ASF GitHub Bot commented on THRIFT-3143: jeking3 commented on issue #1630: THRIFT-3143: Add nodets support URL: https://github.com/apache/thrift/pull/1630#issuecomment-442305147 Do the best you can, and then add the rest to the known failures list. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > add typescript directory support > > > Key: THRIFT-3143 > URL: https://issues.apache.org/jira/browse/THRIFT-3143 > Project: Thrift > Issue Type: New Feature > Components: Node.js - Compiler >Reporter: Kazuki Yasufuku >Priority: Major > > Current typescript support is only work for browser, and generated d.ts uses > internal module. > So, it's hard to use for typescript in node.js. > To solve probrem, I make a pull request that generate typescript code > directory. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (THRIFT-4674) Add stream context support into PHP/THttpClient
[ https://issues.apache.org/jira/browse/THRIFT-4674?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16701353#comment-16701353 ] ASF GitHub Bot commented on THRIFT-4674: jeking3 closed pull request #1636: THRIFT-4674 Added stream context support for PHP THttpClient URL: https://github.com/apache/thrift/pull/1636 This is a PR merged from a forked repository. As GitHub hides the original diff on merge, it is displayed below for the sake of provenance: As this is a foreign pull request (from a fork), the diff is supplied below (as it won't show otherwise due to GitHub magic): diff --git a/lib/php/lib/Transport/THttpClient.php b/lib/php/lib/Transport/THttpClient.php index a89794b086..0158809d29 100644 --- a/lib/php/lib/Transport/THttpClient.php +++ b/lib/php/lib/Transport/THttpClient.php @@ -88,14 +88,23 @@ class THttpClient extends TTransport */ protected $headers_; +/** + * Context additional options + * + * @var array + */ +protected $context_; + /** * Make a new HTTP client. * * @param string $host - * @param int $port + * @param int$port * @param string $uri + * @param string $scheme + * @param array $context */ -public function __construct($host, $port = 80, $uri = '', $scheme = 'http') +public function __construct($host, $port = 80, $uri = '', $scheme = 'http', array $context = array()) { if ((TStringFuncFactory::create()->strlen($uri) > 0) && ($uri{0} != '/')) { $uri = '/' . $uri; @@ -108,6 +117,7 @@ public function __construct($host, $port = 80, $uri = '', $scheme = 'http') $this->handle_ = null; $this->timeout_ = null; $this->headers_ = array(); +$this->context_ = $context; } /** @@ -211,16 +221,21 @@ public function flush() $headers[] = "$key: $value"; } -$options = array('method' => 'POST', +$options = $this->context_; + +$baseHttpOptions = isset($options["http"]) ? $options["http"] : array(); + +$httpOptions = $baseHttpOptions + array('method' => 'POST', 'header' => implode("\r\n", $headers), 'max_redirects' => 1, 'content' => $this->buf_); if ($this->timeout_ > 0) { -$options['timeout'] = $this->timeout_; +$httpOptions['timeout'] = $this->timeout_; } $this->buf_ = ''; -$contextid = stream_context_create(array('http' => $options)); +$options["http"] = $httpOptions; +$contextid = stream_context_create($options); $this->handle_ = @fopen( $this->scheme_ . '://' . $host . $this->uri_, 'r', This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > Add stream context support into PHP/THttpClient > --- > > Key: THRIFT-4674 > URL: https://issues.apache.org/jira/browse/THRIFT-4674 > Project: Thrift > Issue Type: Improvement >Reporter: Evgeniy Efimov >Priority: Major > Fix For: 1.0 > > > In some use-cases it is required to pass additional data into context for > THttpClient, client certificate as an example. > See PR: https://github.com/apache/thrift/pull/1636 -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (THRIFT-3143) add typescript directory support
[ https://issues.apache.org/jira/browse/THRIFT-3143?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16700503#comment-16700503 ] ASF GitHub Bot commented on THRIFT-3143: mustafa-cosar edited a comment on issue #1630: THRIFT-3143: Add nodets support URL: https://github.com/apache/thrift/pull/1630#issuecomment-442055696 I did some improvements to the tests script so most timeouts are fixed. the `erl-nodets` and `d-nodets` tests fail but their `nodejs` counterparts are in `known_failures_Linux.json` so they should be added as well. There is one test that I couldn't figure out: In `hs-nodets` test, the client logs that all the tests are successful and it does call `connection.end()`. However it still times out for some reason... This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > add typescript directory support > > > Key: THRIFT-3143 > URL: https://issues.apache.org/jira/browse/THRIFT-3143 > Project: Thrift > Issue Type: New Feature > Components: Node.js - Compiler >Reporter: Kazuki Yasufuku >Priority: Major > > Current typescript support is only work for browser, and generated d.ts uses > internal module. > So, it's hard to use for typescript in node.js. > To solve probrem, I make a pull request that generate typescript code > directory. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (THRIFT-3143) add typescript directory support
[ https://issues.apache.org/jira/browse/THRIFT-3143?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16700400#comment-16700400 ] ASF GitHub Bot commented on THRIFT-3143: mustafa-cosar commented on issue #1630: THRIFT-3143: Add nodets support URL: https://github.com/apache/thrift/pull/1630#issuecomment-442055696 I did some improvements to the tests script so most timeouts are fixed. the `erl-nodets` and `d-nodets` tests fail but their `nodejs` counterparts are in `known_failures.Linux.json` so they should be added as well. There is one test that I couldn't figure out: In `hs-nodets` test, the client logs that all the tests are successful and it does call `connection.end()`. However it still times out for some reason... This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > add typescript directory support > > > Key: THRIFT-3143 > URL: https://issues.apache.org/jira/browse/THRIFT-3143 > Project: Thrift > Issue Type: New Feature > Components: Node.js - Compiler >Reporter: Kazuki Yasufuku >Priority: Major > > Current typescript support is only work for browser, and generated d.ts uses > internal module. > So, it's hard to use for typescript in node.js. > To solve probrem, I make a pull request that generate typescript code > directory. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (THRIFT-3143) add typescript directory support
[ https://issues.apache.org/jira/browse/THRIFT-3143?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16699027#comment-16699027 ] ASF GitHub Bot commented on THRIFT-3143: mustafa-cosar commented on issue #1630: THRIFT-3143: Add nodets support URL: https://github.com/apache/thrift/pull/1630#issuecomment-441659089 The `nodets` uses `nodejs` as a basis so known failures for `nodejs` probably applies to `nodets` as well. I will check if there are any that `nodejs` doesn't fail. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > add typescript directory support > > > Key: THRIFT-3143 > URL: https://issues.apache.org/jira/browse/THRIFT-3143 > Project: Thrift > Issue Type: New Feature > Components: Node.js - Compiler >Reporter: Kazuki Yasufuku >Priority: Major > > Current typescript support is only work for browser, and generated d.ts uses > internal module. > So, it's hard to use for typescript in node.js. > To solve probrem, I make a pull request that generate typescript code > directory. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (THRIFT-3143) add typescript directory support
[ https://issues.apache.org/jira/browse/THRIFT-3143?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16698925#comment-16698925 ] ASF GitHub Bot commented on THRIFT-3143: jeking3 commented on issue #1630: THRIFT-3143: Add nodets support URL: https://github.com/apache/thrift/pull/1630#issuecomment-441640052 There are still 41 failing cross tests: ``` === *** Following 41 failures were unexpected ***: If it is introduced by you, please fix it before submitting the code. === server-client: protocol: transport: result: c_glib-nodets binarybuffered-ip failure(timeout) c_glib-nodets multi-binary buffered-ip failure(timeout) d-nodetsbinarybuffered-ip failure(1) go-nodets binarybuffered-ip failure(timeout) java-nodets binarybuffered-ip failure(timeout) java-nodets multi-binary buffered-ip failure(timeout) hs-nodets binarybuffered-ip failure(timeout) py-nodets accel-binary buffered-ip failure(timeout) py-nodets binarybuffered-ip failure(timeout) py3-nodets accel-binary buffered-ip failure(timeout) py3-nodets binarybuffered-ip failure(timeout) cpp-nodets binarybuffered-ip failure(timeout) cpp-nodets multi-binary buffered-ip failure(timeout) rb-nodets accel-binary buffered-ip failure(timeout) rb-nodets binarybuffered-ip failure(timeout) csharp-nodets binarybuffered-ip failure(timeout) netcore-nodets binarybuffered-ip failure(timeout) perl-nodets multi-binary buffered-ip failure(timeout) perl-nodets binarybuffered-ip failure(timeout) erl-nodets binarybuffered-ip failure(timeout) rs-nodets binarybuffered-ip failure(timeout) rs-nodets multi-binary buffered-ip failure(timeout) nodets-c_glib binarybuffered-ip failure(1) nodets-cl binarybuffered-ip failure(1) nodets-dbinarybuffered-ip failure(1) nodets-go binarybuffered-ip failure(1) nodets-hs binarybuffered-ip failure(1) nodets-py binary-accel buffered-ip failure(1) nodets-py binarybuffered-ip failure(1) nodets-java binarybuffered-ip failure(64) nodets-nodejs binarybuffered-ip failure(1) nodets-cpp binarybuffered-ip failure(64) nodets-py3 binary-accel buffered-ip failure(1) nodets-py3 binarybuffered-ip failure(1) nodets-php binary-accel buffered-ip failure(255) nodets-php binarybuffered-ip failure(255) nodets-csharp binarybuffered-ip failure(64) nodets-lua binarybuffered-ip failure(timeout) nodets-rs binarybuffered-ip failure(1) nodets-perl binarybuffered-ip failure(111) nodets-nodets binarybuffered-ip failure(timeout) === ``` This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > add typescript directory support > > > Key: THRIFT-3143 > URL: https://issues.apache.org/jira/browse/THRIFT-3143 > Project: Thrift > Issue Type: New Feature > Components: Node.js - Compiler >
[jira] [Commented] (THRIFT-3143) add typescript directory support
[ https://issues.apache.org/jira/browse/THRIFT-3143?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16698592#comment-16698592 ] ASF GitHub Bot commented on THRIFT-3143: mustafa-cosar edited a comment on issue #1630: THRIFT-3143: Add nodets support URL: https://github.com/apache/thrift/pull/1630#issuecomment-441549384 OK, I tried locally with `cpp,java,nodejs` servers and clients. The cross tests with above languages passed. @jeking3 If there is no other failure, can we merge this? This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > add typescript directory support > > > Key: THRIFT-3143 > URL: https://issues.apache.org/jira/browse/THRIFT-3143 > Project: Thrift > Issue Type: New Feature > Components: Node.js - Compiler >Reporter: Kazuki Yasufuku >Priority: Major > > Current typescript support is only work for browser, and generated d.ts uses > internal module. > So, it's hard to use for typescript in node.js. > To solve probrem, I make a pull request that generate typescript code > directory. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (THRIFT-3143) add typescript directory support
[ https://issues.apache.org/jira/browse/THRIFT-3143?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16698591#comment-16698591 ] ASF GitHub Bot commented on THRIFT-3143: mustafa-cosar commented on issue #1630: THRIFT-3143: Add nodets support URL: https://github.com/apache/thrift/pull/1630#issuecomment-441549384 OK, I tried locally with cpp,java,nodejs servers and clients. The cross tests with above languages passed. @jeking3 If there is no other failure, can we merge this? This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > add typescript directory support > > > Key: THRIFT-3143 > URL: https://issues.apache.org/jira/browse/THRIFT-3143 > Project: Thrift > Issue Type: New Feature > Components: Node.js - Compiler >Reporter: Kazuki Yasufuku >Priority: Major > > Current typescript support is only work for browser, and generated d.ts uses > internal module. > So, it's hard to use for typescript in node.js. > To solve probrem, I make a pull request that generate typescript code > directory. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (THRIFT-4671) c glib is unable to handle client close unexpectedly
[ https://issues.apache.org/jira/browse/THRIFT-4671?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16697529#comment-16697529 ] ASF GitHub Bot commented on THRIFT-4671: jeking3 closed pull request #1635: THRIFT-4671: handle client's unexpected close. URL: https://github.com/apache/thrift/pull/1635 This is a PR merged from a forked repository. As GitHub hides the original diff on merge, it is displayed below for the sake of provenance: As this is a foreign pull request (from a fork), the diff is supplied below (as it won't show otherwise due to GitHub magic): diff --git a/lib/c_glib/src/thrift/c_glib/protocol/thrift_protocol.c b/lib/c_glib/src/thrift/c_glib/protocol/thrift_protocol.c index 1433725759..8296a8cad1 100644 --- a/lib/c_glib/src/thrift/c_glib/protocol/thrift_protocol.c +++ b/lib/c_glib/src/thrift/c_glib/protocol/thrift_protocol.c @@ -465,7 +465,7 @@ thrift_protocol_skip (ThriftProtocol *protocol, ThriftType type, GError **error) } case T_STRUCT: { -guint32 result = 0; +gint32 result = 0; gchar *name; gint16 fid; ThriftType ftype; @@ -475,6 +475,10 @@ thrift_protocol_skip (ThriftProtocol *protocol, ThriftType type, GError **error) { result += thrift_protocol_read_field_begin (protocol, , , , error); + if (result < 0) + { +return result; + } if (ftype == T_STOP) { break; @@ -487,7 +491,7 @@ thrift_protocol_skip (ThriftProtocol *protocol, ThriftType type, GError **error) } case T_SET: { -guint32 result = 0; +gint32 result = 0; ThriftType elem_type; guint32 i, size; result += thrift_protocol_read_set_begin (protocol, _type, , @@ -501,7 +505,7 @@ thrift_protocol_skip (ThriftProtocol *protocol, ThriftType type, GError **error) } case T_MAP: { -guint32 result = 0; +gint32 result = 0; ThriftType elem_type; ThriftType key_type; guint32 i, size; @@ -517,7 +521,7 @@ thrift_protocol_skip (ThriftProtocol *protocol, ThriftType type, GError **error) } case T_LIST: { -guint32 result = 0; +gint32 result = 0; ThriftType elem_type; guint32 i, size; result += thrift_protocol_read_list_begin (protocol, _type, , This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > c glib is unable to handle client close unexpectedly > > > Key: THRIFT-4671 > URL: https://issues.apache.org/jira/browse/THRIFT-4671 > Project: Thrift > Issue Type: Bug > Components: C glib - Library >Affects Versions: 0.11.0 > Environment: ubuntu 16.04 > thrift sever: c_glib > thrift client: c_glib, go >Reporter: lixiasong >Priority: Major > Labels: easyfix, ready-to-commit > Original Estimate: 24h > Remaining Estimate: 24h > > When using c_glib thrift server. Once a client connects to the server *but > does nothing (didn't send data to server) when being connected*. If the > client closed unexpectedly(such as being killed) at the moment, *the > thrift-work thread for that client wont't be closed* and read the closed > client sock fd in the server forever. > *I*t has been fixed in my branch, and will be tested and committed later if > it runs well. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (THRIFT-3143) add typescript directory support
[ https://issues.apache.org/jira/browse/THRIFT-3143?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16697155#comment-16697155 ] ASF GitHub Bot commented on THRIFT-3143: jeking3 commented on issue #1630: THRIFT-3143: Add nodets support URL: https://github.com/apache/thrift/pull/1630#issuecomment-441246486 You can run the cross test locally if you use the docker build container. First you prepare the environment: ``` user@ubuntu:~/thrift$ docker pull thrift/thrift-build:ubuntu-bionic user@ubuntu:~/thrift$ docker run -v $(pwd):/thrift/src:rw -it thrift/thrift-build:ubuntu-bionic /bin/bash root@8d4584e2e789:/thrift/src# ./bootstrap.sh root@8d4584e2e789:/thrift/src# ./configure root@8d4584e2e789:/thrift/src# make precross ``` Next you can run a specific combination of tests: ``` root@8d4584e2e789:/thrift/src# test/test.py --retry-count 5 --skip-known-failures --server cpp --client nodets ``` That will help you prepare your next pull request for success. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > add typescript directory support > > > Key: THRIFT-3143 > URL: https://issues.apache.org/jira/browse/THRIFT-3143 > Project: Thrift > Issue Type: New Feature > Components: Node.js - Compiler >Reporter: Kazuki Yasufuku >Priority: Major > > Current typescript support is only work for browser, and generated d.ts uses > internal module. > So, it's hard to use for typescript in node.js. > To solve probrem, I make a pull request that generate typescript code > directory. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (THRIFT-3143) add typescript directory support
[ https://issues.apache.org/jira/browse/THRIFT-3143?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16697063#comment-16697063 ] ASF GitHub Bot commented on THRIFT-3143: mustafa-cosar edited a comment on issue #1630: THRIFT-3143: Add nodets support URL: https://github.com/apache/thrift/pull/1630#issuecomment-441179965 Hi @jeking3 I added nodets to cross test suite ...or not. I think I missed something, the test didn't run. I will check why. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > add typescript directory support > > > Key: THRIFT-3143 > URL: https://issues.apache.org/jira/browse/THRIFT-3143 > Project: Thrift > Issue Type: New Feature > Components: Node.js - Compiler >Reporter: Kazuki Yasufuku >Priority: Major > > Current typescript support is only work for browser, and generated d.ts uses > internal module. > So, it's hard to use for typescript in node.js. > To solve probrem, I make a pull request that generate typescript code > directory. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (THRIFT-3143) add typescript directory support
[ https://issues.apache.org/jira/browse/THRIFT-3143?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16696504#comment-16696504 ] ASF GitHub Bot commented on THRIFT-3143: mustafa-cosar commented on issue #1630: THRIFT-3143: Add nodets support URL: https://github.com/apache/thrift/pull/1630#issuecomment-441179965 Hi @jeking3 I added nodets to cross test suite This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > add typescript directory support > > > Key: THRIFT-3143 > URL: https://issues.apache.org/jira/browse/THRIFT-3143 > Project: Thrift > Issue Type: New Feature > Components: Node.js - Compiler >Reporter: Kazuki Yasufuku >Priority: Major > > Current typescript support is only work for browser, and generated d.ts uses > internal module. > So, it's hard to use for typescript in node.js. > To solve probrem, I make a pull request that generate typescript code > directory. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (THRIFT-4674) Add stream context support into PHP/THttpClient
[ https://issues.apache.org/jira/browse/THRIFT-4674?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16696488#comment-16696488 ] ASF GitHub Bot commented on THRIFT-4674: edefimov opened a new pull request #1636: THRIFT-4674 Added stream context support for PHP THttpClient URL: https://github.com/apache/thrift/pull/1636 This PR adds stream context options support into PHP/THttpClient. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > Add stream context support into PHP/THttpClient > --- > > Key: THRIFT-4674 > URL: https://issues.apache.org/jira/browse/THRIFT-4674 > Project: Thrift > Issue Type: Improvement >Reporter: Evgeniy Efimov >Priority: Major > > In some use-cases it is required to pass additional data into context for > THttpClient, client certificate as an example. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (THRIFT-4671) c glib is unable to handle client close unexpectedly
[ https://issues.apache.org/jira/browse/THRIFT-4671?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16696306#comment-16696306 ] ASF GitHub Bot commented on THRIFT-4671: BoringKen opened a new pull request #1635: THRIFT-4671: handle client's unexpected close. URL: https://github.com/apache/thrift/pull/1635 THRIFT-: handle client's unexpected close. Avoid the server work thread falling into dead loop. Client: c_glib This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > c glib is unable to handle client close unexpectedly > > > Key: THRIFT-4671 > URL: https://issues.apache.org/jira/browse/THRIFT-4671 > Project: Thrift > Issue Type: Bug > Components: C glib - Library >Affects Versions: 0.11.0 > Environment: ubuntu 16.04 > thrift sever: c_glib > thrift client: c_glib, go >Reporter: lixiasong >Priority: Major > Labels: easyfix, ready-to-commit > Original Estimate: 24h > Remaining Estimate: 24h > > When using c_glib thrift server. Once a client connects to the server *but > does nothing (didn't send data to server) when being connected*. If the > client closed unexpectedly(such as being killed) at the moment, *the > thrift-work thread for that client wont't be closed* and read the closed > client sock fd in the server forever. > *I*t has been fixed in my branch, and will be tested and committed later if > it runs well. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (THRIFT-4654) Thrift Dart port is not compatible with Dart 2
[ https://issues.apache.org/jira/browse/THRIFT-4654?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16695907#comment-16695907 ] ASF GitHub Bot commented on THRIFT-4654: jeking3 commented on issue #1617: THRIFT-4654 Minor fixes for dart 1 & 2 compatibility (backwards compatible) URL: https://github.com/apache/thrift/pull/1617#issuecomment-441031973 Reminder: this needs more work. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > Thrift Dart port is not compatible with Dart 2 > -- > > Key: THRIFT-4654 > URL: https://issues.apache.org/jira/browse/THRIFT-4654 > Project: Thrift > Issue Type: Improvement > Components: Dart - Library > Environment: Any OS, under Dart 2+ >Reporter: Rob Becker >Assignee: James E. King III >Priority: Minor > Original Estimate: 2h > Remaining Estimate: 2h > > It is not possible to use the Dart thrift code under Dart 2. Dart 2 requires > opting in via a setting in the pubspec.yaml and the code needs to update the > use of all caps constant names to the new camel case names. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (THRIFT-4668) make socket backlog configurable for python
[ https://issues.apache.org/jira/browse/THRIFT-4668?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16695904#comment-16695904 ] ASF GitHub Bot commented on THRIFT-4668: jeking3 closed pull request #1631: THRIFT-4668: make socket backlog configurable for python URL: https://github.com/apache/thrift/pull/1631 This is a PR merged from a forked repository. As GitHub hides the original diff on merge, it is displayed below for the sake of provenance: As this is a foreign pull request (from a fork), the diff is supplied below (as it won't show otherwise due to GitHub magic): diff --git a/lib/py/src/transport/TSocket.py b/lib/py/src/transport/TSocket.py index c91de9d701..a7d661703d 100644 --- a/lib/py/src/transport/TSocket.py +++ b/lib/py/src/transport/TSocket.py @@ -159,6 +159,15 @@ def __init__(self, host=None, port=9090, unix_socket=None, socket_family=socket. self._unix_socket = unix_socket self._socket_family = socket_family self.handle = None +self._backlog = 128 + +def setBacklog(self, backlog=None): +if not self.handle: +self._backlog = backlog +else: +# We cann't update backlog when it is already listening, since the +# handle has been created. +logger.warn('You have to set backlog before listen.') def listen(self): res0 = self._resolveAddr() @@ -183,7 +192,7 @@ def listen(self): if hasattr(self.handle, 'settimeout'): self.handle.settimeout(None) self.handle.bind(res[4]) -self.handle.listen(128) +self.handle.listen(self._backlog) def accept(self): client, addr = self.handle.accept() This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > make socket backlog configurable for python > --- > > Key: THRIFT-4668 > URL: https://issues.apache.org/jira/browse/THRIFT-4668 > Project: Thrift > Issue Type: Improvement > Components: Python - Library >Affects Versions: 0.11.0 >Reporter: lsh >Priority: Minor > Labels: patch > > The backlog parameter of socket.listen was hard coded, so I can't customize > it in my code. > Adding `setBacklog` to `TServerSocket` can resolve my problem. :) -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (THRIFT-4668) make socket backlog configurable for python
[ https://issues.apache.org/jira/browse/THRIFT-4668?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16695903#comment-16695903 ] ASF GitHub Bot commented on THRIFT-4668: jeking3 commented on issue #1631: THRIFT-4668: make socket backlog configurable for python URL: https://github.com/apache/thrift/pull/1631#issuecomment-441031649 Build failure is the SBCL issue we know about. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > make socket backlog configurable for python > --- > > Key: THRIFT-4668 > URL: https://issues.apache.org/jira/browse/THRIFT-4668 > Project: Thrift > Issue Type: Improvement > Components: Python - Library >Affects Versions: 0.11.0 >Reporter: lsh >Priority: Minor > Labels: patch > > The backlog parameter of socket.listen was hard coded, so I can't customize > it in my code. > Adding `setBacklog` to `TServerSocket` can resolve my problem. :) -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (THRIFT-4670) Twisted, slots, and void method fails with "object has no attribute 'success'"
[ https://issues.apache.org/jira/browse/THRIFT-4670?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16695889#comment-16695889 ] ASF GitHub Bot commented on THRIFT-4670: jeking3 closed pull request #1632: THRIFT-4670: Twisted, slots, and void method fails with "object has no attribute 'success'" URL: https://github.com/apache/thrift/pull/1632 This is a PR merged from a forked repository. As GitHub hides the original diff on merge, it is displayed below for the sake of provenance: As this is a foreign pull request (from a fork), the diff is supplied below (as it won't show otherwise due to GitHub magic): diff --git a/compiler/cpp/src/thrift/generate/t_py_generator.cc b/compiler/cpp/src/thrift/generate/t_py_generator.cc index 9c82b6cf2a..f0a153ca8b 100644 --- a/compiler/cpp/src/thrift/generate/t_py_generator.cc +++ b/compiler/cpp/src/thrift/generate/t_py_generator.cc @@ -1961,8 +1961,10 @@ void t_py_generator::generate_process_function(t_service* tservice, t_function* indent(f_service_) << "def write_results_success_" << tfunction->get_name() << "(self, success, result, seqid, oprot):" << endl; indent_up(); - f_service_ << indent() << "result.success = success" << endl - << indent() << "oprot.writeMessageBegin(\"" << tfunction->get_name() + if (!tfunction->get_returntype()->is_void()) { +f_service_ << indent() << "result.success = success" << endl; + } + f_service_ << indent() << "oprot.writeMessageBegin(\"" << tfunction->get_name() << "\", TMessageType.REPLY, seqid)" << endl << indent() << "result.write(oprot)" << endl << indent() << "oprot.writeMessageEnd()" << endl This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > Twisted, slots, and void method fails with "object has no attribute 'success'" > -- > > Key: THRIFT-4670 > URL: https://issues.apache.org/jira/browse/THRIFT-4670 > Project: Thrift > Issue Type: Bug > Components: Python - Compiler >Affects Versions: 0.9.3, 0.10.0, 0.11.0 >Reporter: Palmer >Priority: Minor > > When generating Twisted code for a void method, the compiler accidentally > assigns a value to the result.success field of the result object, even > though, as a void method, there is no success value and the result object has > no such field. If the slots option is not specified as well, this does not > cause a problem, it just sets a new field that is never used. However, with > the slots option, attempting to set this undefined field causes the error > "object has no attribute 'success'" -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (THRIFT-3143) add typescript directory support
[ https://issues.apache.org/jira/browse/THRIFT-3143?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16695779#comment-16695779 ] ASF GitHub Bot commented on THRIFT-3143: mustafa-cosar edited a comment on issue #1630: THRIFT-3143: Add nodets support URL: https://github.com/apache/thrift/pull/1630#issuecomment-440996256 I reverted C++11 features and rebased. One of the tests failed because of a lisp test. Not related to my changes. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > add typescript directory support > > > Key: THRIFT-3143 > URL: https://issues.apache.org/jira/browse/THRIFT-3143 > Project: Thrift > Issue Type: New Feature > Components: Node.js - Compiler >Reporter: Kazuki Yasufuku >Priority: Major > > Current typescript support is only work for browser, and generated d.ts uses > internal module. > So, it's hard to use for typescript in node.js. > To solve probrem, I make a pull request that generate typescript code > directory. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (THRIFT-3143) add typescript directory support
[ https://issues.apache.org/jira/browse/THRIFT-3143?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16695778#comment-16695778 ] ASF GitHub Bot commented on THRIFT-3143: mustafa-cosar commented on issue #1630: THRIFT-3143: Add nodets support URL: https://github.com/apache/thrift/pull/1630#issuecomment-440996256 One of the tests failed because of a lisp test. Not related to my changes. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > add typescript directory support > > > Key: THRIFT-3143 > URL: https://issues.apache.org/jira/browse/THRIFT-3143 > Project: Thrift > Issue Type: New Feature > Components: Node.js - Compiler >Reporter: Kazuki Yasufuku >Priority: Major > > Current typescript support is only work for browser, and generated d.ts uses > internal module. > So, it's hard to use for typescript in node.js. > To solve probrem, I make a pull request that generate typescript code > directory. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (THRIFT-4668) make socket backlog configurable for python
[ https://issues.apache.org/jira/browse/THRIFT-4668?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16695576#comment-16695576 ] ASF GitHub Bot commented on THRIFT-4668: lshgdut edited a comment on issue #1631: THRIFT-4668: make socket backlog configurable for python URL: https://github.com/apache/thrift/pull/1631#issuecomment-440920684 The value of `socket.SOMAXCONN` depends on different operate systems: - on Window 7, it is `2147483647` - on Centos or Debian, it is `128`, even I have set a different value to `/proc/sys/net/core/somaxconn` According to `Python-2.7.14/Modules/socketmodule.c`: ```c 4967 /* Maximum number of connections for "listen" */ 4968: #ifdef SOMAXCONN 4969: PyModule_AddIntConstant(m, "SOMAXCONN", SOMAXCONN); 4970 #else 4971: PyModule_AddIntConstant(m, "SOMAXCONN", 5); /* Common value */ 4972 #endif ``` The value of `SOMAXCONN` depend on system macro `SOMAXCONN`: - on Window 7, I can not explain why it is 2147483647 - on linux, `SOMAXCONN` is defined in '/usr/include/bits/socket.h +144', hard code to 128 Linux set `SOMAXCONN` macro to` core.sysctl_somaxconn` first, then check the sysctl value of 'net.core.somaxconn' existing or not and override it if possible. When socket listening, if `backlog` is greater than SOMAXCONN, the kernel will take the value of `min(backlog, SOMAXCONN)`. Here is linux kernel code below: ```c // From linux-3.10.0-693.25.4.el7/net/socket.c +1534 /* * Perform a listen. Basically, we allow the protocol to do anything * necessary for a listen, and if that works, we mark the socket as * ready for listening. */ SYSCALL_DEFINE2(listen, int, fd, int, backlog) { struct socket *sock; int err, fput_needed; int somaxconn; sock = sockfd_lookup_light(fd, , _needed); if (sock) { somaxconn = sock_net(sock->sk)->core.sysctl_somaxconn; if ((unsigned int)backlog > somaxconn) backlog = somaxconn; err = security_socket_listen(sock, backlog); if (!err) err = sock->ops->listen(sock, backlog); fput_light(sock->file, fput_needed); } return err; } ``` Therefore, limit `_backlog` to `socket.SOMAXCONN` will make `core.net.somaxconn` not working properly on linux. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > make socket backlog configurable for python > --- > > Key: THRIFT-4668 > URL: https://issues.apache.org/jira/browse/THRIFT-4668 > Project: Thrift > Issue Type: Improvement > Components: Python - Library >Affects Versions: 0.11.0 >Reporter: lsh >Priority: Minor > Labels: patch > > The backlog parameter of socket.listen was hard coded, so I can't customize > it in my code. > Adding `setBacklog` to `TServerSocket` can resolve my problem. :) -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (THRIFT-4668) make socket backlog configurable for python
[ https://issues.apache.org/jira/browse/THRIFT-4668?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16695577#comment-16695577 ] ASF GitHub Bot commented on THRIFT-4668: lshgdut edited a comment on issue #1631: THRIFT-4668: make socket backlog configurable for python URL: https://github.com/apache/thrift/pull/1631#issuecomment-440920684 The value of `socket.SOMAXCONN` depends on different operate systems: - on Window 7, it is `2147483647` - on Centos or Debian, it is `128`, even I have set a different value to `/proc/sys/net/core/somaxconn` According to `Python-2.7.14/Modules/socketmodule.c`: ```c 4967 /* Maximum number of connections for "listen" */ 4968: #ifdef SOMAXCONN 4969: PyModule_AddIntConstant(m, "SOMAXCONN", SOMAXCONN); 4970 #else 4971: PyModule_AddIntConstant(m, "SOMAXCONN", 5); /* Common value */ 4972 #endif ``` The value of `SOMAXCONN` depend on system macro `SOMAXCONN`: - on Window 7, I can not explain why it is 2147483647 - on linux, `SOMAXCONN` is defined in '/usr/include/bits/socket.h +144', hard code to 128 Linux set `SOMAXCONN` macro to` core.sysctl_somaxconn` first, then check the sysctl value of 'net.core.somaxconn' existing or not and override it if possible. When socket listening, if `backlog` is greater than SOMAXCONN, the kernel will take the value of `min(backlog, SOMAXCONN)`. Here is linux kernel code below: ```c // From linux-3.10.0-693.25.4.el7/net/socket.c +1534 /* * Perform a listen. Basically, we allow the protocol to do anything * necessary for a listen, and if that works, we mark the socket as * ready for listening. */ SYSCALL_DEFINE2(listen, int, fd, int, backlog) { struct socket *sock; int err, fput_needed; int somaxconn; sock = sockfd_lookup_light(fd, , _needed); if (sock) { somaxconn = sock_net(sock->sk)->core.sysctl_somaxconn; if ((unsigned int)backlog > somaxconn) backlog = somaxconn; err = security_socket_listen(sock, backlog); if (!err) err = sock->ops->listen(sock, backlog); fput_light(sock->file, fput_needed); } return err; } ``` So limit `_backlog` to `socket.SOMAXCONN` will make `core.net.somaxconn` not working properly on linux. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > make socket backlog configurable for python > --- > > Key: THRIFT-4668 > URL: https://issues.apache.org/jira/browse/THRIFT-4668 > Project: Thrift > Issue Type: Improvement > Components: Python - Library >Affects Versions: 0.11.0 >Reporter: lsh >Priority: Minor > Labels: patch > > The backlog parameter of socket.listen was hard coded, so I can't customize > it in my code. > Adding `setBacklog` to `TServerSocket` can resolve my problem. :) -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (THRIFT-4668) make socket backlog configurable for python
[ https://issues.apache.org/jira/browse/THRIFT-4668?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16695573#comment-16695573 ] ASF GitHub Bot commented on THRIFT-4668: lshgdut commented on issue #1631: THRIFT-4668: make socket backlog configurable for python URL: https://github.com/apache/thrift/pull/1631#issuecomment-440920684 The value of `socket.SOMAXCONN` depends on different operate systems: - on Window 7, it is `2147483647` - on Centos or Debian, it is `128`, even I have set a different value to `/proc/sys/net/core/somaxconn` According to `Python-2.7.14/Modules/socketmodule.c`: ```c 4967 /* Maximum number of connections for "listen" */ 4968: #ifdef SOMAXCONN 4969: PyModule_AddIntConstant(m, "SOMAXCONN", SOMAXCONN); 4970 #else 4971: PyModule_AddIntConstant(m, "SOMAXCONN", 5); /* Common value */ 4972 #endif ``` The value of `SOMAXCONN` depend on system macro `SOMAXCONN`: - on Window 7, I can not explain why it is 2147483647 - on linux, `SOMAXCONN` is defined in '/usr/include/bits/socket.h +144', hard code to 128 Linux set `SOMAXCONN` macro to` core.sysctl_somaxconn` first, then check the sysctl value of 'net.core.somaxconn' existing or not and override it if possible. When socket listening, if `backlog` is greater than SOMAXCONN, the kernel will take the value of `min(backlog, SOMAXCONN)`. Here is linux kernel code below: ```c // From linux-3.10.0-693.25.4.el7/net/socket.c +1534 /* * Perform a listen. Basically, we allow the protocol to do anything * necessary for a listen, and if that works, we mark the socket as * ready for listening. */ SYSCALL_DEFINE2(listen, int, fd, int, backlog) { struct socket *sock; int err, fput_needed; int somaxconn; sock = sockfd_lookup_light(fd, , _needed); if (sock) { somaxconn = sock_net(sock->sk)->core.sysctl_somaxconn; if ((unsigned int)backlog > somaxconn) backlog = somaxconn; err = security_socket_listen(sock, backlog); if (!err) err = sock->ops->listen(sock, backlog); fput_light(sock->file, fput_needed); } return err; } ``` So doesn't it need to check the value of `_backlog` or not? May be a warning better? This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > make socket backlog configurable for python > --- > > Key: THRIFT-4668 > URL: https://issues.apache.org/jira/browse/THRIFT-4668 > Project: Thrift > Issue Type: Improvement > Components: Python - Library >Affects Versions: 0.11.0 >Reporter: lsh >Priority: Minor > Labels: patch > > The backlog parameter of socket.listen was hard coded, so I can't customize > it in my code. > Adding `setBacklog` to `TServerSocket` can resolve my problem. :) -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (THRIFT-4668) make socket backlog configurable for python
[ https://issues.apache.org/jira/browse/THRIFT-4668?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16695534#comment-16695534 ] ASF GitHub Bot commented on THRIFT-4668: lshgdut commented on a change in pull request #1631: THRIFT-4668: make socket backlog configurable for python URL: https://github.com/apache/thrift/pull/1631#discussion_r235592274 ## File path: lib/py/src/transport/TSocket.py ## @@ -183,7 +187,7 @@ def listen(self): if hasattr(self.handle, 'settimeout'): self.handle.settimeout(None) self.handle.bind(res[4]) -self.handle.listen(128) +self.handle.listen(self._backlog) Review comment: We shoud not use `self.handle.listen(self._backlog or 128)` to preserve the existing behavior, because the backlog parameter accept a zero value. Plz have a look at the python source code below: ```c // From Python-2.7.14/Modules/socketmodule.c /* s.listen(n) method */ static PyObject * sock_listen(PySocketSockObject *s, PyObject *arg) { // some code before /* To avoid problems on systems that don't allow a negative backlog * (which doesn't make sense anyway) we force a minimum value of 0. */ if (backlog < 0) backlog = 0; res = listen(s->sock_fd, backlog); // some code more } ``` I have think about `self._backlog if self._backlog is None else 128 `, and the code seems to be redundant with bad smell. So declaring the _backlog attribute with a default value of 128 in the class constructor seems to be a better way, isn't? On the other hand, `__init__` will be called before listen, when the listen method calling, `this.handle.listen` will receive a `_backlog` with a default value of 128, existing behavior preserved! This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > make socket backlog configurable for python > --- > > Key: THRIFT-4668 > URL: https://issues.apache.org/jira/browse/THRIFT-4668 > Project: Thrift > Issue Type: Improvement > Components: Python - Library >Affects Versions: 0.11.0 >Reporter: lsh >Priority: Minor > Labels: patch > > The backlog parameter of socket.listen was hard coded, so I can't customize > it in my code. > Adding `setBacklog` to `TServerSocket` can resolve my problem. :) -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (THRIFT-4668) make socket backlog configurable for python
[ https://issues.apache.org/jira/browse/THRIFT-4668?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16695533#comment-16695533 ] ASF GitHub Bot commented on THRIFT-4668: lshgdut commented on a change in pull request #1631: THRIFT-4668: make socket backlog configurable for python URL: https://github.com/apache/thrift/pull/1631#discussion_r235596635 ## File path: lib/py/src/transport/TSocket.py ## @@ -159,6 +159,10 @@ def __init__(self, host=None, port=9090, unix_socket=None, socket_family=socket. self._unix_socket = unix_socket self._socket_family = socket_family self.handle = None +self._backlog = 128 + +def setBacklog(self, backlog): Review comment: Yeah, nothing works except `_backlog` changed. I should add some warning for it. Thank for your suggestions! This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > make socket backlog configurable for python > --- > > Key: THRIFT-4668 > URL: https://issues.apache.org/jira/browse/THRIFT-4668 > Project: Thrift > Issue Type: Improvement > Components: Python - Library >Affects Versions: 0.11.0 >Reporter: lsh >Priority: Minor > Labels: patch > > The backlog parameter of socket.listen was hard coded, so I can't customize > it in my code. > Adding `setBacklog` to `TServerSocket` can resolve my problem. :) -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (THRIFT-4668) make socket backlog configurable for python
[ https://issues.apache.org/jira/browse/THRIFT-4668?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16695519#comment-16695519 ] ASF GitHub Bot commented on THRIFT-4668: lshgdut commented on a change in pull request #1631: THRIFT-4668: make socket backlog configurable for python URL: https://github.com/apache/thrift/pull/1631#discussion_r235592434 ## File path: lib/py/src/transport/TSocket.py ## @@ -159,6 +159,10 @@ def __init__(self, host=None, port=9090, unix_socket=None, socket_family=socket. self._unix_socket = unix_socket self._socket_family = socket_family self.handle = None +self._backlog = 128 Review comment: Please look at the comment below :) https://github.com/apache/thrift/pull/1631#discussion_r235592274 This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > make socket backlog configurable for python > --- > > Key: THRIFT-4668 > URL: https://issues.apache.org/jira/browse/THRIFT-4668 > Project: Thrift > Issue Type: Improvement > Components: Python - Library >Affects Versions: 0.11.0 >Reporter: lsh >Priority: Minor > Labels: patch > > The backlog parameter of socket.listen was hard coded, so I can't customize > it in my code. > Adding `setBacklog` to `TServerSocket` can resolve my problem. :) -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (THRIFT-4668) make socket backlog configurable for python
[ https://issues.apache.org/jira/browse/THRIFT-4668?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16695516#comment-16695516 ] ASF GitHub Bot commented on THRIFT-4668: lshgdut commented on a change in pull request #1631: THRIFT-4668: make socket backlog configurable for python URL: https://github.com/apache/thrift/pull/1631#discussion_r235592274 ## File path: lib/py/src/transport/TSocket.py ## @@ -183,7 +187,7 @@ def listen(self): if hasattr(self.handle, 'settimeout'): self.handle.settimeout(None) self.handle.bind(res[4]) -self.handle.listen(128) +self.handle.listen(self._backlog) Review comment: We shoud not use `self.handle.listen(self._backlog or 128)` to preserve the existing behavior, because the backlog parameter accept a zero value. Plz have a look at the python source code below: ```c // From Python-2.7.14/Modules/socketmodule.c /* s.listen(n) method */ static PyObject * sock_listen(PySocketSockObject *s, PyObject *arg) { // some code before /* To avoid problems on systems that don't allow a negative backlog * (which doesn't make sense anyway) we force a minimum value of 0. */ if (backlog < 0) backlog = 0; res = listen(s->sock_fd, backlog); // some code more } ``` I have think about `self._backlog if self._backlog is None else 128 `, and the code seems to be redundant with bad smell. So declaring the _backlog attribute with a default value of 128 in the class constructor seems to be a better way, isn't? On the other hand, `__init__` will be called before listen, while the listen method called, `this.handle.listen` will receive a `_backlog` with a default value of 128, existing behavior preserved! This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > make socket backlog configurable for python > --- > > Key: THRIFT-4668 > URL: https://issues.apache.org/jira/browse/THRIFT-4668 > Project: Thrift > Issue Type: Improvement > Components: Python - Library >Affects Versions: 0.11.0 >Reporter: lsh >Priority: Minor > Labels: patch > > The backlog parameter of socket.listen was hard coded, so I can't customize > it in my code. > Adding `setBacklog` to `TServerSocket` can resolve my problem. :) -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (THRIFT-4670) Twisted, slots, and void method fails with "object has no attribute 'success'"
[ https://issues.apache.org/jira/browse/THRIFT-4670?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16695217#comment-16695217 ] ASF GitHub Bot commented on THRIFT-4670: DaGenix opened a new pull request #1632: THRIFT-4670: Twisted, slots, and void method fails with "object has no attribute 'success'" URL: https://github.com/apache/thrift/pull/1632 For a void method, there is no success value, so, it is an error to attempt to assign one to the result object. This error is harmless unless slots is also specified - with slots specified, the attempt to assign to a non-existent field causes an error which makes the service method fail. Client: py This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > Twisted, slots, and void method fails with "object has no attribute 'success'" > -- > > Key: THRIFT-4670 > URL: https://issues.apache.org/jira/browse/THRIFT-4670 > Project: Thrift > Issue Type: Bug > Components: Python - Compiler >Affects Versions: 0.9.3, 0.10.0, 0.11.0 >Reporter: Palmer >Priority: Minor > > When generating Twisted code for a void method, the compiler accidentally > assigns a value to the result.success field of the result object, even > though, as a void method, there is no success value and the result object has > no such field. If the slots option is not specified as well, this does not > cause a problem, it just sets a new field that is never used. However, with > the slots option, attempting to set this undefined field causes the error > "object has no attribute 'success'" -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (THRIFT-4668) make socket backlog configurable for python
[ https://issues.apache.org/jira/browse/THRIFT-4668?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16694926#comment-16694926 ] ASF GitHub Bot commented on THRIFT-4668: jeking3 commented on a change in pull request #1631: THRIFT-4668: make socket backlog configurable for python URL: https://github.com/apache/thrift/pull/1631#discussion_r235463022 ## File path: lib/py/src/transport/TSocket.py ## @@ -183,7 +187,7 @@ def listen(self): if hasattr(self.handle, 'settimeout'): self.handle.settimeout(None) self.handle.bind(res[4]) -self.handle.listen(128) +self.handle.listen(self._backlog) Review comment: To preserve the existing behavior, how about `self.handle.listen(self._backlog or 128)`, in combination with a default value of None in the class constructor, this eliminates the need for setBacklog(). This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > make socket backlog configurable for python > --- > > Key: THRIFT-4668 > URL: https://issues.apache.org/jira/browse/THRIFT-4668 > Project: Thrift > Issue Type: Improvement > Components: Python - Library >Affects Versions: 0.11.0 >Reporter: lsh >Priority: Minor > Labels: patch > > The backlog parameter of socket.listen was hard coded, so I can't customize > it in my code. > Adding `setBacklog` to `TServerSocket` can resolve my problem. :) -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (THRIFT-4668) make socket backlog configurable for python
[ https://issues.apache.org/jira/browse/THRIFT-4668?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16694927#comment-16694927 ] ASF GitHub Bot commented on THRIFT-4668: jeking3 commented on a change in pull request #1631: THRIFT-4668: make socket backlog configurable for python URL: https://github.com/apache/thrift/pull/1631#discussion_r235462834 ## File path: lib/py/src/transport/TSocket.py ## @@ -159,6 +159,10 @@ def __init__(self, host=None, port=9090, unix_socket=None, socket_family=socket. self._unix_socket = unix_socket self._socket_family = socket_family self.handle = None +self._backlog = 128 Review comment: Should backlog be a named parameter with a default of None? This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > make socket backlog configurable for python > --- > > Key: THRIFT-4668 > URL: https://issues.apache.org/jira/browse/THRIFT-4668 > Project: Thrift > Issue Type: Improvement > Components: Python - Library >Affects Versions: 0.11.0 >Reporter: lsh >Priority: Minor > Labels: patch > > The backlog parameter of socket.listen was hard coded, so I can't customize > it in my code. > Adding `setBacklog` to `TServerSocket` can resolve my problem. :) -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (THRIFT-4668) make socket backlog configurable for python
[ https://issues.apache.org/jira/browse/THRIFT-4668?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16694925#comment-16694925 ] ASF GitHub Bot commented on THRIFT-4668: jeking3 commented on a change in pull request #1631: THRIFT-4668: make socket backlog configurable for python URL: https://github.com/apache/thrift/pull/1631#discussion_r235465457 ## File path: lib/py/src/transport/TSocket.py ## @@ -159,6 +159,10 @@ def __init__(self, host=None, port=9090, unix_socket=None, socket_family=socket. self._unix_socket = unix_socket self._socket_family = socket_family self.handle = None +self._backlog = 128 + +def setBacklog(self, backlog): Review comment: What happens when you call this when it is already listening? This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > make socket backlog configurable for python > --- > > Key: THRIFT-4668 > URL: https://issues.apache.org/jira/browse/THRIFT-4668 > Project: Thrift > Issue Type: Improvement > Components: Python - Library >Affects Versions: 0.11.0 >Reporter: lsh >Priority: Minor > Labels: patch > > The backlog parameter of socket.listen was hard coded, so I can't customize > it in my code. > Adding `setBacklog` to `TServerSocket` can resolve my problem. :) -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (THRIFT-4668) make socket backlog configurable for python
[ https://issues.apache.org/jira/browse/THRIFT-4668?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16694118#comment-16694118 ] ASF GitHub Bot commented on THRIFT-4668: lshgdut opened a new pull request #1631: THRIFT-4668: make socket backlog configurable URL: https://github.com/apache/thrift/pull/1631 The backlog parameter of socket.listen was hard coded, so I can't customize it in my code. Adding `setBacklog` to `TServerSocket` can resolve my problem. :) This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > make socket backlog configurable for python > --- > > Key: THRIFT-4668 > URL: https://issues.apache.org/jira/browse/THRIFT-4668 > Project: Thrift > Issue Type: Improvement > Components: Python - Library >Affects Versions: 0.11.0 >Reporter: lsh >Priority: Minor > Labels: patch > > The backlog parameter of socket.listen was hard coded, so I can't customize > it in my code. > Adding `setBacklog` to `TServerSocket` can resolve my problem. :) -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (THRIFT-3143) add typescript directory support
[ https://issues.apache.org/jira/browse/THRIFT-3143?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16693800#comment-16693800 ] ASF GitHub Bot commented on THRIFT-3143: jeking3 commented on a change in pull request #1630: THRIFT-3143: Add nodets support URL: https://github.com/apache/thrift/pull/1630#discussion_r235175049 ## File path: compiler/cpp/src/thrift/generate/t_js_generator.cc ## @@ -302,11 +298,11 @@ class t_js_generator : public t_oop_generator { string ts_declare() { return (ts_module_.empty() ? "declare " : ""); } /** - * Returns "?" if the given field is optional. + * Returns "?" if the given field is optional or has a default value. * @param t_field The field to check * @return string */ - string ts_get_req(t_field* field) { return (field->get_req() == t_field::T_OPTIONAL ? "?" : ""); } + string ts_get_req(t_field* field) {return (field->get_req() == t_field::T_OPTIONAL || field->get_value() != nullptr ? "?" : ""); } Review comment: This is still a C++03 compatible project and you have at least one C++11 concept in one of the files. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > add typescript directory support > > > Key: THRIFT-3143 > URL: https://issues.apache.org/jira/browse/THRIFT-3143 > Project: Thrift > Issue Type: New Feature > Components: Node.js - Compiler >Reporter: Kazuki Yasufuku >Priority: Major > > Current typescript support is only work for browser, and generated d.ts uses > internal module. > So, it's hard to use for typescript in node.js. > To solve probrem, I make a pull request that generate typescript code > directory. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (THRIFT-4207) Accelerated version of TBinaryProtocol allows invalid input to string fields.
[ https://issues.apache.org/jira/browse/THRIFT-4207?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16693773#comment-16693773 ] ASF GitHub Bot commented on THRIFT-4207: jeking3 commented on issue #1274: THRIFT-4207: Make sure Python Accelerated protocol does not allow invalid UTF-8 URL: https://github.com/apache/thrift/pull/1274#issuecomment-440424580 As I have been the only contributor to the conversation on this pull request for the last year, I'm closing this. Feel free to re-open if you have updates. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > Accelerated version of TBinaryProtocol allows invalid input to string fields. > - > > Key: THRIFT-4207 > URL: https://issues.apache.org/jira/browse/THRIFT-4207 > Project: Thrift > Issue Type: Bug > Components: Python - Library >Affects Versions: 0.10.0 >Reporter: Elvis Pranskevichus >Assignee: Aki Sukegawa >Priority: Major > > {{TBinaryProtocolAccelerated}} and {{TCompactProtocolAccelerated}} currently > accept arbitrary bytes as input to string fields even when {{py:utf8strings}} > is on. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (THRIFT-4207) Accelerated version of TBinaryProtocol allows invalid input to string fields.
[ https://issues.apache.org/jira/browse/THRIFT-4207?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16693774#comment-16693774 ] ASF GitHub Bot commented on THRIFT-4207: jeking3 closed pull request #1274: THRIFT-4207: Make sure Python Accelerated protocol does not allow invalid UTF-8 URL: https://github.com/apache/thrift/pull/1274 This is a PR merged from a forked repository. As GitHub hides the original diff on merge, it is displayed below for the sake of provenance: As this is a foreign pull request (from a fork), the diff is supplied below (as it won't show otherwise due to GitHub magic): diff --git a/lib/py/src/ext/protocol.tcc b/lib/py/src/ext/protocol.tcc index c025d0c968..e2e782a6c1 100644 --- a/lib/py/src/ext/protocol.tcc +++ b/lib/py/src/ext/protocol.tcc @@ -419,6 +419,8 @@ bool ProtocolBase::encodeValue(PyObject* value, TType type, PyObject* type case T_STRING: { ScopedPyObject nval; +Py_ssize_t len; +char *str; if (PyUnicode_Check(value)) { nval.reset(PyUnicode_AsUTF8String(value)); @@ -426,11 +428,21 @@ bool ProtocolBase::encodeValue(PyObject* value, TType type, PyObject* type return false; } } else { + if (isUtf8(typeargs)) { +if (PyBytes_AsStringAndSize(value, , ) < 0) { + return false; +} +// Check that input is a valid UTF-8 string. +nval.reset(PyUnicode_DecodeUTF8(str, len, 0)); +if (!nval) { + return false; +} + } Py_INCREF(value); nval.reset(value); } -Py_ssize_t len = PyBytes_Size(nval.get()); +len = PyBytes_Size(nval.get()); if (!detail::check_ssize_t_32(len)) { return false; } diff --git a/lib/py/src/protocol/TProtocol.py b/lib/py/src/protocol/TProtocol.py index fd20cb7906..588d997e57 100644 --- a/lib/py/src/protocol/TProtocol.py +++ b/lib/py/src/protocol/TProtocol.py @@ -118,6 +118,8 @@ def writeDouble(self, dub): pass def writeString(self, str_val): +if isinstance(str_val, bytes): +str_val = str_val.decode('utf8') self.writeBinary(str_to_binary(str_val)) def writeBinary(self, str_val): diff --git a/test/py/FastbinaryTest.py b/test/py/FastbinaryTest.py index 05c0bb6d15..2a87d5fddc 100755 --- a/test/py/FastbinaryTest.py +++ b/test/py/FastbinaryTest.py @@ -74,6 +74,9 @@ def isOpen(self): u"\x20\xce\x91\x74\x74\xce\xb1\xe2\x85\xbd\xce\xba"\ u"\xc7\x83\xe2\x80\xbc" +ooe_bad = OneOfEach() +ooe_bad.zomg_unicode = b'\xbe\xef\xff' + if sys.version_info[0] == 2 and os.environ.get('THRIFT_TEST_PY_NO_UTF8STRINGS'): ooe1.zomg_unicode = ooe1.zomg_unicode.encode('utf8') ooe2.zomg_unicode = ooe2.zomg_unicode.encode('utf8') @@ -167,6 +170,27 @@ def _check_read(self, o): pprint(repr(o)) raise Exception('read value mismatch') +def _check_bad_unicode(self, o): +if (sys.version_info[0] == 2 and +os.environ.get('THRIFT_TEST_PY_NO_UTF8STRINGS')): +return + +try: +prot_slow = self._slow(TTransport.TMemoryBuffer()) +o.write(prot_slow) +except UnicodeError: +pass +else: +raise Exception('UnicodeError not raised') + +try: +prot_fast = self._fast(TTransport.TMemoryBuffer()) +o.write(prot_fast) +except UnicodeError: +pass +else: +raise Exception('UnicodeError not raised') + def do_test(self): self._check_write(HolyMoley()) self._check_read(HolyMoley()) @@ -188,6 +212,8 @@ def do_test(self): self._check_read(Backwards(**{"first_tag2": 4, "second_tag1": 2})) +self._check_bad_unicode(ooe_bad) + # One case where the serialized form changes, but only superficially. o = Backwards(**{"first_tag2": 4, "second_tag1": 2}) trans_fast = TTransport.TMemoryBuffer() This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > Accelerated version of TBinaryProtocol allows invalid input to string fields. > - > > Key: THRIFT-4207 > URL: https://issues.apache.org/jira/browse/THRIFT-4207 > Project: Thrift > Issue Type: Bug > Components: Python - Library >Affects Versions: 0.10.0 >Reporter: Elvis Pranskevichus >Assignee: Aki Sukegawa >Priority: Major > > {{TBinaryProtocolAccelerated}} and {{TCompactProtocolAccelerated}} currently > accept arbitrary bytes as
[jira] [Commented] (THRIFT-3143) add typescript directory support
[ https://issues.apache.org/jira/browse/THRIFT-3143?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16693174#comment-16693174 ] ASF GitHub Bot commented on THRIFT-3143: mustafa-cosar opened a new pull request #1630: THRIFT-3143: Add nodets support URL: https://github.com/apache/thrift/pull/1630 These changes should be backwards compatible since it only adds new functionality and doesn't touch existing functionality. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > add typescript directory support > > > Key: THRIFT-3143 > URL: https://issues.apache.org/jira/browse/THRIFT-3143 > Project: Thrift > Issue Type: New Feature > Components: Node.js - Compiler >Reporter: Kazuki Yasufuku >Priority: Major > > Current typescript support is only work for browser, and generated d.ts uses > internal module. > So, it's hard to use for typescript in node.js. > To solve probrem, I make a pull request that generate typescript code > directory. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (THRIFT-4666) DLang Client Pool Test fails sporadically
[ https://issues.apache.org/jira/browse/THRIFT-4666?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16692060#comment-16692060 ] ASF GitHub Bot commented on THRIFT-4666: jeking3 closed pull request #1629: THRIFT-4666: Attempt to work around dlang client pool test failure (affects CI) URL: https://github.com/apache/thrift/pull/1629 This is a PR merged from a forked repository. As GitHub hides the original diff on merge, it is displayed below for the sake of provenance: As this is a foreign pull request (from a fork), the diff is supplied below (as it won't show otherwise due to GitHub magic): diff --git a/lib/d/test/client_pool_test.d b/lib/d/test/client_pool_test.d index 52207d9c7c..b24c97afd7 100644 --- a/lib/d/test/client_pool_test.d +++ b/lib/d/test/client_pool_test.d @@ -18,6 +18,7 @@ */ module client_pool_test; +import core.sync.semaphore : Semaphore; import core.time : Duration, dur; import core.thread : Thread; import std.algorithm; @@ -28,6 +29,7 @@ import std.getopt; import std.range; import std.stdio; import std.typecons; +import std.variant : Variant; import thrift.base; import thrift.async.libevent; import thrift.async.socket; @@ -37,9 +39,12 @@ import thrift.codegen.async_client_pool; import thrift.codegen.client; import thrift.codegen.client_pool; import thrift.codegen.processor; +import thrift.protocol.base; import thrift.protocol.binary; +import thrift.server.base; import thrift.server.simple; import thrift.server.transport.socket; +import thrift.transport.base; import thrift.transport.buffered; import thrift.transport.socket; import thrift.util.cancellation; @@ -108,11 +113,29 @@ private: } } +class ServerPreServeHandler : TServerEventHandler { + this(Semaphore sem) { +sem_ = sem; + } + + override void preServe() { +sem_.notify(); + } + + Variant createContext(TProtocol input, TProtocol output) { return Variant.init; } + void deleteContext(Variant serverContext, TProtocol input, TProtocol output) {} + void preProcess(Variant serverContext, TTransport transport) {} + +private: + Semaphore sem_; +} + class ServerThread : Thread { - this(ExTestHandler handler, TCancellation cancellation) { + this(ExTestHandler handler, ServerPreServeHandler serverHandler, TCancellation cancellation) { super(); handler_ = handler; cancellation_ = cancellation; +serverHandler_ = serverHandler; } private: void run() { @@ -123,16 +146,17 @@ private: serverTransport.recvTimeout = dur!"seconds"(3); auto transportFactory = new TBufferedTransportFactory; - auto server = new TSimpleServer( -processor, serverTransport, transportFactory, protocolFactory); + auto server = new TSimpleServer(processor, serverTransport, transportFactory, protocolFactory); + server.eventHandler = serverHandler_; server.serve(cancellation_); } catch (Exception e) { writefln("Server thread on port %s failed: %s", handler_.port, e); } } - TCancellation cancellation_; ExTestHandler handler_; + ServerPreServeHandler serverHandler_; + TCancellation cancellation_; } void main(string[] args) { @@ -145,6 +169,9 @@ void main(string[] args) { immutable ports = cast(immutable)array(map!"cast(ushort)a"(iota(port, port + 6))); + // semaphore that will be incremented whenever each server thread has bound and started listening + Semaphore sem = new Semaphore(0); + version (none) { // Cannot use this due to multiple DMD @@BUG@@s: // 1. »function D main is a nested function and cannot be accessed from array« @@ -174,11 +201,10 @@ version (none) { } // Fire up the server threads. - foreach (h; handlers) (new ServerThread(h, serverCancellation)).start(); + foreach (h; handlers) (new ServerThread(h, new ServerPreServeHandler(sem), serverCancellation)).start(); - // Give the servers some time to get up. This should really be accomplished - // via a barrier here and in the preServe() hook. - Thread.sleep(dur!"msecs"(10)); + // wait until all the handlers signal that they're ready to serve + foreach (h; handlers) (sem.wait(dur!`seconds`(1))); syncClientPoolTest(ports, handlers); asyncClientPoolTest(ports, handlers); This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > DLang Client Pool Test fails sporadically > - > > Key: THRIFT-4666 > URL: https://issues.apache.org/jira/browse/THRIFT-4666 > Project: Thrift > Issue Type: Task > Components: Build Process > Environment: Travis Ubuntu Xenial >Reporter:
[jira] [Commented] (THRIFT-4666) DLang Client Pool Test fails sporadically
[ https://issues.apache.org/jira/browse/THRIFT-4666?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16690758#comment-16690758 ] ASF GitHub Bot commented on THRIFT-4666: allengeorge commented on issue #1629: THRIFT-4666: Attempt to work around dlang client pool test failure URL: https://github.com/apache/thrift/pull/1629#issuecomment-439662712 The only build failure is the known SBCL tutorial server failure. Otherwise, everything else passes. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > DLang Client Pool Test fails sporadically > - > > Key: THRIFT-4666 > URL: https://issues.apache.org/jira/browse/THRIFT-4666 > Project: Thrift > Issue Type: Task > Components: Build Process > Environment: Travis Ubuntu Xenial >Reporter: Allen George >Priority: Minor > Attachments: dlang_compile_failure_1_log.txt, > dlang_compile_failure_2_log.txt > > > The DLang Client Pool test (client_pool_test.d) fails sporadically because > the client threads attempt to connect before the server threads have spun up > and bound to port 9090. There's a FIXME in the code saying that instead of a > 100msec wait a barrier should be used. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (THRIFT-4666) DLang Client Pool Test fails sporadically
[ https://issues.apache.org/jira/browse/THRIFT-4666?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16690729#comment-16690729 ] ASF GitHub Bot commented on THRIFT-4666: allengeorge commented on issue #1629: THRIFT-4666: Attempt to work around dlang client pool test failure URL: https://github.com/apache/thrift/pull/1629#issuecomment-439655192 If someone who's dlang-knowledgeable could take a look at this I'd really appreciate it. I think this fixes the sporadic dlang build failures. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > DLang Client Pool Test fails sporadically > - > > Key: THRIFT-4666 > URL: https://issues.apache.org/jira/browse/THRIFT-4666 > Project: Thrift > Issue Type: Task > Components: Build Process > Environment: Travis Ubuntu Xenial >Reporter: Allen George >Priority: Minor > Attachments: dlang_compile_failure_1_log.txt, > dlang_compile_failure_2_log.txt > > > The DLang Client Pool test (client_pool_test.d) fails sporadically because > the client threads attempt to connect before the server threads have spun up > and bound to port 9090. There's a FIXME in the code saying that instead of a > 100msec wait a barrier should be used. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (THRIFT-4666) DLang Client Pool Test fails sporadically
[ https://issues.apache.org/jira/browse/THRIFT-4666?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16690728#comment-16690728 ] ASF GitHub Bot commented on THRIFT-4666: allengeorge opened a new pull request #1629: THRIFT-4666: Attempt to work around dlang client pool test failure URL: https://github.com/apache/thrift/pull/1629 This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > DLang Client Pool Test fails sporadically > - > > Key: THRIFT-4666 > URL: https://issues.apache.org/jira/browse/THRIFT-4666 > Project: Thrift > Issue Type: Task > Components: Build Process > Environment: Travis Ubuntu Xenial >Reporter: Allen George >Priority: Minor > Attachments: dlang_compile_failure_1_log.txt, > dlang_compile_failure_2_log.txt > > > The DLang Client Pool test (client_pool_test.d) fails sporadically because > the client threads attempt to connect before the server threads have spun up > and bound to port 9090. There's a FIXME in the code saying that instead of a > 100msec wait a barrier should be used. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (THRIFT-4656) infinite loop in latest PHP library
[ https://issues.apache.org/jira/browse/THRIFT-4656?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16684690#comment-16684690 ] ASF GitHub Bot commented on THRIFT-4656: sokac commented on issue #1618: THRIFT-4656: Fix infinite loop in PHP TCurlClient URL: https://github.com/apache/thrift/pull/1618#issuecomment-438122866 @jeking3 I updated PR. You were right about oneway - server returns 200 OK with empty body. Looks like https://github.com/apache/thrift/pull/1602/files#diff-74eb8d4b20c423c78252e3e3dc847099R227 unintentionally fixed a bug, and the original version of this PR would have broken it. I tested with simple service that has both oneway and regular functions, everything was fully functional. I noticed php client will block until it gets response, but that's a different problem. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > infinite loop in latest PHP library > --- > > Key: THRIFT-4656 > URL: https://issues.apache.org/jira/browse/THRIFT-4656 > Project: Thrift > Issue Type: Bug > Components: PHP - Library >Reporter: Josip Sokcevic >Priority: Major > Fix For: 0.12.0 > > Attachments: > 0001-THRIFT-4656-Fix-infinite-loop-in-PHP-TCurlClient.patch > > > The latest PHP library can enter into infinite loop state when specific > payload is returned: > HTTP status: 200 > Response body: > > It was introduced with THRIFT-4645, where check was replaced from > {code:java} > !$this->response_{code} > to > {code:java} > $this->response_ === false{code} -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (THRIFT-4664) Rust cannot create ReadHalf/WriteHalf to implement custom tranports
[ https://issues.apache.org/jira/browse/THRIFT-4664?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16683769#comment-16683769 ] ASF GitHub Bot commented on THRIFT-4664: jeking3 closed pull request #1627: THRIFT-4664: Rust cannot create ReadHalf/WriteHalf URL: https://github.com/apache/thrift/pull/1627 This is a PR merged from a forked repository. As GitHub hides the original diff on merge, it is displayed below for the sake of provenance: As this is a foreign pull request (from a fork), the diff is supplied below (as it won't show otherwise due to GitHub magic): diff --git a/lib/rs/src/transport/mod.rs b/lib/rs/src/transport/mod.rs index 939278643b..6e84bfa492 100644 --- a/lib/rs/src/transport/mod.rs +++ b/lib/rs/src/transport/mod.rs @@ -143,6 +143,26 @@ where handle: C, } +impl ReadHalf +where +C: Read, +{ +/// Create a `ReadHalf` associated with readable `handle` +pub fn new(handle: C) -> ReadHalf { +ReadHalf { handle } +} +} + +impl WriteHalf +where +C: Write, +{ +/// Create a `WriteHalf` associated with writable `handle` +pub fn new(handle: C) -> WriteHalf { +WriteHalf { handle } +} +} + impl Read for ReadHalf where C: Read, diff --git a/lib/rs/src/transport/socket.rs b/lib/rs/src/transport/socket.rs index a6f780ac8c..954e2f5866 100644 --- a/lib/rs/src/transport/socket.rs +++ b/lib/rs/src/transport/socket.rs @@ -133,8 +133,9 @@ impl TIoChannel for TTcpChannel { .and_then(|s| s.try_clone().ok()) .map( |cloned| { -(ReadHalf { handle: TTcpChannel { stream: s.stream.take() } }, - WriteHalf { handle: TTcpChannel { stream: Some(cloned) } }) +let read_half = ReadHalf::new( TTcpChannel { stream: s.stream.take() } ); +let write_half = WriteHalf::new( TTcpChannel { stream: Some(cloned) } ); +(read_half, write_half) }, ) .ok_or_else( This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > Rust cannot create ReadHalf/WriteHalf to implement custom tranports > --- > > Key: THRIFT-4664 > URL: https://issues.apache.org/jira/browse/THRIFT-4664 > Project: Thrift > Issue Type: Improvement > Components: Rust - Library >Affects Versions: 0.11.0 > Environment: C:\Users\jake>rustup show > Default host: x86_64-pc-windows-msvc > stable-x86_64-pc-windows-msvc (default) > rustc 1.30.0 (da5f414c2 2018-10-24) >Reporter: J W >Assignee: Allen George >Priority: Minor > > To implement custom transports ("channels") looks like the intention is: > # Optionally, `impl TIoChannel for XXX` providing a split() > # Create `ReadHalf` and `WriteHalf` as passed to various > `XXXProtocol::new()` associated functions > Problems is: > {code:java} > use thrift::transport::{ > TIoChannel, > ReadHalf, > WriteHalf, > }; > impl TIoChannel for TNngChannel { > fn split(self) -> thrift::Result<(ReadHalf, WriteHalf)> where > Self: Sized, > { > //SNIP > Ok(( > ReadHalf { handle: self }, > WriteHalf { handle: clone } > )) > } > {code} > Results in: > error[E0451]: field `handle` of struct `thrift::transport::ReadHalf` is > private > --> runng-thrift\src\nng_channel.rs:57:24 > 57 | ReadHalf \{ handle: self }, > > field `handle` is private > > To enable creation of custom tranports/channels need to be able to create > `ReadHalf` and `WriteHalf` from outside the crate. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (THRIFT-4529) Rust generation should include #![allow(non_snake_case)] or force conform to Rust style guidelines
[ https://issues.apache.org/jira/browse/THRIFT-4529?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16683766#comment-16683766 ] ASF GitHub Bot commented on THRIFT-4529: jeking3 closed pull request #1625: THRIFT-4529: Rust enum variants are now camel-cased URL: https://github.com/apache/thrift/pull/1625 This is a PR merged from a forked repository. As GitHub hides the original diff on merge, it is displayed below for the sake of provenance: As this is a foreign pull request (from a fork), the diff is supplied below (as it won't show otherwise due to GitHub magic): diff --git a/CHANGES b/CHANGES index 9533ad08f3..c24c2b2223 100644 --- a/CHANGES +++ b/CHANGES @@ -1,7 +1,8 @@ Apache Thrift Changelog Breaking Changes since 0.11.0 [for 0.12.0]: - + +* [THRIFT-4529] - Rust enum variants are now camel-cased instead of uppercased to conform to Rust naming conventions * [THRIFT-4448] - Support for golang 1.6 and earlier has been dropped. * [THRIFT-4474] - PHP now uses the PSR-4 loader by default instead of class maps. * [THRIFT-4532] - method signatures changed in the compiler's t_oop_generator. diff --git a/compiler/cpp/src/thrift/generate/t_rs_generator.cc b/compiler/cpp/src/thrift/generate/t_rs_generator.cc index 5cd67f6755..ae30a35715 100644 --- a/compiler/cpp/src/thrift/generate/t_rs_generator.cc +++ b/compiler/cpp/src/thrift/generate/t_rs_generator.cc @@ -499,6 +499,9 @@ class t_rs_generator : public t_generator { // the server half of a Thrift service. string rust_sync_processor_impl_name(t_service *tservice); + // Return the variant name for an enum variant + string rust_enum_variant_name(const string& name); + // Properly uppercase names for use in Rust. string rust_upper_case(const string& name); @@ -873,7 +876,7 @@ void t_rs_generator::render_enum_definition(t_enum* tenum, const string& enum_na render_rustdoc((t_doc*) val); f_gen_ << indent() - << uppercase(val->get_name()) + << rust_enum_variant_name(val->get_name()) << " = " << val->get_value() << "," @@ -934,7 +937,7 @@ void t_rs_generator::render_enum_conversion(t_enum* tenum, const string& enum_na f_gen_ << indent() << val->get_value() - << " => Ok(" << enum_name << "::" << uppercase(val->get_name()) << ")," + << " => Ok(" << enum_name << "::" << rust_enum_variant_name(val->get_name()) << ")," << endl; } f_gen_ << indent() << "_ => {" << endl; @@ -3254,6 +3257,23 @@ string t_rs_generator::rust_sync_processor_impl_name(t_service *tservice) { return "T" + rust_camel_case(tservice->get_name()) + "ProcessFunctions"; } +string t_rs_generator::rust_enum_variant_name(const string ) { + bool all_uppercase = true; + + for (size_t i = 0; i < name.size(); i++) { +if (isalnum(name[i]) && islower(name[i])) { + all_uppercase = false; + break; +} + } + + if (all_uppercase) { +return capitalize(camelcase(lowercase(name))); + } else { +return capitalize(camelcase(name)); + } +} + string t_rs_generator::rust_upper_case(const string& name) { string str(uppercase(underscore(name))); string_replace(str, "__", "_"); diff --git a/lib/rs/README.md b/lib/rs/README.md index 8b35eda95c..7c37a10bc6 100644 --- a/lib/rs/README.md +++ b/lib/rs/README.md @@ -37,6 +37,57 @@ Thrift compiler you're using. Full [Rustdoc](https://docs.rs/thrift/) +## Compatibility + +The Rust library and auto-generated code targets Rust versions 1.28+. +It does not currently use any Rust 2018 features. + +### Breaking Changes + +Breaking changes are minimized. When they are made they will be outlined below with transition guidelines. + +# Thrift 0.12.0 + +* **[THRIFT-4529]** - Rust enum variants are now camel-cased instead of uppercased to conform to Rust naming conventions + +Previously, enum variants were uppercased in the auto-generated code. +For example, the following thrift enum: + +```thrift +// THRIFT +enum Operation { + ADD, + SUBTRACT, + MULTIPLY, + DIVIDE, +} +``` + +used to generate: + +```rust +// OLD AUTO-GENERATED RUST +pub enum Operation { + ADD, + SUBTRACT, + MULTIPLY, + DIVIDE, + } +``` +It *now* generates: +```rust +// NEW AUTO-GENERATED RUST +pub enum Operation { + Add, + Subtract, + Multiply, + Divide, + } +``` + +You will have to change all enum variants in your code to use camel-cased names. +This should be a search and replace. + ## Contributing Bug reports and PRs are always welcome! Please see the diff --git a/lib/rs/test/src/bin/kitchen_sink_server.rs
[jira] [Commented] (THRIFT-4662) Rust const string calls function at compile time
[ https://issues.apache.org/jira/browse/THRIFT-4662?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16683755#comment-16683755 ] ASF GitHub Bot commented on THRIFT-4662: jeking3 closed pull request #1624: THRIFT-4662: Rust const string calls function at compile time URL: https://github.com/apache/thrift/pull/1624 This is a PR merged from a forked repository. As GitHub hides the original diff on merge, it is displayed below for the sake of provenance: As this is a foreign pull request (from a fork), the diff is supplied below (as it won't show otherwise due to GitHub magic): diff --git a/compiler/cpp/src/thrift/generate/t_rs_generator.cc b/compiler/cpp/src/thrift/generate/t_rs_generator.cc index dc11fd3ee2..eccdd275e0 100644 --- a/compiler/cpp/src/thrift/generate/t_rs_generator.cc +++ b/compiler/cpp/src/thrift/generate/t_rs_generator.cc @@ -127,7 +127,7 @@ class t_rs_generator : public t_generator { void render_const_value_holder(const string& name, t_type* ttype, t_const_value* tvalue); // Write the actual const value - the right side of a const definition. - void render_const_value(t_type* ttype, t_const_value* tvalue); + void render_const_value(t_type* ttype, t_const_value* tvalue, bool is_owned = true); // Write a const struct (returned from `const_value` method). void render_const_struct(t_type* ttype, t_const_value* tvalue); @@ -411,6 +411,9 @@ class t_rs_generator : public t_generator { // Return a string representing the rust type given a `t_type`. string to_rust_type(t_type* ttype, bool ordered_float = true); + // Return a string representing the `const` rust type given a `t_type` + string to_rust_const_type(t_type* ttype, bool ordered_float = true); + // Return a string representing the rift `protocol::TType` given a `t_type`. string to_rust_field_type_enum(t_type* ttype); @@ -645,8 +648,8 @@ void t_rs_generator::render_const_value(const string& name, t_type* ttype, t_con throw "cannot generate simple rust constant for " + ttype->get_name(); } - f_gen_ << "pub const " << rust_upper_case(name) << ": " << to_rust_type(ttype) << " = "; - render_const_value(ttype, tvalue); + f_gen_ << "pub const " << rust_upper_case(name) << ": " << to_rust_const_type(ttype) << " = "; + render_const_value(ttype, tvalue, false); f_gen_ << ";" << endl; f_gen_ << endl; } @@ -673,15 +676,22 @@ void t_rs_generator::render_const_value_holder(const string& name, t_type* ttype f_gen_ << endl; } -void t_rs_generator::render_const_value(t_type* ttype, t_const_value* tvalue) { +void t_rs_generator::render_const_value(t_type* ttype, t_const_value* tvalue, bool is_owned) { if (ttype->is_base_type()) { t_base_type* tbase_type = (t_base_type*)ttype; switch (tbase_type->get_base()) { case t_base_type::TYPE_STRING: if (tbase_type->is_binary()) { -f_gen_ << "\"" << tvalue->get_string() << "\""<< ".to_owned().into_bytes()"; +if (is_owned) { + f_gen_ << "\"" << tvalue->get_string() << "\""<< ".to_owned().into_bytes()"; +} else { + f_gen_ << "b\"" << tvalue->get_string() << "\""; +} } else { -f_gen_ << "\"" << tvalue->get_string() << "\""<< ".to_owned()"; +f_gen_ << "\"" << tvalue->get_string() << "\""; +if (is_owned) { + f_gen_ << ".to_owned()"; +} } break; case t_base_type::TYPE_BOOL: @@ -3039,6 +3049,21 @@ string t_rs_generator::to_rust_type(t_type* ttype, bool ordered_float) { throw "cannot find rust type for " + ttype->get_name(); } +string t_rs_generator::to_rust_const_type(t_type* ttype, bool ordered_float) { + if (ttype->is_base_type()) { +t_base_type* tbase_type = ((t_base_type*)ttype); +if (tbase_type->get_base() == t_base_type::TYPE_STRING) { + if (tbase_type->is_binary()) { +return "&[u8]"; + } else { +return ""; + } +} + } + + return to_rust_type(ttype, ordered_float); +} + string t_rs_generator::to_rust_field_type_enum(t_type* ttype) { ttype = get_true_type(ttype); if (ttype->is_base_type()) { diff --git a/lib/rs/test/thrifts/Base_One.thrift b/lib/rs/test/thrifts/Base_One.thrift index 3da083d451..c5fa6c20df 100644 --- a/lib/rs/test/thrifts/Base_One.thrift +++ b/lib/rs/test/thrifts/Base_One.thrift @@ -37,6 +37,9 @@ const list CommonTemperatures = [300.0, 450.0] const double MealsPerDay = 2.5; +const string DefaultRecipeName = "Soup-rise of the Day" +const binary DefaultRecipeBinary = "Soup-rise of the 01010101" + struct Noodle { 1: string flourType 2: Temperature cookTemp This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact
[jira] [Commented] (THRIFT-4664) Rust cannot create ReadHalf/WriteHalf to implement custom tranports
[ https://issues.apache.org/jira/browse/THRIFT-4664?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16683673#comment-16683673 ] ASF GitHub Bot commented on THRIFT-4664: allengeorge commented on issue #1627: THRIFT-4664: Rust cannot create ReadHalf/WriteHalf URL: https://github.com/apache/thrift/pull/1627#issuecomment-437855436 @jeking3 Could this be merged please? The only build failure is with D, with the SSL server unable to bind in one build. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > Rust cannot create ReadHalf/WriteHalf to implement custom tranports > --- > > Key: THRIFT-4664 > URL: https://issues.apache.org/jira/browse/THRIFT-4664 > Project: Thrift > Issue Type: Improvement > Components: Rust - Library >Affects Versions: 0.11.0 > Environment: C:\Users\jake>rustup show > Default host: x86_64-pc-windows-msvc > stable-x86_64-pc-windows-msvc (default) > rustc 1.30.0 (da5f414c2 2018-10-24) >Reporter: J W >Assignee: Allen George >Priority: Minor > > To implement custom transports ("channels") looks like the intention is: > # Optionally, `impl TIoChannel for XXX` providing a split() > # Create `ReadHalf` and `WriteHalf` as passed to various > `XXXProtocol::new()` associated functions > Problems is: > {code:java} > use thrift::transport::{ > TIoChannel, > ReadHalf, > WriteHalf, > }; > impl TIoChannel for TNngChannel { > fn split(self) -> thrift::Result<(ReadHalf, WriteHalf)> where > Self: Sized, > { > //SNIP > Ok(( > ReadHalf { handle: self }, > WriteHalf { handle: clone } > )) > } > {code} > Results in: > error[E0451]: field `handle` of struct `thrift::transport::ReadHalf` is > private > --> runng-thrift\src\nng_channel.rs:57:24 > 57 | ReadHalf \{ handle: self }, > > field `handle` is private > > To enable creation of custom tranports/channels need to be able to create > `ReadHalf` and `WriteHalf` from outside the crate. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (THRIFT-4664) Rust cannot create ReadHalf/WriteHalf to implement custom tranports
[ https://issues.apache.org/jira/browse/THRIFT-4664?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16683672#comment-16683672 ] ASF GitHub Bot commented on THRIFT-4664: allengeorge commented on issue #1627: THRIFT-4664: Rust cannot create ReadHalf/WriteHalf URL: https://github.com/apache/thrift/pull/1627#issuecomment-437855261 @jeikabu TY for the PR! I guess this tells you I didn't check that I could implement new transports from outside the crate :/ I'll make sure of that in the future. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > Rust cannot create ReadHalf/WriteHalf to implement custom tranports > --- > > Key: THRIFT-4664 > URL: https://issues.apache.org/jira/browse/THRIFT-4664 > Project: Thrift > Issue Type: Improvement > Components: Rust - Library >Affects Versions: 0.11.0 > Environment: C:\Users\jake>rustup show > Default host: x86_64-pc-windows-msvc > stable-x86_64-pc-windows-msvc (default) > rustc 1.30.0 (da5f414c2 2018-10-24) >Reporter: J W >Assignee: Allen George >Priority: Minor > > To implement custom transports ("channels") looks like the intention is: > # Optionally, `impl TIoChannel for XXX` providing a split() > # Create `ReadHalf` and `WriteHalf` as passed to various > `XXXProtocol::new()` associated functions > Problems is: > {code:java} > use thrift::transport::{ > TIoChannel, > ReadHalf, > WriteHalf, > }; > impl TIoChannel for TNngChannel { > fn split(self) -> thrift::Result<(ReadHalf, WriteHalf)> where > Self: Sized, > { > //SNIP > Ok(( > ReadHalf { handle: self }, > WriteHalf { handle: clone } > )) > } > {code} > Results in: > error[E0451]: field `handle` of struct `thrift::transport::ReadHalf` is > private > --> runng-thrift\src\nng_channel.rs:57:24 > 57 | ReadHalf \{ handle: self }, > > field `handle` is private > > To enable creation of custom tranports/channels need to be able to create > `ReadHalf` and `WriteHalf` from outside the crate. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (THRIFT-4662) Rust const string calls function at compile time
[ https://issues.apache.org/jira/browse/THRIFT-4662?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16683665#comment-16683665 ] ASF GitHub Bot commented on THRIFT-4662: allengeorge commented on issue #1624: THRIFT-4662: Rust const string calls function at compile time URL: https://github.com/apache/thrift/pull/1624#issuecomment-437853849 @jeking3 These changes look good. Could they be merged please? All the cross-tests pass. Two build failures: SBCL (same issue as before) and D. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > Rust const string calls function at compile time > > > Key: THRIFT-4662 > URL: https://issues.apache.org/jira/browse/THRIFT-4662 > Project: Thrift > Issue Type: Bug > Components: Rust - Compiler >Affects Versions: 0.11.0 > Environment: C:\Users\jake>rustup show > Default host: x86_64-pc-windows-msvc > stable-x86_64-pc-windows-msvc (default) > rustc 1.30.0 (da5f414c2 2018-10-24) >Reporter: J W >Assignee: Allen George >Priority: Major > > *For this thrift:* > const string broker_playback_message = "mmi.developer.playback" > *Generates:* > // thrift -gen rs -out ../rust/thrift/src const_string.thrift > pub const BROKER_PLAYBACK_MESSAGE: String = > "mmi.developer.playback".to_owned(); > *Fails to compile:* > error[E0015]: calls in constants are limited to tuple structs and tuple > variants > note: a limited form of compile-time function evaluation is available on a > nightly compiler via `const fn` > *Fix:* > Probably want to output: > pub const BROKER_PLAYBACK_MESSAGE: = "mmi.developer.playback"; > > Looking at render_const_value() it looks like byte arrays will have the same > issue. > -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (THRIFT-4529) Rust generation should include #![allow(non_snake_case)] or force conform to Rust style guidelines
[ https://issues.apache.org/jira/browse/THRIFT-4529?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16683662#comment-16683662 ] ASF GitHub Bot commented on THRIFT-4529: allengeorge commented on issue #1625: THRIFT-4529: Rust enum variants are now camel-cased URL: https://github.com/apache/thrift/pull/1625#issuecomment-437853453 @jeking3 I've made the requested changes, and, the only thing that fails in Travis is the SBCL build under autotools :/ Could this be merged please? This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > Rust generation should include #![allow(non_snake_case)] or force conform to > Rust style guidelines > -- > > Key: THRIFT-4529 > URL: https://issues.apache.org/jira/browse/THRIFT-4529 > Project: Thrift > Issue Type: Improvement > Components: Rust - Compiler >Affects Versions: 0.11.0 >Reporter: Joshua >Assignee: Allen George >Priority: Minor > > Without this, building a project using a thrift file meant for multiple > languages may end up with many compiler warnings similar to the following: > {code:sh} > warning: variant `EXAMPLE_NAME` should have a camel case name such as > `ExampleName` > {code} -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (THRIFT-4664) Rust cannot create ReadHalf/WriteHalf to implement custom tranports
[ https://issues.apache.org/jira/browse/THRIFT-4664?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16683215#comment-16683215 ] ASF GitHub Bot commented on THRIFT-4664: jeikabu opened a new pull request #1627: THRIFT-4664: Cannot create ReadHalf/WriteHalf URL: https://github.com/apache/thrift/pull/1627 Client: rs Trivial change. Add a `pub` method for users of the library. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > Rust cannot create ReadHalf/WriteHalf to implement custom tranports > --- > > Key: THRIFT-4664 > URL: https://issues.apache.org/jira/browse/THRIFT-4664 > Project: Thrift > Issue Type: Improvement > Components: Rust - Library >Affects Versions: 0.11.0 > Environment: C:\Users\jake>rustup show > Default host: x86_64-pc-windows-msvc > stable-x86_64-pc-windows-msvc (default) > rustc 1.30.0 (da5f414c2 2018-10-24) >Reporter: J W >Priority: Minor > > To implement custom transports ("channels") looks like the intention is: > # Optionally, `impl TIoChannel for XXX` providing a split() > # Create `ReadHalf` and `WriteHalf` as passed to various > `XXXProtocol::new()` associated functions > Problems is: > {code:java} > use thrift::transport::{ > TIoChannel, > ReadHalf, > WriteHalf, > }; > impl TIoChannel for TNngChannel { > fn split(self) -> thrift::Result<(ReadHalf, WriteHalf)> where > Self: Sized, > { > //SNIP > Ok(( > ReadHalf { handle: self }, > WriteHalf { handle: clone } > )) > } > {code} > Results in: > error[E0451]: field `handle` of struct `thrift::transport::ReadHalf` is > private > --> runng-thrift\src\nng_channel.rs:57:24 > 57 | ReadHalf \{ handle: self }, > > field `handle` is private > > To enable creation of custom tranports/channels need to be able to create > `ReadHalf` and `WriteHalf` from outside the crate. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (THRIFT-4662) Rust const string calls function at compile time
[ https://issues.apache.org/jira/browse/THRIFT-4662?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16683161#comment-16683161 ] ASF GitHub Bot commented on THRIFT-4662: jeikabu commented on a change in pull request #1624: THRIFT-4662: Rust const string calls function at compile time URL: https://github.com/apache/thrift/pull/1624#discussion_r232526420 ## File path: lib/rs/test/thrifts/Base_One.thrift ## @@ -37,6 +37,9 @@ const list CommonTemperatures = [300.0, 450.0] const double MealsPerDay = 2.5; +const string DefaultRecipeName = "Something" Review comment: Glad to know my mastery of bad puns may finally be put to good use. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > Rust const string calls function at compile time > > > Key: THRIFT-4662 > URL: https://issues.apache.org/jira/browse/THRIFT-4662 > Project: Thrift > Issue Type: Bug > Components: Rust - Compiler >Affects Versions: 0.11.0 > Environment: C:\Users\jake>rustup show > Default host: x86_64-pc-windows-msvc > stable-x86_64-pc-windows-msvc (default) > rustc 1.30.0 (da5f414c2 2018-10-24) >Reporter: J W >Assignee: Allen George >Priority: Major > > *For this thrift:* > const string broker_playback_message = "mmi.developer.playback" > *Generates:* > // thrift -gen rs -out ../rust/thrift/src const_string.thrift > pub const BROKER_PLAYBACK_MESSAGE: String = > "mmi.developer.playback".to_owned(); > *Fails to compile:* > error[E0015]: calls in constants are limited to tuple structs and tuple > variants > note: a limited form of compile-time function evaluation is available on a > nightly compiler via `const fn` > *Fix:* > Probably want to output: > pub const BROKER_PLAYBACK_MESSAGE: = "mmi.developer.playback"; > > Looking at render_const_value() it looks like byte arrays will have the same > issue. > -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (THRIFT-4662) Rust const string calls function at compile time
[ https://issues.apache.org/jira/browse/THRIFT-4662?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16683143#comment-16683143 ] ASF GitHub Bot commented on THRIFT-4662: jeikabu commented on a change in pull request #1624: THRIFT-4662: Rust const string calls function at compile time URL: https://github.com/apache/thrift/pull/1624#discussion_r232522246 ## File path: compiler/cpp/src/thrift/generate/t_rs_generator.cc ## @@ -645,8 +648,8 @@ void t_rs_generator::render_const_value(const string& name, t_type* ttype, t_con throw "cannot generate simple rust constant for " + ttype->get_name(); } - f_gen_ << "pub const " << rust_upper_case(name) << ": " << to_rust_type(ttype) << " = "; - render_const_value(ttype, tvalue); + f_gen_ << "pub const " << rust_upper_case(name) << ": " << to_rust_const_type(ttype) << " = "; + render_const_value(ttype, tvalue, false); Review comment: Right This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > Rust const string calls function at compile time > > > Key: THRIFT-4662 > URL: https://issues.apache.org/jira/browse/THRIFT-4662 > Project: Thrift > Issue Type: Bug > Components: Rust - Compiler >Affects Versions: 0.11.0 > Environment: C:\Users\jake>rustup show > Default host: x86_64-pc-windows-msvc > stable-x86_64-pc-windows-msvc (default) > rustc 1.30.0 (da5f414c2 2018-10-24) >Reporter: J W >Assignee: Allen George >Priority: Major > > *For this thrift:* > const string broker_playback_message = "mmi.developer.playback" > *Generates:* > // thrift -gen rs -out ../rust/thrift/src const_string.thrift > pub const BROKER_PLAYBACK_MESSAGE: String = > "mmi.developer.playback".to_owned(); > *Fails to compile:* > error[E0015]: calls in constants are limited to tuple structs and tuple > variants > note: a limited form of compile-time function evaluation is available on a > nightly compiler via `const fn` > *Fix:* > Probably want to output: > pub const BROKER_PLAYBACK_MESSAGE: = "mmi.developer.playback"; > > Looking at render_const_value() it looks like byte arrays will have the same > issue. > -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (THRIFT-4545) Appveyor builds are failing due to a haskell / cabal update in chocolatey
[ https://issues.apache.org/jira/browse/THRIFT-4545?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16683026#comment-16683026 ] ASF GitHub Bot commented on THRIFT-4545: jeking3 closed pull request #1626: THRIFT-4545: fix haskell build on windows, fix appveyor stale packages URL: https://github.com/apache/thrift/pull/1626 This is a PR merged from a forked repository. As GitHub hides the original diff on merge, it is displayed below for the sake of provenance: As this is a foreign pull request (from a fork), the diff is supplied below (as it won't show otherwise due to GitHub magic): diff --git a/build/appveyor/MSVC-appveyor-build.bat b/build/appveyor/MSVC-appveyor-build.bat index 31aac57c99..a4b92a29cd 100644 --- a/build/appveyor/MSVC-appveyor-build.bat +++ b/build/appveyor/MSVC-appveyor-build.bat @@ -21,8 +21,6 @@ CALL cl_setenv.bat || EXIT /B MKDIR "%BUILDDIR%" || EXIT /B CD "%BUILDDIR%" || EXIT /B -:: Haskell is disabled for cmake (Windows), see Jira THRIFT-4545 - @ECHO ON cmake "%SRCDIR%" ^ -G"%GENERATOR%" ^ @@ -38,7 +36,6 @@ CD "%BUILDDIR%" || EXIT /B -DOPENSSL_USE_STATIC_LIBS=OFF ^ -DZLIB_LIBRARY="%WIN3P%\zlib-inst\lib\zlib%ZLIB_LIB_SUFFIX%.lib" ^ -DZLIB_ROOT="%WIN3P%\zlib-inst" ^ --DWITH_HASKELL=OFF ^ -DWITH_PYTHON=%WITH_PYTHON% ^ -DWITH_%THREADMODEL%THREADS=ON ^ -DWITH_SHARED_LIB=OFF ^ diff --git a/build/appveyor/MSVC-appveyor-install.bat b/build/appveyor/MSVC-appveyor-install.bat index f9eb0c67da..f973d29cff 100644 --- a/build/appveyor/MSVC-appveyor-install.bat +++ b/build/appveyor/MSVC-appveyor-install.bat @@ -31,14 +31,14 @@ choco feature enable -n allowGlobalConfirmation || EXIT /B :: Things to install when NOT running in appveyor: IF "%APPVEYOR_BUILD_ID%" == "" ( cup -y chocolatey || EXIT /B -cinst -c "%BUILDCACHE%" -y curl || EXIT /B -cinst -c "%BUILDCACHE%" -y 7zip || EXIT /B -cinst -c "%BUILDCACHE%" -y python3|| EXIT /B -cinst -c "%BUILDCACHE%" -y openssl.light || EXIT /B +cinst -y curl || EXIT /B +cinst -y 7zip || EXIT /B +cinst -y python3 || EXIT /B +cinst -y openssl.light|| EXIT /B ) -cinst -c "%BUILDCACHE%" -y jdk8 || EXIT /B -cinst -c "%BUILDCACHE%" -y winflexbison3 || EXIT /B +cinst -y jdk8 || EXIT /B +cinst -y winflexbison3|| EXIT /B :: zlib - not available through chocolatey CD "%APPVEYOR_SCRIPTS%" || EXIT /B @@ -56,5 +56,4 @@ pip.exe ^ tornado ^ twisted || EXIT /B -:: Haskell (GHC) and cabal (disabled: see Jira THRIFT-4545) -:: cinst -c "%BUILDCACHE%" -y ghc|| EXIT /B +cinst -y ghc || EXIT /B diff --git a/build/appveyor/cl_setenv.bat b/build/appveyor/cl_setenv.bat index 10af2d3477..62856cba0c 100644 --- a/build/appveyor/cl_setenv.bat +++ b/build/appveyor/cl_setenv.bat @@ -37,7 +37,6 @@ CALL cl_setcompiler.bat || EXIT /B CALL cl_setgenerator.bat || EXIT /B SET APPVEYOR_SCRIPTS=%APPVEYOR_BUILD_FOLDER%\build\appveyor -SET BUILDCACHE=%APPVEYOR_BUILD_FOLDER%\..\build\cache SET BUILDDIR=%APPVEYOR_BUILD_FOLDER%\..\build\%PROFILE%\%PLATFORM% SET INSTDIR=%APPVEYOR_BUILD_FOLDER%\..\build\%PROFILE%\%PLATFORM% SET SRCDIR=%APPVEYOR_BUILD_FOLDER% diff --git a/build/appveyor/cl_showenv.bat b/build/appveyor/cl_showenv.bat index 3dda546e50..a70a4906aa 100644 --- a/build/appveyor/cl_showenv.bat +++ b/build/appveyor/cl_showenv.bat @@ -38,7 +38,6 @@ ECHO BASH = %BASH% ECHO BOOST_ROOT= %BOOST_ROOT% ECHO BOOST_INCLUDEDIR = %BOOST_INCLUDEDIR% ECHO BOOST_LIBRARYDIR = %BOOST_LIBRARYDIR% -ECHO BUILDCACHE= %BUILDCACHE% ECHO BUILDDIR = %BUILDDIR% ECHO COMPILER = %COMPILER% ECHO GENERATOR = %GENERATOR% diff --git a/lib/hs/thrift.cabal b/lib/hs/thrift.cabal index 03a9814f38..a907d78e81 100644 --- a/lib/hs/thrift.cabal +++ b/lib/hs/thrift.cabal @@ -18,8 +18,8 @@ -- Name: thrift -Version:1.0.0-dev -Cabal-Version: >= 1.24 +Version:0.99.0 +Cabal-Version: 1.24 License:OtherLicense Category: Foreign Build-Type: Simple @@ -63,8 +63,7 @@ Library Thrift.Transport.IOBuffer, Thrift.Transport.Memory, Thrift.Types - Default-Language: -Haskell2010 + Default-Language: Haskell2010 Default-Extensions: DeriveDataTypeable, ExistentialQuantification, @@ -82,3 +81,4 @@ Test-Suite spec Ghc-Options: -Wall main-is: Spec.hs Build-Depends: base, thrift, hspec, QuickCheck >= 2.8.2, bytestring >= 0.10, unordered-containers >= 0.2.6 + Default-Language: Haskell2010
[jira] [Commented] (THRIFT-4545) Appveyor builds are failing due to a haskell / cabal update in chocolatey
[ https://issues.apache.org/jira/browse/THRIFT-4545?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16682994#comment-16682994 ] ASF GitHub Bot commented on THRIFT-4545: jeking3 commented on a change in pull request #1626: THRIFT-4545: fix haskell build on windows, fix appveyor stale packages URL: https://github.com/apache/thrift/pull/1626#discussion_r232501820 ## File path: lib/hs/thrift.cabal ## @@ -18,8 +18,8 @@ -- Name: thrift -Version:1.0.0-dev -Cabal-Version: >= 1.24 +Version:0.99.0 Review comment: @jfarrell @Jens-G note: cabal will no longer allow "1.0.0-dev" as a version... this happens with newer versions of cabal. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > Appveyor builds are failing due to a haskell / cabal update in chocolatey > - > > Key: THRIFT-4545 > URL: https://issues.apache.org/jira/browse/THRIFT-4545 > Project: Thrift > Issue Type: Bug > Components: Build Process >Affects Versions: 0.11.0 >Reporter: James E. King III >Assignee: James E. King III >Priority: Major > > Working on it. > Progress: Downloading cabal 2.2.0.0... 100% > Progress: Downloading ghc 8.4.1... 100% -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (THRIFT-4529) Rust generation should include #![allow(non_snake_case)] or force conform to Rust style guidelines
[ https://issues.apache.org/jira/browse/THRIFT-4529?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16682943#comment-16682943 ] ASF GitHub Bot commented on THRIFT-4529: allengeorge edited a comment on issue #1625: THRIFT-4529: Rust enum variants are now camel-cased URL: https://github.com/apache/thrift/pull/1625#issuecomment-437686887 I've updated the `CHANGES` file, plus added a `Compatibility` section to the Rust `README` along with transition guidelines. I will do this in the future for any breaking changes. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > Rust generation should include #![allow(non_snake_case)] or force conform to > Rust style guidelines > -- > > Key: THRIFT-4529 > URL: https://issues.apache.org/jira/browse/THRIFT-4529 > Project: Thrift > Issue Type: Improvement > Components: Rust - Compiler >Affects Versions: 0.11.0 >Reporter: Joshua >Assignee: Allen George >Priority: Minor > > Without this, building a project using a thrift file meant for multiple > languages may end up with many compiler warnings similar to the following: > {code:sh} > warning: variant `EXAMPLE_NAME` should have a camel case name such as > `ExampleName` > {code} -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (THRIFT-4529) Rust generation should include #![allow(non_snake_case)] or force conform to Rust style guidelines
[ https://issues.apache.org/jira/browse/THRIFT-4529?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16682939#comment-16682939 ] ASF GitHub Bot commented on THRIFT-4529: allengeorge commented on issue #1625: THRIFT-4529: Rust enum variants are now camel-cased URL: https://github.com/apache/thrift/pull/1625#issuecomment-437686887 I've updated the `CHANGES` file, plus added a `COMPATIBILITY` section to the Rust `README` along with transition guidelines. I will do this in the future for any breaking changes. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > Rust generation should include #![allow(non_snake_case)] or force conform to > Rust style guidelines > -- > > Key: THRIFT-4529 > URL: https://issues.apache.org/jira/browse/THRIFT-4529 > Project: Thrift > Issue Type: Improvement > Components: Rust - Compiler >Affects Versions: 0.11.0 >Reporter: Joshua >Assignee: Allen George >Priority: Minor > > Without this, building a project using a thrift file meant for multiple > languages may end up with many compiler warnings similar to the following: > {code:sh} > warning: variant `EXAMPLE_NAME` should have a camel case name such as > `ExampleName` > {code} -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (THRIFT-4529) Rust generation should include #![allow(non_snake_case)] or force conform to Rust style guidelines
[ https://issues.apache.org/jira/browse/THRIFT-4529?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16682930#comment-16682930 ] ASF GitHub Bot commented on THRIFT-4529: allengeorge commented on issue #1625: THRIFT-4529: Camel-case Rust enum variants URL: https://github.com/apache/thrift/pull/1625#issuecomment-437685253 Yes - you're right: it is a breaking change. I'll make the README entries, and make sure I do so for any (hopefully rare!) breaking changes I make in the future. Thank you for catching this. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > Rust generation should include #![allow(non_snake_case)] or force conform to > Rust style guidelines > -- > > Key: THRIFT-4529 > URL: https://issues.apache.org/jira/browse/THRIFT-4529 > Project: Thrift > Issue Type: Improvement > Components: Rust - Compiler >Affects Versions: 0.11.0 >Reporter: Joshua >Assignee: Allen George >Priority: Minor > > Without this, building a project using a thrift file meant for multiple > languages may end up with many compiler warnings similar to the following: > {code:sh} > warning: variant `EXAMPLE_NAME` should have a camel case name such as > `ExampleName` > {code} -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (THRIFT-4529) Rust generation should include #![allow(non_snake_case)] or force conform to Rust style guidelines
[ https://issues.apache.org/jira/browse/THRIFT-4529?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16682921#comment-16682921 ] ASF GitHub Bot commented on THRIFT-4529: allengeorge commented on issue #1625: THRIFT-4529: Camel-case Rust enum variants URL: https://github.com/apache/thrift/pull/1625#issuecomment-437683052 I did not include a flag for backwards-compatibility. I'd prefer not to, because this change makes the autogenerated code conform to Rust naming conventions. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > Rust generation should include #![allow(non_snake_case)] or force conform to > Rust style guidelines > -- > > Key: THRIFT-4529 > URL: https://issues.apache.org/jira/browse/THRIFT-4529 > Project: Thrift > Issue Type: Improvement > Components: Rust - Compiler >Affects Versions: 0.11.0 >Reporter: Joshua >Assignee: Allen George >Priority: Minor > > Without this, building a project using a thrift file meant for multiple > languages may end up with many compiler warnings similar to the following: > {code:sh} > warning: variant `EXAMPLE_NAME` should have a camel case name such as > `ExampleName` > {code} -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (THRIFT-4656) infinite loop in latest PHP library
[ https://issues.apache.org/jira/browse/THRIFT-4656?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16682920#comment-16682920 ] ASF GitHub Bot commented on THRIFT-4656: allengeorge commented on issue #1618: THRIFT-4656: Fix infinite loop in PHP TCurlClient URL: https://github.com/apache/thrift/pull/1618#issuecomment-437682746 Oh. Hmm - you're absolutely right. Because the autogen'd code is protocol agnostic it doesn't know that in some cases the protocol has no concept of a one-way interaction. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > infinite loop in latest PHP library > --- > > Key: THRIFT-4656 > URL: https://issues.apache.org/jira/browse/THRIFT-4656 > Project: Thrift > Issue Type: Bug > Components: PHP - Library >Reporter: Josip Sokcevic >Priority: Major > Fix For: 0.12.0 > > Attachments: > 0001-THRIFT-4656-Fix-infinite-loop-in-PHP-TCurlClient.patch > > > The latest PHP library can enter into infinite loop state when specific > payload is returned: > HTTP status: 200 > Response body: > > It was introduced with THRIFT-4645, where check was replaced from > {code:java} > !$this->response_{code} > to > {code:java} > $this->response_ === false{code} -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (THRIFT-4656) infinite loop in latest PHP library
[ https://issues.apache.org/jira/browse/THRIFT-4656?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16682916#comment-16682916 ] ASF GitHub Bot commented on THRIFT-4656: jeking3 commented on issue #1618: THRIFT-4656: Fix infinite loop in PHP TCurlClient URL: https://github.com/apache/thrift/pull/1618#issuecomment-437681718 Sounds like an improvement, but here's the issue I think exists with oneway: 1. Generated client code sends a oneway request. 2. HTTP transport packages it up and send it to the server. This could be through a proxy. 3. Server receives it. Since it is a oneway request, my assumption is the server, no matter what the language, has a HTTP transport that issues a response (like 200 OK without content, or 204 as you stated). If it did not, it wouldn't work through a proxy (or at least, the proxy might behave badly). An example of this can be found at https://github.com/apache/thrift/blob/master/lib/py/src/server/THttpServer.py#L86. 4. Client has no generated response reader code because it is oneway, so the client code never attempts to get the transport response (which is fair, oneway probably came around before we had a transport with a mandatory response). However now there's a transport reply sitting in the buffer. 5. The next request sent by the client that is normal (not oneway) would send the request and then read the HTTP 1.1 200 OK response buffered from from the oneway request, and the actual reply to the two-way request would get queued up; however the response processor for the twoway request, seeing no payload, would throw an exception. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > infinite loop in latest PHP library > --- > > Key: THRIFT-4656 > URL: https://issues.apache.org/jira/browse/THRIFT-4656 > Project: Thrift > Issue Type: Bug > Components: PHP - Library >Reporter: Josip Sokcevic >Priority: Major > Fix For: 0.12.0 > > Attachments: > 0001-THRIFT-4656-Fix-infinite-loop-in-PHP-TCurlClient.patch > > > The latest PHP library can enter into infinite loop state when specific > payload is returned: > HTTP status: 200 > Response body: > > It was introduced with THRIFT-4645, where check was replaced from > {code:java} > !$this->response_{code} > to > {code:java} > $this->response_ === false{code} -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (THRIFT-4529) Rust generation should include #![allow(non_snake_case)] or force conform to Rust style guidelines
[ https://issues.apache.org/jira/browse/THRIFT-4529?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16682917#comment-16682917 ] ASF GitHub Bot commented on THRIFT-4529: jeking3 commented on issue #1625: THRIFT-4529: Camel-case enum variants URL: https://github.com/apache/thrift/pull/1625#issuecomment-437681878 I assume the default setting is to use the previous way, and this is a new way, so it is backwards compatible? This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > Rust generation should include #![allow(non_snake_case)] or force conform to > Rust style guidelines > -- > > Key: THRIFT-4529 > URL: https://issues.apache.org/jira/browse/THRIFT-4529 > Project: Thrift > Issue Type: Improvement > Components: Rust - Compiler >Affects Versions: 0.11.0 >Reporter: Joshua >Assignee: Allen George >Priority: Minor > > Without this, building a project using a thrift file meant for multiple > languages may end up with many compiler warnings similar to the following: > {code:sh} > warning: variant `EXAMPLE_NAME` should have a camel case name such as > `ExampleName` > {code} -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (THRIFT-4529) Rust generation should include #![allow(non_snake_case)] or force conform to Rust style guidelines
[ https://issues.apache.org/jira/browse/THRIFT-4529?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16682909#comment-16682909 ] ASF GitHub Bot commented on THRIFT-4529: allengeorge commented on issue #1625: THRIFT-4529: Camel-case enum variants URL: https://github.com/apache/thrift/pull/1625#issuecomment-437680138 Build failure caused by a PEBCAK on my part (forgot to change enum variant names in cross-tests). Fixing now. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > Rust generation should include #![allow(non_snake_case)] or force conform to > Rust style guidelines > -- > > Key: THRIFT-4529 > URL: https://issues.apache.org/jira/browse/THRIFT-4529 > Project: Thrift > Issue Type: Improvement > Components: Rust - Compiler >Affects Versions: 0.11.0 >Reporter: Joshua >Assignee: Allen George >Priority: Minor > > Without this, building a project using a thrift file meant for multiple > languages may end up with many compiler warnings similar to the following: > {code:sh} > warning: variant `EXAMPLE_NAME` should have a camel case name such as > `ExampleName` > {code} -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (THRIFT-4656) infinite loop in latest PHP library
[ https://issues.apache.org/jira/browse/THRIFT-4656?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16682908#comment-16682908 ] ASF GitHub Bot commented on THRIFT-4656: allengeorge commented on issue #1618: THRIFT-4656: Fix infinite loop in PHP TCurlClient URL: https://github.com/apache/thrift/pull/1618#issuecomment-437680054 Would it be better for the language runtime to return a `204` indicating a successful request but no content? This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > infinite loop in latest PHP library > --- > > Key: THRIFT-4656 > URL: https://issues.apache.org/jira/browse/THRIFT-4656 > Project: Thrift > Issue Type: Bug > Components: PHP - Library >Reporter: Josip Sokcevic >Priority: Major > Fix For: 0.12.0 > > Attachments: > 0001-THRIFT-4656-Fix-infinite-loop-in-PHP-TCurlClient.patch > > > The latest PHP library can enter into infinite loop state when specific > payload is returned: > HTTP status: 200 > Response body: > > It was introduced with THRIFT-4645, where check was replaced from > {code:java} > !$this->response_{code} > to > {code:java} > $this->response_ === false{code} -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (THRIFT-4545) Appveyor builds are failing due to a haskell / cabal update in chocolatey
[ https://issues.apache.org/jira/browse/THRIFT-4545?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16682905#comment-16682905 ] ASF GitHub Bot commented on THRIFT-4545: jeking3 opened a new pull request #1626: THRIFT-4545: fix haskell build on windows, fix appveyor stale packages URL: https://github.com/apache/thrift/pull/1626 This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > Appveyor builds are failing due to a haskell / cabal update in chocolatey > - > > Key: THRIFT-4545 > URL: https://issues.apache.org/jira/browse/THRIFT-4545 > Project: Thrift > Issue Type: Bug > Components: Build Process >Affects Versions: 0.11.0 >Reporter: James E. King III >Priority: Major > > Working on it. > Progress: Downloading cabal 2.2.0.0... 100% > Progress: Downloading ghc 8.4.1... 100% -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (THRIFT-4662) Rust const string calls function at compile time
[ https://issues.apache.org/jira/browse/THRIFT-4662?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16682888#comment-16682888 ] ASF GitHub Bot commented on THRIFT-4662: allengeorge commented on issue #1624: THRIFT-4662: Rust const string calls function at compile time URL: https://github.com/apache/thrift/pull/1624#issuecomment-437673078 No - that’s ok. I’ve set the minimum Rust version to 1.28 for a while now, so I think we should be fine. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > Rust const string calls function at compile time > > > Key: THRIFT-4662 > URL: https://issues.apache.org/jira/browse/THRIFT-4662 > Project: Thrift > Issue Type: Bug > Components: Rust - Compiler >Affects Versions: 0.11.0 > Environment: C:\Users\jake>rustup show > Default host: x86_64-pc-windows-msvc > stable-x86_64-pc-windows-msvc (default) > rustc 1.30.0 (da5f414c2 2018-10-24) >Reporter: J W >Assignee: Allen George >Priority: Major > > *For this thrift:* > const string broker_playback_message = "mmi.developer.playback" > *Generates:* > // thrift -gen rs -out ../rust/thrift/src const_string.thrift > pub const BROKER_PLAYBACK_MESSAGE: String = > "mmi.developer.playback".to_owned(); > *Fails to compile:* > error[E0015]: calls in constants are limited to tuple structs and tuple > variants > note: a limited form of compile-time function evaluation is available on a > nightly compiler via `const fn` > *Fix:* > Probably want to output: > pub const BROKER_PLAYBACK_MESSAGE: = "mmi.developer.playback"; > > Looking at render_const_value() it looks like byte arrays will have the same > issue. > -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (THRIFT-4662) Rust const string calls function at compile time
[ https://issues.apache.org/jira/browse/THRIFT-4662?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16682737#comment-16682737 ] ASF GitHub Bot commented on THRIFT-4662: jake-ruyi commented on a change in pull request #1624: THRIFT-4662: Rust const string calls function at compile time URL: https://github.com/apache/thrift/pull/1624#discussion_r232474522 ## File path: compiler/cpp/src/thrift/generate/t_rs_generator.cc ## @@ -673,15 +676,22 @@ void t_rs_generator::render_const_value_holder(const string& name, t_type* ttype f_gen_ << endl; } -void t_rs_generator::render_const_value(t_type* ttype, t_const_value* tvalue) { +void t_rs_generator::render_const_value(t_type* ttype, t_const_value* tvalue, bool isowned) { if (ttype->is_base_type()) { t_base_type* tbase_type = (t_base_type*)ttype; switch (tbase_type->get_base()) { case t_base_type::TYPE_STRING: if (tbase_type->is_binary()) { -f_gen_ << "\"" << tvalue->get_string() << "\""<< ".to_owned().into_bytes()"; +if (isowned) { Review comment: That explains it. I could have sworn it needed to be `&'static`, but I found a bunch of examples that were `` and assumed I was thinking of something else... Do you think it's worth outputting `&'static str` instead so it works with Rust <1.17? This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > Rust const string calls function at compile time > > > Key: THRIFT-4662 > URL: https://issues.apache.org/jira/browse/THRIFT-4662 > Project: Thrift > Issue Type: Bug > Components: Rust - Compiler >Affects Versions: 0.11.0 > Environment: C:\Users\jake>rustup show > Default host: x86_64-pc-windows-msvc > stable-x86_64-pc-windows-msvc (default) > rustc 1.30.0 (da5f414c2 2018-10-24) >Reporter: J W >Assignee: Allen George >Priority: Major > > *For this thrift:* > const string broker_playback_message = "mmi.developer.playback" > *Generates:* > // thrift -gen rs -out ../rust/thrift/src const_string.thrift > pub const BROKER_PLAYBACK_MESSAGE: String = > "mmi.developer.playback".to_owned(); > *Fails to compile:* > error[E0015]: calls in constants are limited to tuple structs and tuple > variants > note: a limited form of compile-time function evaluation is available on a > nightly compiler via `const fn` > *Fix:* > Probably want to output: > pub const BROKER_PLAYBACK_MESSAGE: = "mmi.developer.playback"; > > Looking at render_const_value() it looks like byte arrays will have the same > issue. > -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (THRIFT-4662) Rust const string calls function at compile time
[ https://issues.apache.org/jira/browse/THRIFT-4662?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16682738#comment-16682738 ] ASF GitHub Bot commented on THRIFT-4662: jake-ruyi commented on a change in pull request #1624: THRIFT-4662: Rust const string calls function at compile time URL: https://github.com/apache/thrift/pull/1624#discussion_r232474522 ## File path: compiler/cpp/src/thrift/generate/t_rs_generator.cc ## @@ -673,15 +676,22 @@ void t_rs_generator::render_const_value_holder(const string& name, t_type* ttype f_gen_ << endl; } -void t_rs_generator::render_const_value(t_type* ttype, t_const_value* tvalue) { +void t_rs_generator::render_const_value(t_type* ttype, t_const_value* tvalue, bool isowned) { if (ttype->is_base_type()) { t_base_type* tbase_type = (t_base_type*)ttype; switch (tbase_type->get_base()) { case t_base_type::TYPE_STRING: if (tbase_type->is_binary()) { -f_gen_ << "\"" << tvalue->get_string() << "\""<< ".to_owned().into_bytes()"; +if (isowned) { Review comment: That explains it. I could have sworn it needed to be `&'static`, but I found a bunch of examples that were `` and it compiled so I assumed I was thinking of something else... Do you think it's worth outputting `&'static str` instead so it works with Rust <1.17? This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > Rust const string calls function at compile time > > > Key: THRIFT-4662 > URL: https://issues.apache.org/jira/browse/THRIFT-4662 > Project: Thrift > Issue Type: Bug > Components: Rust - Compiler >Affects Versions: 0.11.0 > Environment: C:\Users\jake>rustup show > Default host: x86_64-pc-windows-msvc > stable-x86_64-pc-windows-msvc (default) > rustc 1.30.0 (da5f414c2 2018-10-24) >Reporter: J W >Assignee: Allen George >Priority: Major > > *For this thrift:* > const string broker_playback_message = "mmi.developer.playback" > *Generates:* > // thrift -gen rs -out ../rust/thrift/src const_string.thrift > pub const BROKER_PLAYBACK_MESSAGE: String = > "mmi.developer.playback".to_owned(); > *Fails to compile:* > error[E0015]: calls in constants are limited to tuple structs and tuple > variants > note: a limited form of compile-time function evaluation is available on a > nightly compiler via `const fn` > *Fix:* > Probably want to output: > pub const BROKER_PLAYBACK_MESSAGE: = "mmi.developer.playback"; > > Looking at render_const_value() it looks like byte arrays will have the same > issue. > -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (THRIFT-4529) Rust generation should include #![allow(non_snake_case)] or force conform to Rust style guidelines
[ https://issues.apache.org/jira/browse/THRIFT-4529?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16682504#comment-16682504 ] ASF GitHub Bot commented on THRIFT-4529: allengeorge opened a new pull request #1625: THRIFT-4529: Camel-case enum variants URL: https://github.com/apache/thrift/pull/1625 Client: rs This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > Rust generation should include #![allow(non_snake_case)] or force conform to > Rust style guidelines > -- > > Key: THRIFT-4529 > URL: https://issues.apache.org/jira/browse/THRIFT-4529 > Project: Thrift > Issue Type: Improvement > Components: Rust - Compiler >Affects Versions: 0.11.0 >Reporter: Joshua >Assignee: Allen George >Priority: Minor > > Without this, building a project using a thrift file meant for multiple > languages may end up with many compiler warnings similar to the following: > {code:sh} > warning: variant `EXAMPLE_NAME` should have a camel case name such as > `ExampleName` > {code} -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (THRIFT-4662) Rust const string calls function at compile time
[ https://issues.apache.org/jira/browse/THRIFT-4662?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16682470#comment-16682470 ] ASF GitHub Bot commented on THRIFT-4662: allengeorge commented on a change in pull request #1624: THRIFT-4662: Rust const string calls function at compile time URL: https://github.com/apache/thrift/pull/1624#discussion_r232455479 ## File path: compiler/cpp/src/thrift/generate/t_rs_generator.cc ## @@ -3076,6 +3086,21 @@ string t_rs_generator::to_rust_field_type_enum(t_type* ttype) { throw "cannot find TType for " + ttype->get_name(); } +string t_rs_generator::to_rust_const_type(t_type* ttype, bool ordered_float) { Review comment: Please reorder implementation just before `to_rust_type` This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > Rust const string calls function at compile time > > > Key: THRIFT-4662 > URL: https://issues.apache.org/jira/browse/THRIFT-4662 > Project: Thrift > Issue Type: Bug > Components: Rust - Compiler >Affects Versions: 0.11.0 > Environment: C:\Users\jake>rustup show > Default host: x86_64-pc-windows-msvc > stable-x86_64-pc-windows-msvc (default) > rustc 1.30.0 (da5f414c2 2018-10-24) >Reporter: J W >Assignee: Allen George >Priority: Major > > *For this thrift:* > const string broker_playback_message = "mmi.developer.playback" > *Generates:* > // thrift -gen rs -out ../rust/thrift/src const_string.thrift > pub const BROKER_PLAYBACK_MESSAGE: String = > "mmi.developer.playback".to_owned(); > *Fails to compile:* > error[E0015]: calls in constants are limited to tuple structs and tuple > variants > note: a limited form of compile-time function evaluation is available on a > nightly compiler via `const fn` > *Fix:* > Probably want to output: > pub const BROKER_PLAYBACK_MESSAGE: = "mmi.developer.playback"; > > Looking at render_const_value() it looks like byte arrays will have the same > issue. > -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (THRIFT-4662) Rust const string calls function at compile time
[ https://issues.apache.org/jira/browse/THRIFT-4662?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16682471#comment-16682471 ] ASF GitHub Bot commented on THRIFT-4662: allengeorge commented on issue #1624: THRIFT-4662: Rust const string calls function at compile time URL: https://github.com/apache/thrift/pull/1624#issuecomment-437592044 @jake-ruyi Thank you for the PR. I've some minor comments, but overall this looks good. If you can address them, I'll approve and this can be merged in. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > Rust const string calls function at compile time > > > Key: THRIFT-4662 > URL: https://issues.apache.org/jira/browse/THRIFT-4662 > Project: Thrift > Issue Type: Bug > Components: Rust - Compiler >Affects Versions: 0.11.0 > Environment: C:\Users\jake>rustup show > Default host: x86_64-pc-windows-msvc > stable-x86_64-pc-windows-msvc (default) > rustc 1.30.0 (da5f414c2 2018-10-24) >Reporter: J W >Assignee: Allen George >Priority: Major > > *For this thrift:* > const string broker_playback_message = "mmi.developer.playback" > *Generates:* > // thrift -gen rs -out ../rust/thrift/src const_string.thrift > pub const BROKER_PLAYBACK_MESSAGE: String = > "mmi.developer.playback".to_owned(); > *Fails to compile:* > error[E0015]: calls in constants are limited to tuple structs and tuple > variants > note: a limited form of compile-time function evaluation is available on a > nightly compiler via `const fn` > *Fix:* > Probably want to output: > pub const BROKER_PLAYBACK_MESSAGE: = "mmi.developer.playback"; > > Looking at render_const_value() it looks like byte arrays will have the same > issue. > -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (THRIFT-4662) Rust const string calls function at compile time
[ https://issues.apache.org/jira/browse/THRIFT-4662?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16682469#comment-16682469 ] ASF GitHub Bot commented on THRIFT-4662: allengeorge commented on a change in pull request #1624: THRIFT-4662: Rust const string calls function at compile time URL: https://github.com/apache/thrift/pull/1624#discussion_r232455435 ## File path: lib/rs/test/thrifts/Base_One.thrift ## @@ -37,6 +37,9 @@ const list CommonTemperatures = [300.0, 450.0] const double MealsPerDay = 2.5; +const string DefaultRecipeName = "Something" Review comment: This is incredibly minor, but, could you please choose some funny recipe names here? :) This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > Rust const string calls function at compile time > > > Key: THRIFT-4662 > URL: https://issues.apache.org/jira/browse/THRIFT-4662 > Project: Thrift > Issue Type: Bug > Components: Rust - Compiler >Affects Versions: 0.11.0 > Environment: C:\Users\jake>rustup show > Default host: x86_64-pc-windows-msvc > stable-x86_64-pc-windows-msvc (default) > rustc 1.30.0 (da5f414c2 2018-10-24) >Reporter: J W >Assignee: Allen George >Priority: Major > > *For this thrift:* > const string broker_playback_message = "mmi.developer.playback" > *Generates:* > // thrift -gen rs -out ../rust/thrift/src const_string.thrift > pub const BROKER_PLAYBACK_MESSAGE: String = > "mmi.developer.playback".to_owned(); > *Fails to compile:* > error[E0015]: calls in constants are limited to tuple structs and tuple > variants > note: a limited form of compile-time function evaluation is available on a > nightly compiler via `const fn` > *Fix:* > Probably want to output: > pub const BROKER_PLAYBACK_MESSAGE: = "mmi.developer.playback"; > > Looking at render_const_value() it looks like byte arrays will have the same > issue. > -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (THRIFT-4662) Rust const string calls function at compile time
[ https://issues.apache.org/jira/browse/THRIFT-4662?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16682466#comment-16682466 ] ASF GitHub Bot commented on THRIFT-4662: allengeorge commented on a change in pull request #1624: THRIFT-4662: Rust const string calls function at compile time URL: https://github.com/apache/thrift/pull/1624#discussion_r232455248 ## File path: compiler/cpp/src/thrift/generate/t_rs_generator.cc ## @@ -127,7 +127,7 @@ class t_rs_generator : public t_generator { void render_const_value_holder(const string& name, t_type* ttype, t_const_value* tvalue); // Write the actual const value - the right side of a const definition. - void render_const_value(t_type* ttype, t_const_value* tvalue); + void render_const_value(t_type* ttype, t_const_value* tvalue, bool isowned = true); Review comment: Could you change parameter name to `owned` or `is_owned`? This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > Rust const string calls function at compile time > > > Key: THRIFT-4662 > URL: https://issues.apache.org/jira/browse/THRIFT-4662 > Project: Thrift > Issue Type: Bug > Components: Rust - Compiler >Affects Versions: 0.11.0 > Environment: C:\Users\jake>rustup show > Default host: x86_64-pc-windows-msvc > stable-x86_64-pc-windows-msvc (default) > rustc 1.30.0 (da5f414c2 2018-10-24) >Reporter: J W >Assignee: Allen George >Priority: Major > > *For this thrift:* > const string broker_playback_message = "mmi.developer.playback" > *Generates:* > // thrift -gen rs -out ../rust/thrift/src const_string.thrift > pub const BROKER_PLAYBACK_MESSAGE: String = > "mmi.developer.playback".to_owned(); > *Fails to compile:* > error[E0015]: calls in constants are limited to tuple structs and tuple > variants > note: a limited form of compile-time function evaluation is available on a > nightly compiler via `const fn` > *Fix:* > Probably want to output: > pub const BROKER_PLAYBACK_MESSAGE: = "mmi.developer.playback"; > > Looking at render_const_value() it looks like byte arrays will have the same > issue. > -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (THRIFT-4662) Rust const string calls function at compile time
[ https://issues.apache.org/jira/browse/THRIFT-4662?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16682467#comment-16682467 ] ASF GitHub Bot commented on THRIFT-4662: allengeorge commented on a change in pull request #1624: THRIFT-4662: Rust const string calls function at compile time URL: https://github.com/apache/thrift/pull/1624#discussion_r232455421 ## File path: compiler/cpp/src/thrift/generate/t_rs_generator.cc ## @@ -673,15 +676,22 @@ void t_rs_generator::render_const_value_holder(const string& name, t_type* ttype f_gen_ << endl; } -void t_rs_generator::render_const_value(t_type* ttype, t_const_value* tvalue) { +void t_rs_generator::render_const_value(t_type* ttype, t_const_value* tvalue, bool isowned) { Review comment: Please rename parameter to `owned` or `is_owned`. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > Rust const string calls function at compile time > > > Key: THRIFT-4662 > URL: https://issues.apache.org/jira/browse/THRIFT-4662 > Project: Thrift > Issue Type: Bug > Components: Rust - Compiler >Affects Versions: 0.11.0 > Environment: C:\Users\jake>rustup show > Default host: x86_64-pc-windows-msvc > stable-x86_64-pc-windows-msvc (default) > rustc 1.30.0 (da5f414c2 2018-10-24) >Reporter: J W >Assignee: Allen George >Priority: Major > > *For this thrift:* > const string broker_playback_message = "mmi.developer.playback" > *Generates:* > // thrift -gen rs -out ../rust/thrift/src const_string.thrift > pub const BROKER_PLAYBACK_MESSAGE: String = > "mmi.developer.playback".to_owned(); > *Fails to compile:* > error[E0015]: calls in constants are limited to tuple structs and tuple > variants > note: a limited form of compile-time function evaluation is available on a > nightly compiler via `const fn` > *Fix:* > Probably want to output: > pub const BROKER_PLAYBACK_MESSAGE: = "mmi.developer.playback"; > > Looking at render_const_value() it looks like byte arrays will have the same > issue. > -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (THRIFT-4662) Rust const string calls function at compile time
[ https://issues.apache.org/jira/browse/THRIFT-4662?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16682468#comment-16682468 ] ASF GitHub Bot commented on THRIFT-4662: allengeorge commented on a change in pull request #1624: THRIFT-4662: Rust const string calls function at compile time URL: https://github.com/apache/thrift/pull/1624#discussion_r232455395 ## File path: compiler/cpp/src/thrift/generate/t_rs_generator.cc ## @@ -645,8 +648,8 @@ void t_rs_generator::render_const_value(const string& name, t_type* ttype, t_con throw "cannot generate simple rust constant for " + ttype->get_name(); } - f_gen_ << "pub const " << rust_upper_case(name) << ": " << to_rust_type(ttype) << " = "; - render_const_value(ttype, tvalue); + f_gen_ << "pub const " << rust_upper_case(name) << ": " << to_rust_const_type(ttype) << " = "; + render_const_value(ttype, tvalue, false); Review comment: Interesting. This works because the call isn't recursive. Only the top-level type will be owned/static. The types in containers will all be owned because of the internal call to `to_rust_type`. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > Rust const string calls function at compile time > > > Key: THRIFT-4662 > URL: https://issues.apache.org/jira/browse/THRIFT-4662 > Project: Thrift > Issue Type: Bug > Components: Rust - Compiler >Affects Versions: 0.11.0 > Environment: C:\Users\jake>rustup show > Default host: x86_64-pc-windows-msvc > stable-x86_64-pc-windows-msvc (default) > rustc 1.30.0 (da5f414c2 2018-10-24) >Reporter: J W >Assignee: Allen George >Priority: Major > > *For this thrift:* > const string broker_playback_message = "mmi.developer.playback" > *Generates:* > // thrift -gen rs -out ../rust/thrift/src const_string.thrift > pub const BROKER_PLAYBACK_MESSAGE: String = > "mmi.developer.playback".to_owned(); > *Fails to compile:* > error[E0015]: calls in constants are limited to tuple structs and tuple > variants > note: a limited form of compile-time function evaluation is available on a > nightly compiler via `const fn` > *Fix:* > Probably want to output: > pub const BROKER_PLAYBACK_MESSAGE: = "mmi.developer.playback"; > > Looking at render_const_value() it looks like byte arrays will have the same > issue. > -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (THRIFT-4662) Rust const string calls function at compile time
[ https://issues.apache.org/jira/browse/THRIFT-4662?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16682465#comment-16682465 ] ASF GitHub Bot commented on THRIFT-4662: allengeorge commented on a change in pull request #1624: THRIFT-4662: Rust const string calls function at compile time URL: https://github.com/apache/thrift/pull/1624#discussion_r232455341 ## File path: compiler/cpp/src/thrift/generate/t_rs_generator.cc ## @@ -414,6 +414,9 @@ class t_rs_generator : public t_generator { // Return a string representing the rift `protocol::TType` given a `t_type`. string to_rust_field_type_enum(t_type* ttype); + // Return a string representing the `const` rust type given a `t_type` + string to_rust_const_type(t_type* ttype, bool ordered_float = true); Review comment: Good call. Makes sense to split this out from the `to_rust_type`. Could you please put this function declaration right before `to_rust_type`? And do the same for implementation please? It just ensures that code that has similar functionality is in the same part of the file. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > Rust const string calls function at compile time > > > Key: THRIFT-4662 > URL: https://issues.apache.org/jira/browse/THRIFT-4662 > Project: Thrift > Issue Type: Bug > Components: Rust - Compiler >Affects Versions: 0.11.0 > Environment: C:\Users\jake>rustup show > Default host: x86_64-pc-windows-msvc > stable-x86_64-pc-windows-msvc (default) > rustc 1.30.0 (da5f414c2 2018-10-24) >Reporter: J W >Assignee: Allen George >Priority: Major > > *For this thrift:* > const string broker_playback_message = "mmi.developer.playback" > *Generates:* > // thrift -gen rs -out ../rust/thrift/src const_string.thrift > pub const BROKER_PLAYBACK_MESSAGE: String = > "mmi.developer.playback".to_owned(); > *Fails to compile:* > error[E0015]: calls in constants are limited to tuple structs and tuple > variants > note: a limited form of compile-time function evaluation is available on a > nightly compiler via `const fn` > *Fix:* > Probably want to output: > pub const BROKER_PLAYBACK_MESSAGE: = "mmi.developer.playback"; > > Looking at render_const_value() it looks like byte arrays will have the same > issue. > -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (THRIFT-4662) Rust const string calls function at compile time
[ https://issues.apache.org/jira/browse/THRIFT-4662?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16682454#comment-16682454 ] ASF GitHub Bot commented on THRIFT-4662: allengeorge commented on a change in pull request #1624: THRIFT-4662: Rust const string calls function at compile time URL: https://github.com/apache/thrift/pull/1624#discussion_r232454772 ## File path: compiler/cpp/src/thrift/generate/t_rs_generator.cc ## @@ -673,15 +676,22 @@ void t_rs_generator::render_const_value_holder(const string& name, t_type* ttype f_gen_ << endl; } -void t_rs_generator::render_const_value(t_type* ttype, t_const_value* tvalue) { +void t_rs_generator::render_const_value(t_type* ttype, t_const_value* tvalue, bool isowned) { if (ttype->is_base_type()) { t_base_type* tbase_type = (t_base_type*)ttype; switch (tbase_type->get_base()) { case t_base_type::TYPE_STRING: if (tbase_type->is_binary()) { -f_gen_ << "\"" << tvalue->get_string() << "\""<< ".to_owned().into_bytes()"; +if (isowned) { Review comment: No - the PR isn't being held up because of const binary. I just didn't have time to look at it yet. I usually pull the branch locally and test in a container to ensure everything works. For one thing, I'm unsure that ``` const FOO: = "ladeda"; ``` works in rust 1.28. EDIT. Oh, nm - I see it was [enabled in 1.17](https://rust-lang-nursery.github.io/edition-guide/rust-2018/ownership-and-lifetimes/simpler-lifetimes-in-static-and-const.html) This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > Rust const string calls function at compile time > > > Key: THRIFT-4662 > URL: https://issues.apache.org/jira/browse/THRIFT-4662 > Project: Thrift > Issue Type: Bug > Components: Rust - Compiler >Affects Versions: 0.11.0 > Environment: C:\Users\jake>rustup show > Default host: x86_64-pc-windows-msvc > stable-x86_64-pc-windows-msvc (default) > rustc 1.30.0 (da5f414c2 2018-10-24) >Reporter: J W >Assignee: Allen George >Priority: Major > > *For this thrift:* > const string broker_playback_message = "mmi.developer.playback" > *Generates:* > // thrift -gen rs -out ../rust/thrift/src const_string.thrift > pub const BROKER_PLAYBACK_MESSAGE: String = > "mmi.developer.playback".to_owned(); > *Fails to compile:* > error[E0015]: calls in constants are limited to tuple structs and tuple > variants > note: a limited form of compile-time function evaluation is available on a > nightly compiler via `const fn` > *Fix:* > Probably want to output: > pub const BROKER_PLAYBACK_MESSAGE: = "mmi.developer.playback"; > > Looking at render_const_value() it looks like byte arrays will have the same > issue. > -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (THRIFT-4662) Rust const string calls function at compile time
[ https://issues.apache.org/jira/browse/THRIFT-4662?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16682130#comment-16682130 ] ASF GitHub Bot commented on THRIFT-4662: jake-ruyi commented on a change in pull request #1624: THRIFT-4662: Rust const string calls function at compile time URL: https://github.com/apache/thrift/pull/1624#discussion_r232434691 ## File path: compiler/cpp/src/thrift/generate/t_rs_generator.cc ## @@ -673,15 +676,22 @@ void t_rs_generator::render_const_value_holder(const string& name, t_type* ttype f_gen_ << endl; } -void t_rs_generator::render_const_value(t_type* ttype, t_const_value* tvalue) { +void t_rs_generator::render_const_value(t_type* ttype, t_const_value* tvalue, bool isowned) { if (ttype->is_base_type()) { t_base_type* tbase_type = (t_base_type*)ttype; switch (tbase_type->get_base()) { case t_base_type::TYPE_STRING: if (tbase_type->is_binary()) { -f_gen_ << "\"" << tvalue->get_string() << "\""<< ".to_owned().into_bytes()"; +if (isowned) { Review comment: My goals were: 1. Make `const binary` compile 2. Preserve existing behavior Presently, neither `const string` nor `const binary` compiles. I'm more interested in getting our stuff compiling than addressing the semantics of `binary`. Should I revert the binary portion? This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > Rust const string calls function at compile time > > > Key: THRIFT-4662 > URL: https://issues.apache.org/jira/browse/THRIFT-4662 > Project: Thrift > Issue Type: Bug > Components: Rust - Compiler >Affects Versions: 0.11.0 > Environment: C:\Users\jake>rustup show > Default host: x86_64-pc-windows-msvc > stable-x86_64-pc-windows-msvc (default) > rustc 1.30.0 (da5f414c2 2018-10-24) >Reporter: J W >Assignee: Allen George >Priority: Major > > *For this thrift:* > const string broker_playback_message = "mmi.developer.playback" > *Generates:* > // thrift -gen rs -out ../rust/thrift/src const_string.thrift > pub const BROKER_PLAYBACK_MESSAGE: String = > "mmi.developer.playback".to_owned(); > *Fails to compile:* > error[E0015]: calls in constants are limited to tuple structs and tuple > variants > note: a limited form of compile-time function evaluation is available on a > nightly compiler via `const fn` > *Fix:* > Probably want to output: > pub const BROKER_PLAYBACK_MESSAGE: = "mmi.developer.playback"; > > Looking at render_const_value() it looks like byte arrays will have the same > issue. > -- This message was sent by Atlassian JIRA (v7.6.3#76005)