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

Bump gevent and greenlet dependencies for Python 3.7 compatibility #259

Merged
merged 2 commits into from
Feb 11, 2020

Conversation

apakulov-stripe
Copy link
Contributor

@apakulov-stripe apakulov-stripe commented Feb 10, 2020

During the rollout of confidant 5.2.0 we ran into a number of issues with Python 3.7.

First of all pypi does not have python3.7 binaries for gevent 1.2.1: https://pypi.org/project/gevent/1.2.1/#files and same for greenlet 0.4.12: https://pypi.org/project/greenlet/0.4.12/#files

I tried bumping gevent to 1.3.0 and greenlet to 0.4.14, but gevent workers were segfaulting after the startup with the following log statements:

[18839] [CRITICAL] WORKER TIMEOUT (pid:20879)

After enabling https://docs.python.org/3/library/faulthandler.html we were able to get the actual stacktrace:

[2020-02-05 00:17:00.379340] Fatal Python error: Segmentation fault
[2020-02-05 00:17:00.379513] 
[2020-02-05 00:17:00.379578] Thread 0x00007fd427e5c700 (most recent call first):
[2020-02-05 00:17:00.380093]   File "/vendor3/lib/python3.7/site-packages/gevent/_threading.py", line 80 in wait
[2020-02-05 00:17:00.380242]   File "/vendor3/lib/python3.7/site-packages/gevent/_threading.py", line 162 in get
[2020-02-05 00:17:00.380312]   File "/vendor3/lib/python3.7/site-packages/gevent/threadpool.py", line 270 in _worker
[2020-02-05 00:17:00.380358] 
[2020-02-05 00:17:00.380410] Current thread 0x00007fd4325e3700 (most recent call first):
[2020-02-05 00:17:00.380481]   File "/confidant/authnz/__init__.py", line 70 in user_is_user_type
[2020-02-05 00:17:00.380554]   File "/confidant/authnz/__init__.py", line 109 in user_is_authorized
[2020-02-05 00:17:00.380613]   File "/confidant/routes/v1.py", line 505 in _get_credentials
[2020-02-05 00:17:00.380673]   File "/confidant/routes/v1.py", line 137 in get_service
[2020-02-05 00:17:00.380730]   File "/confidant/authnz/__init__.py", line 215 in decorated
[2020-02-05 00:17:00.380791]   File "/vendor3/lib/python3.7/site-packages/flask/app.py", line 1935 in dispatch_request
[2020-02-05 00:17:00.380853]   File "/vendor3/lib/python3.7/site-packages/flask/app.py", line 1949 in full_dispatch_request
[2020-02-05 00:17:00.380915]   File "/vendor3/lib/python3.7/site-packages/flask/app.py", line 2446 in wsgi_app
[2020-02-05 00:17:00.381047]   File "/vendor3/lib/python3.7/site-packages/guard.py", line 62 in __call__
[2020-02-05 00:17:00.381204]   File "/vendor3/lib/python3.7/site-packages/flask/app.py", line 2463 in __call__
[2020-02-05 00:17:00.381326]   File "/vendor3/lib/python3.7/site-packages/gunicorn/workers/base_async.py", line 107 in handle_request
[2020-02-05 00:17:00.381422]   File "/vendor3/lib/python3.7/site-packages/gunicorn/workers/ggevent.py", line 160 in handle_request
[2020-02-05 00:17:00.381493]   File "/vendor3/lib/python3.7/site-packages/gunicorn/workers/base_async.py", line 56 in handle
[2020-02-05 00:17:00.381576]   File "/vendor3/lib/python3.7/site-packages/gunicorn/workers/ggevent.py", line 155 in handle
[2020-02-05 00:17:00.381651]   File "/vendor3/lib/python3.7/site-packages/gevent/baseserver.py", line 26 in _handle_and_close_when_done

Bumping both dependencies to the latest stable version resolved the segfaults above.

@ryan-lane
Copy link
Contributor

Hey, this is great. Thanks for the PR! Do you mind matrixing the github workflow so that we're testing 3.6 and 3.7, so we see the tests pass on 3.7?

Here's an example from kmsauth: https://github.com/lyft/python-kmsauth/blob/master/.github/workflows/pull_request.yml#L18-L27

@ryan-lane
Copy link
Contributor

We're dropping support for 2.x, so for now just matrixing between 3.6 and 3.7 is OK.

@ryan-lane
Copy link
Contributor

That failure for 3.8 is due to coverage being out out date. Bump to the newest version and it should fix it.

@apakulov-stripe apakulov-stripe force-pushed the apakulov/python37-dep-fix branch from 72856f9 to deb1907 Compare February 11, 2020 01:20
@ryan-lane
Copy link
Contributor

Awesome. Thanks so much for the changes! No worries about 3.8. I'll followup with another change to pull that into the matrix and fix the tests for it.

@ryan-lane ryan-lane merged commit 9f0702e into lyft:master Feb 11, 2020
@apakulov-stripe
Copy link
Contributor Author

That failure for 3.8 is due to coverage being out out date. Bump to the newest version and it should fix it.

Ouch, just noticed it :)

@ryan-lane
Copy link
Contributor

I take it back. I was being lazy and mocking things outside of request context, and I guess how that's handled in 3.8 is different. Fixed in #260 :)

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.

2 participants