Python 3 Support - Githubissues

aaronlindner commented 10 years ago

Is there any plan to support Python 3? I'd like to use python-zimbra, but everything we've done here is in Python 3.

dploeger commented 10 years ago

Yes, I'm definitely planning on supporting Py3. This will be my first move towards it. If you can help, I'd be glad to accept pull requests. ;)

dploeger commented 10 years ago

Could you please checkout the new py3-branch? The tests seem to be fine (although the requests/responses are sorted in another way then in py2, so they give warnings)

It would be nice to know, if it really works in a py3-environment.

aaronlindner commented 10 years ago

I had to change in tools/auth.py line 43 to use a capital S timestamp = int(datetime.now().strftime("%S")) * 1000

And then I get to where its trying to send the request, but there seems to be some descrepency on when to use bytes vs. string format.

With SECRET_KEY as string: Traceback (most recent call last): File "zimbra.py", line 63, in main(sys.argv[1:]) File "zimbra.py", line 47, in main SECRET_KEY File "c:\Python34\lib\site-packages\pythonzimbra\tools\auth.py", line 104, in authenticate server.send_request(auth_request, response) File "c:\Python34\lib\site-packages\pythonzimbra\communication.py", line 52, in send_request self.timeout File "c:\Python34\lib\urllib\request.py", line 153, in urlopen return opener.open(url, data, timeout) File "c:\Python34\lib\urllib\request.py", line 453, in open req = meth(req) File "c:\Python34\lib\urllib\request.py", line 1166, in dorequest raise TypeError(msg) TypeError: POST data should be bytes or an iterable of bytes. It cannot be of type str.

With SECRET_KEY as bytes: Traceback (most recent call last): File "zimbra.py", line 63, in main(sys.argv[1:]) File "zimbra.py", line 47, in main SECRET_KEY File "c:\Python34\lib\site-packages\pythonzimbra\tools\auth.py", line 47, in authenticate pak = preauth.create_preauth(account, key, by, expires, timestamp) File "c:\Python34\lib\site-packages\pythonzimbra\tools\preauth.py", line 37, in create_preauth ).hexdigest() AttributeError: 'bytes' object has no attribute 'encode'

Which is farther then I got before just running 2to3.py

Thanks

JocelynDelalande commented 10 years ago

As a global advice, try using from __future__ import unicode_litterals (available from python 2.6), default strings will be unicode ones, no longer bytes, even when running with python 2.

This really helps you to be consistent and avoid handling py2/py3 string disjunctions by hand.

asmartin commented 9 years ago

Is there any update on python3 support for this?

dploeger commented 9 years ago

I've redone the py3-thing from scratch. (Please note the new py3-branch)

Could someone with a python 3 installation test this? At least the tests from the test suite complete.

@aaronlindner Could you perhaps again test this?

aaronlindner commented 9 years ago

I have tested this in python3.4 and I can get it to work with some minor changes. urllib.request.urlopen takes in data as bytes not str as well as returning bytes for read.

In connection.py, the urlopen call needs to have request.get_request().encode("utf-8") Then the 4 reads below it need to be ....read().decode("utf-8")

Not sure if you want to add this as more conditions on if sys.version. Another alternative would be to use the requests package, which I believe would take care of all this.

With this done I was able to read my inbox folder from following exactly what you have in the docs.

Thanks much for the help!

JocelynDelalande commented 9 years ago

I have tested this in python3.4 and I can get it to work with some minor changes. urllib.request.urlopen takes in data as bytes not str as well as returning bytes for read.

The propper way to handle that is to use from __future__ import unicode_literals and do explicit bytes conversion with myvar.encode('utf-8') ; no need to do conditionals on sys.version.

dploeger commented 9 years ago

Thanks, everybody. I just pushed an updated version and would be ready to release a release candidate of 2.0 with Python 3 support.

I also added the force to use TLSv1 when using Python 2, because my new test server didn't like ssl2/3-requests and - thanks to POODLE - SSLv3 should be gone now hopefully.

Could you please pull the new version and test again in your environments? Not that forcing TLSv1 will break older releases...

dploeger commented 9 years ago

I've reached a coverage of 100% for py2,py3, pypi and pypi3.

Now I'm waiting on a positive reply from one of you guys to create a RC. Thanks.

JocelynDelalande commented 9 years ago

Hi @dploeger sorry for late answer, I took some time to test/review the py3 branch

Missing things ?

It's missing things from the master, like a setup.py, maybe others things ?

API change

Seems to be an API change, if I do the AuthRequest by hand, I previously had a _content in the response, I can now grab the token directly from the response.

Before (master):

{u'authToken': u'0_xxx', u'lifetime': u'43200000', u'skin': u'serenity'}

After (py3 branch):

{u'authToken': {'_content': u'0_xxx'}, u'lifetime': {'_content': u'43200000'}, 'xmlns': u'urn:zimbraAccount', u'skin': {'_content': u'serenity'}}

Is that intended ? (just asking, IMHO it's ok to break API on major realeses) ? What's the actual change behind ?

Tests involving special chars ?

I would suggest adding some tests that use special chars. It can be done in dedicated tests but IMHO, the better is to use special chars wherever you can in your test cases. For most people arround the globe, those "special" chars that gives us (developpers) nightmares are their "normal" chars.

e.g: the CreateAccountRequest in test_admin.py would be a good candidate, for eg introducing a last_name with some "é" in it

Apart from that points, I did some quick tests using pythonzimbra and it seems to be working well. Thanks for all the job released to community :-)

dploeger commented 9 years ago

Missing things: Yes, I removed the setup.py, because I thought I had wrongly copied it from the pypi-branch. Were you relying on that? I could copy it back
API change: Yes, I had to do that in 267b50d18eb91b89ec315df25640538426dd0c97. The idea behind it is, that the responses returned by the get_response-method of either RequestXml or RequestJson should be the identical, which they weren't (this was basically a mistake in the RequestXml-class). I will post that warning again in the release notes
Tests involving special chars: You're right. I will do that.

Could you tell me, if you'd like to have the setup.py back?

JocelynDelalande commented 9 years ago

Missing things: Yes, I removed the setup.py, because I thought I had wrongly copied it from the pypi-branch. Were you relying on that? I could copy it back

Every python dev is relying on that :-)

API change: Yes, I had to do that in 267b50d. The idea behind it is, that the responses returned by the get_response-method of either RequestXml or RequestJson should be the identical, which they weren't (this was basically a mistake in the RequestXml-class). I will post that warning again in the release notes

Ok, good idea.

Could you tell me, if you'd like to have the setup.py back?

Sure, I added it myself in #2 a couple of months ago ;-)

dploeger commented 9 years ago

Oh, whoops. That was where they came from. :)

Recreated them. Sorry.

I will add the character tests and then merge the branches back together and create an rc.

dploeger commented 9 years ago

Added the character tests in 663f799a39365fc613062a2be378187c33a8602b.

Thanks, @JocelynDelalande for your idea about the non-ascii-character tests. That revealed some bugs in request/response handling!

dploeger commented 9 years ago

Thanks everyone for contributing your ideas. I will merge the py3 branch now to master and create a release candidate for 2.0

Zimbra-Community / python-zimbra

Python 3 Support #5

Missing things ?

API change

Tests involving special chars ?