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

Two fixes to thriftpy_gevent_worker.py for py3 compat #50

Merged
merged 1 commit into from Nov 16, 2017

Conversation

aawilson
Copy link
Contributor

@aawilson aawilson commented Nov 2, 2017

First fix: Wrap low-level "thread" import with try-except to fallback to
the Python 3-specific "_thread" module.

Second fix: Remove now-obsolete tuple-unpacking syntax from
_greenlet_switch_tracer method signature, and replace with
vesion-agnostic tuple unpack in method body.

@aawilson
Copy link
Contributor Author

aawilson commented Nov 3, 2017

In the process of fixing the tests

@aawilson aawilson force-pushed the 49/gevent_python_3 branch 2 times, most recently from edabd96 to be8a4c6 Compare November 3, 2017 19:40
@aawilson
Copy link
Contributor Author

aawilson commented Nov 3, 2017

Test requirements updated. Note that this PR changes py27 requirements to use gevent 1.2(.2), and the env requirement to remove the <1.2 guard. If there's some reason I'm missing that would make that a bad idea, it's highly likely they can both be set back down to the lower version, but it should be fine as long as you're reasonably sure this project's tests exercise what they need to exercise.

Will probably need a version increment, but I'm not familiar enough with this project's standards to know what your bar is for that, or whether it belongs in the PR itself.

First fix: Wrap low-level "thread" import with try-except to fallback to
the Python 3-specific "_thread" module.

Second fix: Remove now-obsolete tuple-unpacking syntax from
_greenlet_switch_tracer method signature, and replace with
vesion-agnostic tuple unpack in method body

README updated and py27 test markers removed from thriftpy_gevent worker
test
@aawilson
Copy link
Contributor Author

@wooparadog Any chance this can get looked at? I'd like to stop using a github reference in my requirements files.

@wooparadog
Copy link
Member

@aawilson Thanks for your contribution, sorry about not having time to look at your pull request. But I'll look into it within this week.

@wooparadog wooparadog merged commit f7e3c88 into Thriftpy:master Nov 16, 2017
@aawilson aawilson deleted the 49/gevent_python_3 branch November 17, 2017 00:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants