Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Python 3 support #143

Merged

Conversation

@CyrilRoelandteNovance
Copy link
Collaborator

@CyrilRoelandteNovance CyrilRoelandteNovance commented Jan 21, 2014

These eleven patches should bring support for Python 3. After applying them, HTTPretty behaves the same way in Python 2 and 3 (meaning that the bugs reported for the Python 2 version still exist :/).

Return a decoded string rather than bytes.
Thanks to Victor Stinner for this patch.
This ensures Python 3 compatibility.
The parse_requestline() function needs a text string as an input. In Python 3,
it had bytes.
StringIO might be either 'StringIO.StringIO' or 'io.BytesIO' depending on the
version of Python used. Do not hardcode the type in the test.
* in record_request, do not try to call json.dumps() on a dict() containing
  bytes;
* in test_recording_calls(), do not hardcode the expected version of Tornado.

There was a json.dumps(some_dict) and soe_dict had bytes values
In Python 2, dict(request.headers) will have all keys in lower case, while In
Python 3, the capital letters will still be there. This is because of internal
changes in the requests library. Let's use lower-case headers, so that this is
not an issue.
In Python 3, the parse_request() method uses iso-8859-1 to decode the path, but
not in Python 2. This causes test failures in Python 3. Fix that by encoding
the path back to iso-8859-1.
…reaming_responses()

This was cause by the use of text strings instead of bytes.
Bytes were needed in some place, instead of text strings.
@CyrilRoelandteNovance
Copy link
Collaborator Author

@CyrilRoelandteNovance CyrilRoelandteNovance commented Jan 21, 2014

The Python3 test fail, but it's because of a bug that is already there in Python 2 but just does not happen when running the test suite because of some dark magic. I filed a bug report there:

#144

I do not think it should block this pull request, since it's a Py2 bug and is not related to Python 3 support.

@coagulant
Copy link
Contributor

@coagulant coagulant commented Jan 23, 2014

I think, there is no need for unicode_literals, if we're targeting python 3.3 only (and this is perfectly pragmatic).
In fact using unicode_literals is considered harmful sometimes, see PythonCharmers/python-future#22 for examples.

Probably, #36 fix won't be necessary.

@CyrilRoelandteNovance
Copy link
Collaborator Author

@CyrilRoelandteNovance CyrilRoelandteNovance commented Jan 23, 2014

@coagulent: This is definitely not pragmatic. I would love to see every piece of software use Python 3, but this will not happen overnight. For instance, OpenStack still uses Python 2, and porting HTTPretty to Python 3 is one of the numerous tasks to tackle to get it to work on Python 3. In the meantime, we need to remain compatible with Python 2.

@coagulant
Copy link
Contributor

@coagulant coagulant commented Jan 23, 2014

"if we're targeting python 3.3 only" was supposed to be "2.x and 3.3+, not 3.2, 3.1, 3.0"
Sorry for misunderstanding.

@hroncok
Copy link
Contributor

@hroncok hroncok commented Jan 28, 2014

Hi, I'm interested in Py3 support as well. Is there anything I can do to help get this to actual release of HTTPretty?

@CyrilRoelandteNovance
Copy link
Collaborator Author

@CyrilRoelandteNovance CyrilRoelandteNovance commented Jan 28, 2014

@coagulant: Well, I'd like to make as few changes as possible. And supporting 3.0, 3.1 and 3.2 does not seem too bad :) So I'd rather keep it this way for now, and maybe consider such changes for a future release.

@hroncok: you can try the code in this pull request and see whether it has more bugs than the Python 2 version (master). I think not, so I intend to press the "merge" button ASAP and ask Gabriel to push a new release on PyPI.

@coagulant
Copy link
Contributor

@coagulant coagulant commented Jan 28, 2014

@CyrilRoelandteNovance Removing unicode_literals is another story. Django uses it after all, but I'd rather not see them in the codebase for reasons mentioned above. I'll submit a pull request after release, so we can discuss what approach is better ;)

@hroncok Btw, httpretty 0.5.9 version had some kind of python 3 support. I've been using it a bit for my py3 projects for some time. In case you need pypi version (with old api and known bugs I guess). Hope this won't be needed after new release.

@hroncok
Copy link
Contributor

@hroncok hroncok commented Jan 28, 2014

I somehow managed to run 0.6.5 however, I'm trying to package httpretty for Fedora and I don't want to package old and possibly buggy version when it seems Python 3 support is coming to new codebase.

BTW I don't think supporting Python 3 < 3.3 is worthy. While there are still systems running old versions of Python 2, Python 3 is still somehow considered a "new" thing and every system I know runs 3.3. If you want to keep only one code for both Pythons, supporting 2.6, 2.7 and 3.3 seems the best option. It's very well explained here http://lucumr.pocoo.org/2013/5/21/porting-to-python-3-redux/#drop-2-5-3-1-and-3-2

@jamielennox
Copy link
Contributor

@jamielennox jamielennox commented Jan 28, 2014

@hroncok - i've had a fedora package in the works for a while. I've been waiting for the 0.8 release to contain python 3 support and the fixes to the setup.py (which the fedora package could ignore)

@jamielennox
Copy link
Contributor

@jamielennox jamielennox commented Jan 28, 2014

Also i agree with supporting >3.3. Things like b'' are not supported in 3.0-3.2 which i think we rely on in the py3 branch.

@CyrilRoelandteNovance
Copy link
Collaborator Author

@CyrilRoelandteNovance CyrilRoelandteNovance commented Jan 28, 2014

OK. I'm just not willing to do any more changes that do not improve the Python 3.3 port for now.

I really do not know whether we currently support 3.0, 3.1 and 3.2, btw :)

CyrilRoelandteNovance added a commit that referenced this pull request Feb 3, 2014
Python 3 support
@CyrilRoelandteNovance CyrilRoelandteNovance merged commit de6c488 into gabrielfalcao:master Feb 3, 2014
1 check passed
1 check passed
default The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants