eventlet.websocket is not always used from eventlet.wsgi, so do not assume eventlet.set_idle exists#949
Merged
4383 merged 2 commits intoeventlet:masterfrom Mar 29, 2024
Merged
Conversation
Contributor
Author
|
Note that I don't know of any good way to test this inside Eventlet... it seems fiendishly complicated to mock a Gunicorn worker, so that's left as an exercise to the maintainer :) |
4383
approved these changes
Mar 26, 2024
Member
4383
left a comment
4383
left a comment
There was a problem hiding this comment.
Concerning myself, this patch LGTM.
I'd appreciate to have a secondary opinion from Tim.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #949 +/- ##
=====================================
- Coverage 56% 56% -1%
=====================================
Files 89 89
Lines 9766 9767 +1
Branches 1818 1819 +1
=====================================
Hits 5475 5475
- Misses 3920 3921 +1
Partials 371 371
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Member
|
Well, I think these changes are well isolated and should not trigger side effects. Let's go ahead to fix the latest release. Thanks @dhdaines for your patch,for your time and for the interesting discussion. Much appreciated. |
openstack-mirroring
pushed a commit
to openstack/requirements
that referenced
this pull request
Apr 5, 2024
It will allow us to benefit from several recent fixes related to the new asyncio hub. We started to implement some devstack integrations with the new eventlet asyncio hub [1], so it is worth using a version that contains useful fixes. See https://review.opendev.org/c/openstack/devstack/+/914108 Also this patch propose to blacklist version 0.36.0 which is buggy version in some wsgi context. 0.36.1 fix that wsgi problem. For further details see: - eventlet/eventlet#946 - eventlet/eventlet#949 Change-Id: I2add78de0e3d2f439964afa0d0c9d854a52b5f7f
openstack-mirroring
pushed a commit
to openstack/openstack
that referenced
this pull request
Apr 5, 2024
* Update requirements from branch 'master'
to 4ba203d335842ba76005e0ab3a395c6b1f5853c4
- Merge "bump eventlet to the latest version"
- bump eventlet to the latest version
It will allow us to benefit from several recent fixes
related to the new asyncio hub.
We started to implement some devstack integrations
with the new eventlet asyncio hub [1], so it is worth
using a version that contains useful fixes.
See https://review.opendev.org/c/openstack/devstack/+/914108
Also this patch propose to blacklist version 0.36.0 which is
buggy version in some wsgi context. 0.36.1 fix that wsgi problem.
For further details see:
- eventlet/eventlet#946
- eventlet/eventlet#949
Change-Id: I2add78de0e3d2f439964afa0d0c9d854a52b5f7f
clrpackages
pushed a commit
to clearlinux-pkgs/pypi-eventlet
that referenced
this pull request
Jul 10, 2024
…rsion 0.36.1 0.36.1 ====== * [fix] eventlet.websocket is not always used from eventlet.wsgi, so do not assume eventlet.set_idle exists eventlet/eventlet#949 0.36.0 ====== * [fix] Make sure asyncio hub doesn't use greendns for asyncio DNS APIs eventlet/eventlet#938 * [fix] Make asyncio.to_thread work with the same semantics as normal asyncio eventlet/eventlet#930 * [fix] Refactor congruence checks based on assert at runtime eventlet/eventlet#932 * [tests] Run tests on macOS in CI, and some fixes to get it in reasonable state (#list eventlet/eventlet#934 * [fix] Fix wsgi.server shutdown for in-flight requests eventlet/eventlet#912 * [feature] Add debug convenience helpers - asyncio, threads eventlet/eventlet#925 * [fix] Handle errors better. eventlet/eventlet#923 (NEWS truncated at 15 lines) CVEs fixed in this build: CVE-2023-29483
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
In the case where Eventlet's WebSocket implementation is used, for instance, in a Gunicorn worker, it ends up being the case that
WebSocketWSGIis not running under an Eventlet server, and thus shouldn't assume that the environment containseventlet.set_idle(which is an implementation detail of the Eventlet server and thus quite meaniningless).This is notably the case when using Flask-SocketIO with Gunicorn as suggested in the documentation: https://flask-socketio.readthedocs.io/en/latest/deployment.html#gunicorn-web-server
It seems quite safe to just avoid trying to call it in this case. If, for instance, the engineio wrapper (https://github.com/miguelgrinberg/python-engineio/blob/main/src/engineio/async_drivers/eventlet.py#L39) wishes to provide
eventlet.set_idlefor some reason, then it could do so.Fixes #946