Details

    • Type: Improvement
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 0.9
    • Fix Version/s: 0.10.0
    • Component/s: Python - Compiler
    • Labels:
      None

      Description

      add support for python 3.x

        Issue Links

          Activity

          Hide
          avi Avi Saranga added a comment -

          Generally;

          • python library (using 2to3 + minor changes)
          • python compiler (t_py_generator.cc) convert bytes object to str

          I'll post my patches once they are tested kthx

          Show
          avi Avi Saranga added a comment - Generally; python library (using 2to3 + minor changes) python compiler (t_py_generator.cc) convert bytes object to str I'll post my patches once they are tested kthx
          Hide
          jfarrell Jake Farrell added a comment -

          Avi, we've had several patches for 2.x and 3 support and have opted to stay with the current python version due to it being the most widely used across all operating systems. We want to try and support as much as we can without having to have forks of each client for major version changes as seen within python.

          That said, I look forward to reviewing your patches and discussing what we can do to make the client library better and more robust for pythons multiple versions. thank you for contributing

          Show
          jfarrell Jake Farrell added a comment - Avi, we've had several patches for 2.x and 3 support and have opted to stay with the current python version due to it being the most widely used across all operating systems. We want to try and support as much as we can without having to have forks of each client for major version changes as seen within python. That said, I look forward to reviewing your patches and discussing what we can do to make the client library better and more robust for pythons multiple versions. thank you for contributing
          Hide
          avi Avi Saranga added a comment -

          Jake,
          Thanks for the prompt response, the changes seems to be very minor (compiler wise)
          as for the actual library, 2to3 does a good job converting,
          maybe a configure option and a few macros for the compiler code and a py3 library ?

          Show
          avi Avi Saranga added a comment - Jake, Thanks for the prompt response, the changes seems to be very minor (compiler wise) as for the actual library, 2to3 does a good job converting, maybe a configure option and a few macros for the compiler code and a py3 library ?
          Hide
          tbartelmess Thomas Bartelmess added a comment -

          Is there any timeline for Python 3 support?

          Show
          tbartelmess Thomas Bartelmess added a comment - Is there any timeline for Python 3 support?
          Hide
          jfarrell Jake Farrell added a comment -

          There has been no timeline set for this, the biggest thing we are trying to avoid is having a py2 and a py3 library to support

          Show
          jfarrell Jake Farrell added a comment - There has been no timeline set for this, the biggest thing we are trying to avoid is having a py2 and a py3 library to support
          Hide
          bergundy Roey Berman added a comment -

          Oops, attached in wrong issue, sorry ..

          Show
          bergundy Roey Berman added a comment - Oops, attached in wrong issue, sorry ..
          Hide
          wgwang wgwang added a comment -

          I forked the repository https://github.com/wgwang/thrift. And add support for python3. I used it in python 3.3.2 in my projects. Till now, it support two options:

          1. -gen py:python3 – for generally python3 code generator.
          2. -gen py:python3,tornado – for generating codes support tornado with python3.

          The following is my changes:

          1. Using the patch of issue THRIFT-2096 for generator, and fixing some bugs.
          2. Adding new py3 lib (https://github.com/wgwang/thrift/tree/master/lib/py3), and using 2to3 to get basic codes from origin py lib.
          3. In transport layer, change StringIO to BytesIO

          Show
          wgwang wgwang added a comment - I forked the repository https://github.com/wgwang/thrift . And add support for python3. I used it in python 3.3.2 in my projects. Till now, it support two options: 1. -gen py:python3 – for generally python3 code generator. 2. -gen py:python3,tornado – for generating codes support tornado with python3. The following is my changes: 1. Using the patch of issue THRIFT-2096 for generator, and fixing some bugs. 2. Adding new py3 lib ( https://github.com/wgwang/thrift/tree/master/lib/py3 ), and using 2to3 to get basic codes from origin py lib. 3. In transport layer, change StringIO to BytesIO
          Hide
          jamesob james o'beirne added a comment -

          wgwang thanks for stepping in and taking some concrete action here. I'd like to consider using your fork for a production product – would you say it's stable enough to do so?

          Show
          jamesob james o'beirne added a comment - wgwang thanks for stepping in and taking some concrete action here. I'd like to consider using your fork for a production product – would you say it's stable enough to do so?
          Hide
          eevee Eevee added a comment -

          There's another Python 3 branch submitted as a pull request on GitHub: https://github.com/apache/thrift/pull/144

          (Based on commit activity it looks like pull requests are never merged, so mentioning it here as well.)

          This version is the same code running on both 2 and 3, using the six library, and generates dual-version code as well with no need for an extra command line argument. It'll require Python 2.6 for e.g. the `as` syntax, but at this stage in the game not many libraries still work on Python 2.5.

          Show
          eevee Eevee added a comment - There's another Python 3 branch submitted as a pull request on GitHub: https://github.com/apache/thrift/pull/144 (Based on commit activity it looks like pull requests are never merged, so mentioning it here as well.) This version is the same code running on both 2 and 3, using the six library, and generates dual-version code as well with no need for an extra command line argument. It'll require Python 2.6 for e.g. the `as` syntax, but at this stage in the game not many libraries still work on Python 2.5.
          Hide
          githubbot ASF GitHub Bot added a comment -
          Show
          githubbot ASF GitHub Bot added a comment - Github user bufferoverflow commented on the pull request: https://github.com/apache/thrift/pull/144#issuecomment-47165500 https://issues.apache.org/jira/browse/THRIFT-1857
          Hide
          roger.meier Roger Meier added a comment -

          I like the approach with the six library and will review it om Friday.
          ;-r

          Show
          roger.meier Roger Meier added a comment - I like the approach with the six library and will review it om Friday. ;-r
          Hide
          jfarrell Jake Farrell added a comment -

          Most of the items that have been blocking 3.x support are due to backports not reaching 2.4 as it is EOL. I'm in favor of not introducing the extra dependency if we can avoid it and just making the switch to using the newer style that is in 2.7-3.0 compatible

          ping Todd Lipcon

          Show
          jfarrell Jake Farrell added a comment - Most of the items that have been blocking 3.x support are due to backports not reaching 2.4 as it is EOL. I'm in favor of not introducing the extra dependency if we can avoid it and just making the switch to using the newer style that is in 2.7-3.0 compatible ping Todd Lipcon
          Hide
          eevee Eevee added a comment -

          You need six even for code targeting 2.7+; it exists to smooth out changes in 3.x that weren't backported.

          The PR could easily be rewritten without it, but it's a very small and common dependency.

          Show
          eevee Eevee added a comment - You need six even for code targeting 2.7+; it exists to smooth out changes in 3.x that weren't backported. The PR could easily be rewritten without it, but it's a very small and common dependency.
          Hide
          tlipcon Todd Lipcon added a comment -

          I think we're probably OK with new versions of Thrift requiring 2.6. I don't think we're OK with a dependency on 2.7 or later, though – almost all of our customers run on RHEL6 which only has 2.6.

          Show
          tlipcon Todd Lipcon added a comment - I think we're probably OK with new versions of Thrift requiring 2.6. I don't think we're OK with a dependency on 2.7 or later, though – almost all of our customers run on RHEL6 which only has 2.6.
          Hide
          eevee Eevee added a comment -

          I don't think there's anything in thrift that would benefit from being 2.7-only, anyway. All the necessary 3.x compatibility syntax went into 2.6.

          Show
          eevee Eevee added a comment - I don't think there's anything in thrift that would benefit from being 2.7-only, anyway. All the necessary 3.x compatibility syntax went into 2.6.
          Hide
          wgwang wgwang added a comment -

          james o'beirne I think so. I used it in my production a few months. Until now, I haven't found any bug.

          Show
          wgwang wgwang added a comment - james o'beirne I think so. I used it in my production a few months. Until now, I haven't found any bug.
          Hide
          roger.meier Roger Meier added a comment -

          Thanks tbartelmess, great approach with python-six!
          I've reviewed and tests pull request 144 and the test suite fails.
          make cross

          ERROR: testBackwards (__main__.AcceleratedBinaryTest)
          ----------------------------------------------------------------------
          Traceback (most recent call last):
            File "./SerializationTest.py", line 204, in testBackwards
              obj = self._deserialize(VersioningTestV1, self._serialize(self.v2obj))
            File "./SerializationTest.py", line 195, in _deserialize
              ret.read(prot)
            File "gen-py-default/ThriftTest/ttypes.py", line 1030, in read
              fastbinary.decode_binary(self, iprot.trans, (self.__class__, self.thrift_spec))
          TypeError: expecting stringio input
          
          ok======================================================================
          

          Could you please check this and the review findings made by Eevee?

          Thanks!
          -roger

          Show
          roger.meier Roger Meier added a comment - Thanks tbartelmess, great approach with python-six! I've reviewed and tests pull request 144 and the test suite fails. make cross ERROR: testBackwards (__main__.AcceleratedBinaryTest) ---------------------------------------------------------------------- Traceback (most recent call last): File "./SerializationTest.py", line 204, in testBackwards obj = self._deserialize(VersioningTestV1, self._serialize(self.v2obj)) File "./SerializationTest.py", line 195, in _deserialize ret.read(prot) File "gen-py-default/ThriftTest/ttypes.py", line 1030, in read fastbinary.decode_binary(self, iprot.trans, (self.__class__, self.thrift_spec)) TypeError: expecting stringio input ok====================================================================== Could you please check this and the review findings made by Eevee? Thanks! -roger
          Hide
          eevee Eevee added a comment -

          Hey, I picked this up and fixed a lot of other issues that were missed ­— I don't think tbartelmess actually ran the full test suite against Python 3. Perk of being on Arch.

          I'm still having trouble with Roger Meier's issue, though, and I don't see how to fix it without rewriting a lot of fastbinary.c, a daunting task.

          But... I'm also not seeing any speed boost from the C module.

          make check, run from lib/py/, Python 3.4 (no C module) versus Python 2.7 (hacked a bit to keep the C module still working):

          make check  47.63s user 15.01s system 12% cpu 8:18.35 total
          make PYTHON=/usr/bin/python2 check  48.07s user 15.48s system 12% cpu 8:19.09 total
          

          That makes some sense to me: it looks like the C module repeatedly calls back into Python code asking to read a handful of bytes at a time, which would negate a lot of the speed improvement.

          Unless I'm missing something here (which is possible! the original commits claim the C extension provides a vast speedup), can I just remove the C extension entirely? There's probably room for improvement using C code, but I don't think what's there now is cutting it.

          Show
          eevee Eevee added a comment - Hey, I picked this up and fixed a lot of other issues that were missed ­— I don't think tbartelmess actually ran the full test suite against Python 3. Perk of being on Arch. I'm still having trouble with Roger Meier 's issue, though, and I don't see how to fix it without rewriting a lot of fastbinary.c , a daunting task. But... I'm also not seeing any speed boost from the C module. make check , run from lib/py/ , Python 3.4 (no C module) versus Python 2.7 (hacked a bit to keep the C module still working): make check 47.63s user 15.01s system 12% cpu 8:18.35 total make PYTHON=/usr/bin/python2 check 48.07s user 15.48s system 12% cpu 8:19.09 total That makes some sense to me: it looks like the C module repeatedly calls back into Python code asking to read a handful of bytes at a time, which would negate a lot of the speed improvement. Unless I'm missing something here (which is possible! the original commits claim the C extension provides a vast speedup), can I just remove the C extension entirely? There's probably room for improvement using C code, but I don't think what's there now is cutting it.
          Hide
          githubbot ASF GitHub Bot added a comment -

          GitHub user eevee opened a pull request:

          https://github.com/apache/thrift/pull/213

          Python 3 support, redux

          As mentioned on [the Jira ticket](https://issues.apache.org/jira/browse/THRIFT-1857?focusedCommentId=14104848&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14104848), there are still some minor problems with `fastbinary.c`, which uses a private API that no longer exists in Python 3. I think it's worth just removing outright.

          But the test suite passes completely on Python 3, now.

          You can merge this pull request into a Git repository by running:

          $ git pull https://github.com/eevee/thrift python3

          Alternatively you can review and apply these changes as the patch at:

          https://github.com/apache/thrift/pull/213.patch

          To close this pull request, make a commit to your master/trunk branch
          with (at least) the following in the commit message:

          This closes #213


          commit 3b4b03ea24d90787736b4da8ce70ee31455cbf6e
          Author: Thomas Bartelmess <tbartelmess@marketcircle.com>
          Date: 2014-04-30T20:21:05Z

          Python 3 support

          Added six to install_requires in setup.py

          commit 03d7a6dc2d6d410d5b9b89abd0a464b751d2162b
          Author: Eevee (Alex Munroe) <eevee.git@veekun.com>
          Date: 2014-08-19T23:07:36Z

          Fix useless warnings about a broken C extension on 3.x, PyPy, etc.

          Also remove use_2to3, since this is supposed to be single-codebase now.

          commit 3d912d748041f9a446cd0bf7580462d93316f60e
          Author: Eevee (Alex Munroe) <eevee.git@veekun.com>
          Date: 2014-08-19T23:12:16Z

          Fix a few minor nitpicks with the Python 3 fixes.

          commit ee916e9486364fa4130ff109b58ad43fef636757
          Author: Eevee (Alex Munroe) <eevee.git@veekun.com>
          Date: 2014-08-19T23:28:07Z

          JSONProtocol's constants need to be bytestrings.

          commit 2b820077a77bd539c979cbae0998d4c562e80c35
          Author: Eevee (Alex Munroe) <eevee.git@veekun.com>
          Date: 2014-08-19T23:28:55Z

          Use print_function in the Python tests.

          commit 242ed1e7aea7cf05512495aabcaf1452bbc27c6a
          Author: Eevee (Alex Munroe) <eevee.git@veekun.com>
          Date: 2014-08-19T23:29:09Z

          Unbreak the Python tests when the C extension doesn't exist.

          If there's no C part, everything gets built into `build/lib/`, but the
          shell globs were looking for a dot.

          commit 9c5161bc17d4ba7e89e8eb08bed2a481dc58f878
          Author: Eevee (Alex Munroe) <eevee.git@veekun.com>
          Date: 2014-08-19T23:30:06Z

          Trailing whitespace my editor fixed for me.

          commit 863efcb78e43d1adbe17fd024b2546d054055497
          Author: Eevee (Alex Munroe) <eevee.git@veekun.com>
          Date: 2014-08-20T02:53:07Z

          Probably don't need this 2.4 compat any more.

          commit 6981c4b3e0b8ed5480fc4dd794659beefe34a90d
          Author: Eevee (Alex Munroe) <eevee.git@veekun.com>
          Date: 2014-08-20T02:53:20Z

          Stop generating code that contains xrange and iteritems.

          commit 6b3d2749ec80c6fb1b20d4f6a810d84c7edf0dda
          Author: Eevee (Alex Munroe) <eevee.git@veekun.com>
          Date: 2014-08-20T02:53:50Z

          Make JSON and compact protocols use bytes everywhere.

          commit 5fec5813830eb6437ecb00effabfd6647dec6d9c
          Author: Eevee (Alex Munroe) <eevee.git@veekun.com>
          Date: 2014-08-20T02:54:31Z

          Python 3 support in test/py/.

          commit 2c06f4acea8993f3766e6120755d1ac23d00eef8
          Author: Eevee (Alex Munroe) <eevee.git@veekun.com>
          Date: 2014-08-20T02:54:44Z

          Miscellaneous module moves and text/bytes stuff that was missed.

          commit 516406ba0f5698783552dbb012118e8964033698
          Author: Eevee (Alex Munroe) <eevee.git@veekun.com>
          Date: 2014-08-20T03:14:21Z

          Fix more sys.path.insert glob shenanigans.

          commit 4c5ae2e5004de16f3389414b129a1ca88abda925
          Author: Eevee (Alex Munroe) <eevee.git@veekun.com>
          Date: 2014-08-20T03:43:51Z

          Fix a bad relativized import.

          commit eb0132e1658428440c7b36624afc07d62616dd82
          Author: Eevee (Alex Munroe) <eevee.git@veekun.com>
          Date: 2014-08-20T03:44:07Z

          Fix another old-style exception syntax in the Python generator.

          commit 43ea49778757ad0799769fc4cef242fea00b4914
          Author: Eevee (Alex Munroe) <eevee.git@veekun.com>
          Date: 2014-08-20T03:46:13Z

          Run 2to3 on the Python tutorial code.

          commit c0fd1a85b7ee2016c0b237a07e45aac23fcb2386
          Author: Eevee (Alex Munroe) <eevee.git@veekun.com>
          Date: 2014-08-20T04:08:50Z

          Don't compile to itervalues, either.

          commit 3b767742c1a5372a78943885f11a2f23e985a46a
          Author: Eevee (Alex Munroe) <eevee.git@veekun.com>
          Date: 2014-08-20T04:09:06Z

          Fix passing a float to struct.pack() – tolerated in 2, not 3.

          commit 19d9f4ebcf90679a29235ae4c4b8ba45d01405ef
          Author: Eevee (Alex Munroe) <eevee.git@veekun.com>
          Date: 2014-09-11T22:42:50Z

          And one more round of fixing JSON decoding.


          Show
          githubbot ASF GitHub Bot added a comment - GitHub user eevee opened a pull request: https://github.com/apache/thrift/pull/213 Python 3 support, redux As mentioned on [the Jira ticket] ( https://issues.apache.org/jira/browse/THRIFT-1857?focusedCommentId=14104848&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14104848 ), there are still some minor problems with `fastbinary.c`, which uses a private API that no longer exists in Python 3. I think it's worth just removing outright. But the test suite passes completely on Python 3, now. You can merge this pull request into a Git repository by running: $ git pull https://github.com/eevee/thrift python3 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/thrift/pull/213.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #213 commit 3b4b03ea24d90787736b4da8ce70ee31455cbf6e Author: Thomas Bartelmess <tbartelmess@marketcircle.com> Date: 2014-04-30T20:21:05Z Python 3 support Added six to install_requires in setup.py commit 03d7a6dc2d6d410d5b9b89abd0a464b751d2162b Author: Eevee (Alex Munroe) <eevee.git@veekun.com> Date: 2014-08-19T23:07:36Z Fix useless warnings about a broken C extension on 3.x, PyPy, etc. Also remove use_2to3, since this is supposed to be single-codebase now. commit 3d912d748041f9a446cd0bf7580462d93316f60e Author: Eevee (Alex Munroe) <eevee.git@veekun.com> Date: 2014-08-19T23:12:16Z Fix a few minor nitpicks with the Python 3 fixes. commit ee916e9486364fa4130ff109b58ad43fef636757 Author: Eevee (Alex Munroe) <eevee.git@veekun.com> Date: 2014-08-19T23:28:07Z JSONProtocol's constants need to be bytestrings. commit 2b820077a77bd539c979cbae0998d4c562e80c35 Author: Eevee (Alex Munroe) <eevee.git@veekun.com> Date: 2014-08-19T23:28:55Z Use print_function in the Python tests. commit 242ed1e7aea7cf05512495aabcaf1452bbc27c6a Author: Eevee (Alex Munroe) <eevee.git@veekun.com> Date: 2014-08-19T23:29:09Z Unbreak the Python tests when the C extension doesn't exist. If there's no C part, everything gets built into `build/lib/`, but the shell globs were looking for a dot. commit 9c5161bc17d4ba7e89e8eb08bed2a481dc58f878 Author: Eevee (Alex Munroe) <eevee.git@veekun.com> Date: 2014-08-19T23:30:06Z Trailing whitespace my editor fixed for me. commit 863efcb78e43d1adbe17fd024b2546d054055497 Author: Eevee (Alex Munroe) <eevee.git@veekun.com> Date: 2014-08-20T02:53:07Z Probably don't need this 2.4 compat any more. commit 6981c4b3e0b8ed5480fc4dd794659beefe34a90d Author: Eevee (Alex Munroe) <eevee.git@veekun.com> Date: 2014-08-20T02:53:20Z Stop generating code that contains xrange and iteritems. commit 6b3d2749ec80c6fb1b20d4f6a810d84c7edf0dda Author: Eevee (Alex Munroe) <eevee.git@veekun.com> Date: 2014-08-20T02:53:50Z Make JSON and compact protocols use bytes everywhere. commit 5fec5813830eb6437ecb00effabfd6647dec6d9c Author: Eevee (Alex Munroe) <eevee.git@veekun.com> Date: 2014-08-20T02:54:31Z Python 3 support in test/py/. commit 2c06f4acea8993f3766e6120755d1ac23d00eef8 Author: Eevee (Alex Munroe) <eevee.git@veekun.com> Date: 2014-08-20T02:54:44Z Miscellaneous module moves and text/bytes stuff that was missed. commit 516406ba0f5698783552dbb012118e8964033698 Author: Eevee (Alex Munroe) <eevee.git@veekun.com> Date: 2014-08-20T03:14:21Z Fix more sys.path.insert glob shenanigans. commit 4c5ae2e5004de16f3389414b129a1ca88abda925 Author: Eevee (Alex Munroe) <eevee.git@veekun.com> Date: 2014-08-20T03:43:51Z Fix a bad relativized import. commit eb0132e1658428440c7b36624afc07d62616dd82 Author: Eevee (Alex Munroe) <eevee.git@veekun.com> Date: 2014-08-20T03:44:07Z Fix another old-style exception syntax in the Python generator. commit 43ea49778757ad0799769fc4cef242fea00b4914 Author: Eevee (Alex Munroe) <eevee.git@veekun.com> Date: 2014-08-20T03:46:13Z Run 2to3 on the Python tutorial code. commit c0fd1a85b7ee2016c0b237a07e45aac23fcb2386 Author: Eevee (Alex Munroe) <eevee.git@veekun.com> Date: 2014-08-20T04:08:50Z Don't compile to itervalues, either. commit 3b767742c1a5372a78943885f11a2f23e985a46a Author: Eevee (Alex Munroe) <eevee.git@veekun.com> Date: 2014-08-20T04:09:06Z Fix passing a float to struct.pack() – tolerated in 2, not 3. commit 19d9f4ebcf90679a29235ae4c4b8ba45d01405ef Author: Eevee (Alex Munroe) <eevee.git@veekun.com> Date: 2014-09-11T22:42:50Z And one more round of fixing JSON decoding.
          Hide
          haijunz Haijun Zhu added a comment -

          The C module does provide vast speedup according to our perf test. It should have minimal interaction with python, probably just get the struct/buffer and return buffer/struct.
          I made it to work in python 3 by copying some code from python2's cStringIO.c, and making the initialization code python2/3 compatible.

          Show
          haijunz Haijun Zhu added a comment - The C module does provide vast speedup according to our perf test. It should have minimal interaction with python, probably just get the struct/buffer and return buffer/struct. I made it to work in python 3 by copying some code from python2's cStringIO.c, and making the initialization code python2/3 compatible.
          Hide
          eevee Eevee added a comment -

          Is your perf test available?

          The problem (I believe) is that when decoding, it repeatedly calls back into Python asking for a handful of bytes at a time to be written to a StringIO. I don't know why it does this (instead of just asking for a buffer, or getting a file object, or whatever) and I'm wary of mucking with it without knowing for sure.

          Show
          eevee Eevee added a comment - Is your perf test available? The problem (I believe) is that when decoding, it repeatedly calls back into Python asking for a handful of bytes at a time to be written to a StringIO. I don't know why it does this (instead of just asking for a buffer, or getting a file object, or whatever) and I'm wary of mucking with it without knowing for sure.
          Hide
          roger.meier Roger Meier added a comment -

          We really need this! Any updates available?
          ;-r

          Show
          roger.meier Roger Meier added a comment - We really need this! Any updates available? ;-r
          Hide
          jeroenvlek Jeroen Vlek added a comment -

          After reading this thread I'm under the impression that you guys are very close. What's keeping you from giving the final push?

          Show
          jeroenvlek Jeroen Vlek added a comment - After reading this thread I'm under the impression that you guys are very close. What's keeping you from giving the final push?
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user jfarrell commented on the pull request:

          https://github.com/apache/thrift/pull/213#issuecomment-83257178

          hey @eevee, thanks for the work on this patch, any chance you could rebase this against the current trunk please, would like to see it make it in for THRIFT-1857

          Show
          githubbot ASF GitHub Bot added a comment - Github user jfarrell commented on the pull request: https://github.com/apache/thrift/pull/213#issuecomment-83257178 hey @eevee, thanks for the work on this patch, any chance you could rebase this against the current trunk please, would like to see it make it in for THRIFT-1857
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user helgridly commented on the pull request:

          https://github.com/apache/thrift/pull/213#issuecomment-83771333

          I did the rebase, ran tests on python3, and fixed up a few issues over (in my own fork)https://github.com/broadinstitute/thrift/tree/eevee/python3

          However, the tests fail on python2 with the same issue documented on JIRA (here)https://issues.apache.org/jira/browse/THRIFT-1857?focusedCommentId=14046961&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14046961

          @jfarrell How would you like me to proceed? I can open a separate PR with my changes, but I don't know what to do about the failing tests on python2.

          Show
          githubbot ASF GitHub Bot added a comment - Github user helgridly commented on the pull request: https://github.com/apache/thrift/pull/213#issuecomment-83771333 I did the rebase, ran tests on python3, and fixed up a few issues over (in my own fork) https://github.com/broadinstitute/thrift/tree/eevee/python3 However, the tests fail on python2 with the same issue documented on JIRA (here) https://issues.apache.org/jira/browse/THRIFT-1857?focusedCommentId=14046961&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14046961 @jfarrell How would you like me to proceed? I can open a separate PR with my changes, but I don't know what to do about the failing tests on python2.
          Hide
          jfarrell Jake Farrell added a comment -

          Haijun Zhu any chance you could share what you did for the fastbinary.c as a patch attached to this ticket? Eevee patch has been picked up by helgridly as noted in [1] and I would like to see what we can do to help get this into the next release candidate, only blocker right now sounds like the fastbinary.c portion not working with the new updates

          [1]: https://github.com/apache/thrift/pull/213

          Show
          jfarrell Jake Farrell added a comment - Haijun Zhu any chance you could share what you did for the fastbinary.c as a patch attached to this ticket? Eevee patch has been picked up by helgridly as noted in [1] and I would like to see what we can do to help get this into the next release candidate, only blocker right now sounds like the fastbinary.c portion not working with the new updates [1] : https://github.com/apache/thrift/pull/213
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user gwax commented on the pull request:

          https://github.com/apache/thrift/pull/440#issuecomment-92496624

          I have added the relevant (existing) Jira issue to the description. Is that sufficient or should I do something further?

          Show
          githubbot ASF GitHub Bot added a comment - Github user gwax commented on the pull request: https://github.com/apache/thrift/pull/440#issuecomment-92496624 I have added the relevant (existing) Jira issue to the description. Is that sufficient or should I do something further?
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user jfarrell commented on the pull request:

          https://github.com/apache/thrift/pull/440#issuecomment-92498770

          perfect, thanks

          Show
          githubbot ASF GitHub Bot added a comment - Github user jfarrell commented on the pull request: https://github.com/apache/thrift/pull/440#issuecomment-92498770 perfect, thanks
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user asfgit closed the pull request at:

          https://github.com/apache/thrift/pull/440

          Show
          githubbot ASF GitHub Bot added a comment - Github user asfgit closed the pull request at: https://github.com/apache/thrift/pull/440
          Hide
          hudson Hudson added a comment -

          SUCCESS: Integrated in Thrift #1508 (See https://builds.apache.org/job/Thrift/1508/)
          THRIFT-1857 Python 3.X Support - Replace deprecated "," with "as" in python exception generation code. (roger: rev 7726b03ac11c54502dad3a72c124c24fc17db1b3)

          • compiler/cpp/src/generate/t_py_generator.cc
          Show
          hudson Hudson added a comment - SUCCESS: Integrated in Thrift #1508 (See https://builds.apache.org/job/Thrift/1508/ ) THRIFT-1857 Python 3.X Support - Replace deprecated "," with "as" in python exception generation code. (roger: rev 7726b03ac11c54502dad3a72c124c24fc17db1b3) compiler/cpp/src/generate/t_py_generator.cc
          Hide
          githubbot ASF GitHub Bot added a comment -

          GitHub user jeroenvlek opened a pull request:

          https://github.com/apache/thrift/pull/455

          THRIFT-1857: Added some missing leading periods for the ttypes import

          This should fix the ttypes import.

          You can merge this pull request into a Git repository by running:

          $ git pull https://github.com/jeroenvlek/thrift master

          Alternatively you can review and apply these changes as the patch at:

          https://github.com/apache/thrift/pull/455.patch

          To close this pull request, make a commit to your master/trunk branch
          with (at least) the following in the commit message:

          This closes #455


          commit 928701c6f28cfd6bd96fdc8e2f3b4a46c65fc1e4
          Author: Jeroen Vlek <jv@datamantics.com>
          Date: 2015-04-21T06:37:56Z

          THRIFT-1857: Added some missing leading periods for the ttypes import


          Show
          githubbot ASF GitHub Bot added a comment - GitHub user jeroenvlek opened a pull request: https://github.com/apache/thrift/pull/455 THRIFT-1857 : Added some missing leading periods for the ttypes import This should fix the ttypes import. You can merge this pull request into a Git repository by running: $ git pull https://github.com/jeroenvlek/thrift master Alternatively you can review and apply these changes as the patch at: https://github.com/apache/thrift/pull/455.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #455 commit 928701c6f28cfd6bd96fdc8e2f3b4a46c65fc1e4 Author: Jeroen Vlek <jv@datamantics.com> Date: 2015-04-21T06:37:56Z THRIFT-1857 : Added some missing leading periods for the ttypes import
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user jeroenvlek commented on the pull request:

          https://github.com/apache/thrift/pull/455#issuecomment-94666360

          Oops, build failed. I'll look into it.

          Show
          githubbot ASF GitHub Bot added a comment - Github user jeroenvlek commented on the pull request: https://github.com/apache/thrift/pull/455#issuecomment-94666360 Oops, build failed. I'll look into it.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user jeroenvlek closed the pull request at:

          https://github.com/apache/thrift/pull/455

          Show
          githubbot ASF GitHub Bot added a comment - Github user jeroenvlek closed the pull request at: https://github.com/apache/thrift/pull/455
          Hide
          jeroenvlek Jeroen Vlek added a comment -

          Sorry guys, I wrongfully assumed that all of the above was already in the master branch. I should have clicked the links and checked the pull requests' statuses before opening my own. I closed it for now.

          Show
          jeroenvlek Jeroen Vlek added a comment - Sorry guys, I wrongfully assumed that all of the above was already in the master branch. I should have clicked the links and checked the pull requests' statuses before opening my own. I closed it for now.
          Hide
          githubbot ASF GitHub Bot added a comment -

          GitHub user gwax opened a pull request:

          https://github.com/apache/thrift/pull/493

          THRIFT-1857 Replace implicit relative imports with explicit relative imports.

          You can merge this pull request into a Git repository by running:

          $ git pull https://github.com/CloverHealth/thrift python3

          Alternatively you can review and apply these changes as the patch at:

          https://github.com/apache/thrift/pull/493.patch

          To close this pull request, make a commit to your master/trunk branch
          with (at least) the following in the commit message:

          This closes #493


          commit edf7e3e8678d4620e1c4662b2906457a076a943d
          Author: George Leslie-Waksman <george@cloverhealth.com>
          Date: 2015-05-14T19:24:06Z

          Fix relative imports to be explicit.


          Show
          githubbot ASF GitHub Bot added a comment - GitHub user gwax opened a pull request: https://github.com/apache/thrift/pull/493 THRIFT-1857 Replace implicit relative imports with explicit relative imports. You can merge this pull request into a Git repository by running: $ git pull https://github.com/CloverHealth/thrift python3 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/thrift/pull/493.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #493 commit edf7e3e8678d4620e1c4662b2906457a076a943d Author: George Leslie-Waksman <george@cloverhealth.com> Date: 2015-05-14T19:24:06Z Fix relative imports to be explicit.
          Hide
          jensg Jens Geyer added a comment -

          I just stumbled across this. Travis is green and it is only one small addition ... any open issue here? Then we should let him know.

          Show
          jensg Jens Geyer added a comment - I just stumbled across this. Travis is green and it is only one small addition ... any open issue here? Then we should let him know.
          Hide
          marek.jagielski@gmail.com Marek Jagielski added a comment -

          Hi,
          What is the status of this issue?

          I need to support jython 2.7+ and cpython 3.4+. By default, on jython 2.7, thrift 0.9.3 works fine with communication between java and py.
          I tried adapt thrift py library (ver 0.9.3) by myself. Among others by replacing: cStringIO.StringIO with io.BytesIO. Surprisingly communication worked on 3.4, ... but it stopped on jython. I will continue to investigate.
          However, I would prefer to use an official solution, it is why a question.

          Thanks,
          Marek

          Show
          marek.jagielski@gmail.com Marek Jagielski added a comment - Hi, What is the status of this issue? I need to support jython 2.7+ and cpython 3.4+. By default, on jython 2.7, thrift 0.9.3 works fine with communication between java and py. I tried adapt thrift py library (ver 0.9.3) by myself. Among others by replacing: cStringIO.StringIO with io.BytesIO. Surprisingly communication worked on 3.4, ... but it stopped on jython. I will continue to investigate. However, I would prefer to use an official solution, it is why a question. Thanks, Marek
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user asfgit closed the pull request at:

          https://github.com/apache/thrift/pull/213

          Show
          githubbot ASF GitHub Bot added a comment - Github user asfgit closed the pull request at: https://github.com/apache/thrift/pull/213
          Hide
          nsuke Aki Sukegawa added a comment -

          Committed, thanks all !

          Show
          nsuke Aki Sukegawa added a comment - Committed, thanks all !
          Hide
          hudson Hudson added a comment -

          SUCCESS: Integrated in Thrift #1715 (See https://builds.apache.org/job/Thrift/1715/)
          THRIFT-1857 Python 3 Support Client: Python Patch: Thomas Bartelmess, (nsuke: rev 760511f59b349c59982a64e249e6cf24c2b2f8f6)

          • test/py/TestServer.py
          • lib/py/src/protocol/TJSONProtocol.py
          • lib/py/src/TSerialization.py
          • tutorial/py.twisted/PythonClient.py
          • lib/py/src/compat.py
          • tutorial/py/PythonClient.py
          • build/docker/ubuntu/Dockerfile
          • test/py/TestSocket.py
          • lib/py/src/transport/TTransport.py
          • lib/py/src/transport/TZlibTransport.py
          • lib/py/src/server/TServer.py
          • lib/py/src/transport/TTwisted.py
          • lib/py/setup.py
          • lib/py/src/protocol/TBinaryProtocol.py
          • tutorial/py/PythonServer.py
          • test/py.twisted/test_suite.py
          • lib/py/src/transport/THttpClient.py
          • test/py/TestSyntax.py
          • test/py/SerializationTest.py
          • tutorial/py.tornado/PythonServer.py
          • compiler/cpp/src/generate/t_py_generator.cc
          • lib/py/src/TSCons.py
          • lib/py/src/server/THttpServer.py
          • lib/py/src/protocol/TBase.py
          • lib/py/src/TTornado.py
          • test/py/TSimpleJSONProtocolTest.py
          • lib/py/src/transport/TSocket.py
          • lib/py/src/protocol/TCompactProtocol.py
          • tutorial/py.tornado/PythonClient.py
          • tutorial/py.twisted/PythonServer.tac
          • build/travis/installDependencies.sh
          • test/py/TestEof.py
          • lib/py/src/protocol/TProtocol.py
          • lib/py/src/server/TProcessPoolServer.py
          • test/py.tornado/test_suite.py
          • test/py/TestClient.py
          • contrib/Vagrantfile
          • lib/py/src/server/TNonblockingServer.py
          • tutorial/py.twisted/PythonServer.py
          • test/py/RunClientServer.py
            THRIFT-1857 Python 3 Support Client: Python Patch: Nobuaki Sukegawa (nsuke: rev a185d7e78589a42e076379ae7165857e5e828e5c)
          • contrib/Vagrantfile
          • configure.ac
          • build/travis/installDependencies.sh
          • test/py/TestClient.py
          • Makefile.am
          • build/docker/centos/Dockerfile
          • test/known_failures_Linux.json
          • test/tests.json
          • test/py/TestServer.py
          • build/docker/ubuntu/Dockerfile
          • lib/py/Makefile.am
          Show
          hudson Hudson added a comment - SUCCESS: Integrated in Thrift #1715 (See https://builds.apache.org/job/Thrift/1715/ ) THRIFT-1857 Python 3 Support Client: Python Patch: Thomas Bartelmess, (nsuke: rev 760511f59b349c59982a64e249e6cf24c2b2f8f6) test/py/TestServer.py lib/py/src/protocol/TJSONProtocol.py lib/py/src/TSerialization.py tutorial/py.twisted/PythonClient.py lib/py/src/compat.py tutorial/py/PythonClient.py build/docker/ubuntu/Dockerfile test/py/TestSocket.py lib/py/src/transport/TTransport.py lib/py/src/transport/TZlibTransport.py lib/py/src/server/TServer.py lib/py/src/transport/TTwisted.py lib/py/setup.py lib/py/src/protocol/TBinaryProtocol.py tutorial/py/PythonServer.py test/py.twisted/test_suite.py lib/py/src/transport/THttpClient.py test/py/TestSyntax.py test/py/SerializationTest.py tutorial/py.tornado/PythonServer.py compiler/cpp/src/generate/t_py_generator.cc lib/py/src/TSCons.py lib/py/src/server/THttpServer.py lib/py/src/protocol/TBase.py lib/py/src/TTornado.py test/py/TSimpleJSONProtocolTest.py lib/py/src/transport/TSocket.py lib/py/src/protocol/TCompactProtocol.py tutorial/py.tornado/PythonClient.py tutorial/py.twisted/PythonServer.tac build/travis/installDependencies.sh test/py/TestEof.py lib/py/src/protocol/TProtocol.py lib/py/src/server/TProcessPoolServer.py test/py.tornado/test_suite.py test/py/TestClient.py contrib/Vagrantfile lib/py/src/server/TNonblockingServer.py tutorial/py.twisted/PythonServer.py test/py/RunClientServer.py THRIFT-1857 Python 3 Support Client: Python Patch: Nobuaki Sukegawa (nsuke: rev a185d7e78589a42e076379ae7165857e5e828e5c) contrib/Vagrantfile configure.ac build/travis/installDependencies.sh test/py/TestClient.py Makefile.am build/docker/centos/Dockerfile test/known_failures_Linux.json test/tests.json test/py/TestServer.py build/docker/ubuntu/Dockerfile lib/py/Makefile.am
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user gwax commented on the issue:

          https://github.com/apache/thrift/pull/493

          If nobody is going to look at it for 2 years...

          Show
          githubbot ASF GitHub Bot added a comment - Github user gwax commented on the issue: https://github.com/apache/thrift/pull/493 If nobody is going to look at it for 2 years...
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user gwax closed the pull request at:

          https://github.com/apache/thrift/pull/493

          Show
          githubbot ASF GitHub Bot added a comment - Github user gwax closed the pull request at: https://github.com/apache/thrift/pull/493
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user avis commented on the issue:

          https://github.com/apache/thrift/pull/493

          Maybe I will...

          Show
          githubbot ASF GitHub Bot added a comment - Github user avis commented on the issue: https://github.com/apache/thrift/pull/493 Maybe I will...
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user jeking3 commented on the issue:

          https://github.com/apache/thrift/pull/493

          Hi, I know there's a long backlog here. We've worked it down from 120 open PRs to under 100 in the last month. Typically it has been up to the submitter to refresh the pull request if it does not pass the CI builds and once they do, it has been up to the submitter to email the dev mailing list to get the attention of a committer to finish the commit.

          If you can rebase against master and reopen/push and it passes the CI build then we'll be in a much better position to merge it.

          Show
          githubbot ASF GitHub Bot added a comment - Github user jeking3 commented on the issue: https://github.com/apache/thrift/pull/493 Hi, I know there's a long backlog here. We've worked it down from 120 open PRs to under 100 in the last month. Typically it has been up to the submitter to refresh the pull request if it does not pass the CI builds and once they do, it has been up to the submitter to email the dev mailing list to get the attention of a committer to finish the commit. If you can rebase against master and reopen/push and it passes the CI build then we'll be in a much better position to merge it.

            People

            • Assignee:
              nsuke Aki Sukegawa
              Reporter:
              avi Avi Saranga
            • Votes:
              5 Vote for this issue
              Watchers:
              30 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development