Skip to content
This repository has been archived by the owner on Jun 9, 2020. It is now read-only.

Python 3 support #153

Closed
jayvdb opened this issue Mar 28, 2019 · 8 comments
Closed

Python 3 support #153

jayvdb opened this issue Mar 28, 2019 · 8 comments

Comments

@jayvdb
Copy link
Contributor

jayvdb commented Mar 28, 2019

This project has Python-2 only syntax. Those should be easy to fix, in order to see the more difficult problems.

See https://travis-ci.org/jayvdb/pywebsocket/jobs/512359638

@ricea
Copy link
Contributor

ricea commented Mar 28, 2019

Are you volunteering?

We will need to continue to support python 2 for the time being, but IIUC that shouldn't be a big problem.

@jayvdb
Copy link
Contributor Author

jayvdb commented Mar 28, 2019

Bugs in Python 2 also #154

I am volunteering - raising new non-duplicate issues is a significant contribution to any project.

Note there are significant enhancements to this project at https://github.com/web-platform-tests/wpt/tree/master/tools/pywebsocket , and likely quite a few Python 3 fixes , such as web-platform-tests/wpt#15141 . But it take a bit of effort to pull their enhancements in.

@ricea
Copy link
Contributor

ricea commented Mar 28, 2019

Note there are significant enhancements to this project at https://github.com/web-platform-tests/wpt/tree/master/tools/pywebsocket , and likely quite a few Python 3 fixes , such as web-platform-tests/wpt#15141 . But it take a bit of effort to pull their enhancements in.

Oh dear. I didn't realise they'd forked it. I shall have to allocate some time for converging the versions.

@jayvdb
Copy link
Contributor Author

jayvdb commented Apr 1, 2019

I've done a quick modernise of the syntax, which you can see at https://github.com/jayvdb/pywebsocket/tree/py3 .

However what remains is a mammoth task of bytes vs str, and some important decisions need to be made by the maintainers for which layers of the codebase will be using bytes and when it switches to strings(unicode).

I've done a bit of local-only hacking with headers dicts being unicode keys and raw bytes values, and that is a fairly easy path to get a lot of the test passing without large scale modifications, but I assume that approach will be unacceptable for merging as it leaves a lot of the onus of bytes/unicode on the caller, and there is py2/3 bits littered through the layers of the codebase instead of being kept in one spot. The user of the library will probably want to be able to put bytes or unicode into header values and expect they are handled correctly, magically. Anyways, I dont want to go too far without some direction. To do this nicely would require the use of a headers dict which does a lot of the conversion internally, like the requests library (however iirc its headers dict cant handle all types of HTTP header streams as it can simply dumb down the stream and 99.99% of users do not care. Relevant issues at https://github.com/kennethreitz/requests/issues?q=is%3Aissue+sort%3Aupdated-desc+memento+is%3Aclosed), so that user layer and network layers essentially use the headers dict to do all the messy stuff, and the network layer then deals strictly with bytes.

@foolip
Copy link

foolip commented May 8, 2019

@Ms2ger @gsnedders FYI that py3 changes in WPT's pywebsocket have been noticed.

@Hexcles
Copy link

Hexcles commented Feb 13, 2020

To close the loop, the answer is https://github.com/GoogleChromeLabs/pywebsocket3
(@ricea perhaps it's a good idea to add a link to pywebsocket3 in README.)

Also for people interested in WPT, we have an issue tracking the switch: web-platform-tests/wpt#21749

@gsnedders
Copy link

Oh dear. I didn't realise they'd forked it. I shall have to allocate some time for converging the versions.

IIRC, we took some of the Python 3 changes because we'd already forked pywebsocket and there didn't seem to be any movement here, per the discussion in web-platform-tests/wpt#21868.

@ricea
Copy link
Contributor

ricea commented Jun 3, 2020

I have a PR to link to pywebsocket3 in the README. Closing this issue as we won't make further changes to this version of pywebsocket.

@ricea ricea closed this as completed Jun 3, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants