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

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

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 ?

Is there any timeline for Python 3 support?

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

Oops, attached in wrong issue, sorry ..

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

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?

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.

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

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

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

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.

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.

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.

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

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

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.

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.

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.

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.

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

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

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

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.

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

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?

Github user jfarrell commented on the pull request:

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

perfect, thanks

Github user asfgit closed the pull request at:

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

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

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

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.

Github user jeroenvlek closed the pull request at:

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

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.

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.

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.

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

Github user asfgit closed the pull request at:

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

Committed, thanks all !

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

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...

Github user gwax closed the pull request at:

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

Github user avis commented on the issue:

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

Maybe I will...

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.