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

Migrate to gunicorn + gevent #1350

Closed
wants to merge 1 commit into from
Closed

Migrate to gunicorn + gevent #1350

wants to merge 1 commit into from

Conversation

robhudson
Copy link
Contributor

No description provided.

@robhudson robhudson requested a review from stevejalim April 3, 2024 20:35
Copy link

codecov bot commented Apr 3, 2024

Codecov Report

Attention: Patch coverage is 0% with 17 lines in your changes are missing coverage. Please review.

Project coverage is 87.24%. Comparing base (f945e17) to head (f01aaee).

Files Patch % Lines
basket/wsgi_config.py 0.00% 17 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1350      +/-   ##
==========================================
- Coverage   87.82%   87.24%   -0.58%     
==========================================
  Files          42       43       +1     
  Lines        2571     2588      +17     
==========================================
  Hits         2258     2258              
- Misses        313      330      +17     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@robhudson
Copy link
Contributor Author

I'm seeing some Python 3.12 + gevent issues:

RuntimeError: can't create new thread at interpreter shutdown

Python 3.12 no longer allows creating a thread on interpreter shutdown:
python/cpython#104826

Copy link
Contributor

@stevejalim stevejalim left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

r+ but why are we rolling this away from uwsgi? I half remember some niggles, which is presumably where gunicorn+gevent (rather than gunicorn+meinheld) is overall more stable, yeah?

Makes me wonder if we should also move Nucleus to match - I think we should

@robhudson
Copy link
Contributor Author

Basket has been a good project to be the "lead", so to speak, on some library choices in the past. Since it's smaller and less complex. pmac and I discussed migrating basket to match bedrock so they are more in sync.

In some ways, this has already been beneficial since we've just discovered an issue with gevent workers under Python 3.12 due to the above mentioned change. So it's something to solve for bedrock (and basket, assuming we agree this is a good change).

@robhudson
Copy link
Contributor Author

Historical note: We switched to granian on bedrock so closing this for now until the dust settles.

@robhudson robhudson closed this Jul 9, 2024
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