An asyncio hub for eventlet #870

itamarst · 2024-01-03T19:36:14Z

Fixes #869

I gave up on multi-reader support, I can't even figure out how it works and it seems like a terrible idea (though people are using it, based on quick GitHub search).

Original source: https://gist.github.com/miguelgrinberg/829a20792d7283ae27b1f6a390c378b9

codecov · 2024-01-03T19:36:40Z

Codecov Report

Attention: 8 lines in your changes are missing coverage. Please review.

Comparison is base (a94a3cf) 54% compared to head (457c9d9) 55%.

Files	Patch %	Lines
eventlet/hubs/asyncio.py	90%	2 Missing and 6 partials ⚠️

Additional details and impacted files

@@          Coverage Diff           @@
##           master   #870    +/-   ##
======================================
  Coverage      54%    55%            
======================================
  Files          88     89     +1     
  Lines        9744   9832    +88     
  Branches     1814   1829    +15     
======================================
+ Hits         5297   5408   +111     
+ Misses       4081   4053    -28     
- Partials      366    371     +5

Flag	Coverage Δ
ipv6	`22% <14%> (-1%)`	⬇️
py310asyncio	`51% <91%> (?)`
py310epolls	`52% <16%> (-1%)`	⬇️
py310poll	`52% <16%> (-1%)`	⬇️
py310selects	`52% <16%> (-1%)`	⬇️
py311asyncio	`51% <91%> (?)`
py311epolls	`52% <16%> (-1%)`	⬇️
py312asyncio	`49% <91%> (?)`
py312epolls	`50% <16%> (-1%)`	⬇️
py37epolls	`50% <16%> (-1%)`	⬇️
py38epolls	`52% <16%> (-1%)`	⬇️
py38openssl	`51% <16%> (-1%)`	⬇️
py38poll	`52% <16%> (-1%)`	⬇️
py38selects	`52% <16%> (-1%)`	⬇️
py39dnspython1	`50% <16%> (-1%)`	⬇️
py39epolls	`52% <16%> (-1%)`	⬇️
py39poll	`52% <16%> (-1%)`	⬇️
py39selects	`52% <16%> (-1%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

tipabu · 2024-01-03T21:09:33Z

FWIW, I've worked with code that needed to allow multiple readers. The long and short of it was needing to have a lock that would play nice with both greenthreads and real threads to work around #432; more details are available in openstack/swift@6d16079 / LP #1710328

Unfortunately, it also worked across forks, which seemed unnecessary and likely to impact performance. Eventually I managed to convince myself it wasn't necessary since we were always using UDP/Unix domain sockets for logging, so we wound up dropping the logging lock in openstack/swift@e09d4bc

I don't really have anything to add beyond that; I'll try to give this a spin in the near future, though. I just wanted to add some color to

I gave up on multi-reader support, I can't even figure out how it works and it seems like a terrible idea (though people are using it, based on quick GitHub search).

itamarst · 2024-01-03T22:13:19Z

If someone can make it work, great, I just gave up because how it works at all is an utter mystery to me (notice eventlet.hubs.hub.Hub.secondaries is never accessed by anything!). Given the goal here is for people to port to asyncio, and I suspect this isn't an asyncio feature anyway, so porting would have to drop such usages anyway. And it sounds like in practice Swift doesn't need it anymore?

AUTHORS

4383 · 2024-01-10T14:59:41Z

eventlet/hubs/asyncio.py

+def is_available():
+    """Python 3 always has asyncio."""
+    return True


Seems related to python 2/3 compat. As we dropped support of Python 2.7, I think we could also remove that check

No, the Hub API requires this, if you don't have this it emits an warning. See https://github.com/eventlet/eventlet/blob/master/eventlet/hubs/__init__.py#L81-L88

eventlet/hubs/asyncio.py

tests/greenio_test.py

4383 · 2024-01-10T15:20:36Z

Docstrings should be documented. If we want to encourage migration to eventlet and, hence, hope to see this feature adopted, then we need to provide a well documented hub.

Do you already plan to add docstring or should we add it to our todo list, and, add doc through a follow up patch?

Concerning gates, they seems happy, even when the asyncio hub is enabled. That's encouraging!

itamarst · 2024-01-10T15:34:46Z

(I have implemented the start of next step, running asyncio in combination with eventlet, just to see if it actually works, and ... it does! I have a test with aiohttp talking to eventlet's WSGI server. So feeling good about this approach.)

itamarst · 2024-01-10T15:36:23Z

Docstrings should be documented. If we want to encourage migration to eventlet and, hence, hope to see this feature adopted, then we need to provide a well documented hub.

The public APIs are the eventlet.hubs.Hub interface, so in general that documentation is the relevant one. Users won't be using this class directly much, possibly not at all.

Will see if there's anything that does need it though.

Concerning gates, they seems happy, even when the asyncio hub is enabled. That's encouraging!

Gates?

4383 · 2024-01-10T16:22:05Z

Docstrings should be documented. If we want to encourage migration to eventlet and, hence, hope to see this feature adopted, then we need to provide a well documented hub.

The public APIs are the eventlet.hubs.Hub interface, so in general that documentation is the relevant one. Users won't be using this class directly much, possibly not at all.

Indeed.

Will see if there's anything that does need it though.

WFM.

Concerning gates, they seems happy, even when the asyncio hub is enabled. That's encouraging!

Gates?

jobs/tests triggered by github actions

…r detection.

itamarst · 2024-01-10T16:31:36Z

Filed #874 for multiple reader support.

4383

As far as I can see, these changes LGTM.

That could be useful, during the squash, to indicate in the commit message, that this patch is inspired from https://gist.github.com/miguelgrinberg/829a20792d7283ae27b1f6a390c378b9 . We shouldn't skimp on the history.

4383 · 2024-01-11T16:21:37Z

That could be useful, during the squash, to indicate in the commit message, that this patch is inspired from https://gist.github.com/miguelgrinberg/829a20792d7283ae27b1f6a390c378b9 . We shouldn't skimp on the history.

Oh just seen you already put a comment, awesome!

4383 · 2024-01-11T16:23:06Z

I let you rewrite the commit message and drop useless info.
I let you push the merge button.

* Update requirements from branch 'master' to 08f829d8375b4059af365191e0907069be9fb739 - Update eventlet to 0.35.0 0.35.0 ====== eventlet/eventlet#897 * [fix] fix truncate size nullable eventlet/eventlet#789 * [fix] Handle transport endpoint shutdown in conditions eventlet/eventlet#884 * [fix] Rework reject_bad_requests option eventlet/eventlet#890 * [fix] Fix NameError introduced by #826 eventlet/eventlet#890 * [feature] Support awaiting GreenThread in an `async def` context eventlet/eventlet#889 * [feature] Asyncio hub support for Python 3.7 to 3.9 eventlet/eventlet#886 * [fix] Fix bad exceptions handlings eventlet/eventlet#883 * [feature] Support using asyncio coroutines from inside greenlets eventlet/eventlet#877 * [removal] Remove deprecated CGIHTTPServer and SimpleHTTPServer eventlet/eventlet#881 * [feature] Add an asyncio hub for eventlet eventlet/eventlet#870 0.34.3 ====== eventlet/eventlet#875 * Fix security issue in the wsgi module related to RFC 9112 eventlet/eventlet#826 * Fix segfault, a new approach for greening existing locks eventlet/eventlet#866 * greendns: fix getaddrinfo parameter name eventlet/eventlet#809 * Fix deprecation warning on ssl.PROTOCOL_TLS eventlet/eventlet#872 * Pytests, fix error at teardown of TestGreenSocket.test_full_duplex eventlet/eventlet#871 * Skip test which uses Py cgi module eventlet/eventlet#865 * Drop old code based on python < 3.7.34.2 ====== eventlet/eventlet#861 * Allowing inheritance of GreenSSLSocket without overriding the __new_ method eventlet/eventlet#796 * [bug] Fix broken API related to `__version__` removal eventlet/eventlet#859 * [doc] Fix pypi broken link eventlet/eventlet#857 0.34.1 ====== eventlet/eventlet#842 * [bug] Fix memory leak in greendns eventlet/eventlet#810 * [infra] Fix OIDC authentication failure eventlet/eventlet#855 * [bug] Ignore asyncore and asynchat for Python 3.12+ eventlet/eventlet#804 0.34.0 (Not released on Pypi but landed with 0.34.1) ==================================================== * Dropped support for Python 3.6 and earlier. * Fix Python 3.13 compat by adding missing attibute '_is_main_interpreter' eventlet/eventlet#847 * Add support of Python 3.12 eventlet/eventlet#817 * Drop unmaintained and unused stdlib tests eventlet/eventlet#820 * Fix tests and CI for Python 3.7 and higher eventlet/eventlet#831 and eventlet/eventlet#832 * Stop claiming to create universal wheels eventlet/eventlet#841 * Fix green logging locks for Python versions <= 3.10 eventlet/eventlet#754 Change-Id: I909be1d1812eaed574525866dbc123083684571d

0.35.0 ====== eventlet/eventlet#897 * [fix] fix truncate size nullable eventlet/eventlet#789 * [fix] Handle transport endpoint shutdown in conditions eventlet/eventlet#884 * [fix] Rework reject_bad_requests option eventlet/eventlet#890 * [fix] Fix NameError introduced by #826 eventlet/eventlet#890 * [feature] Support awaiting GreenThread in an `async def` context eventlet/eventlet#889 * [feature] Asyncio hub support for Python 3.7 to 3.9 eventlet/eventlet#886 * [fix] Fix bad exceptions handlings eventlet/eventlet#883 * [feature] Support using asyncio coroutines from inside greenlets eventlet/eventlet#877 * [removal] Remove deprecated CGIHTTPServer and SimpleHTTPServer eventlet/eventlet#881 * [feature] Add an asyncio hub for eventlet eventlet/eventlet#870 0.34.3 ====== eventlet/eventlet#875 * Fix security issue in the wsgi module related to RFC 9112 eventlet/eventlet#826 * Fix segfault, a new approach for greening existing locks eventlet/eventlet#866 * greendns: fix getaddrinfo parameter name eventlet/eventlet#809 * Fix deprecation warning on ssl.PROTOCOL_TLS eventlet/eventlet#872 * Pytests, fix error at teardown of TestGreenSocket.test_full_duplex eventlet/eventlet#871 * Skip test which uses Py cgi module eventlet/eventlet#865 * Drop old code based on python < 3.7.34.2 ====== eventlet/eventlet#861 * Allowing inheritance of GreenSSLSocket without overriding the __new_ method eventlet/eventlet#796 * [bug] Fix broken API related to `__version__` removal eventlet/eventlet#859 * [doc] Fix pypi broken link eventlet/eventlet#857 0.34.1 ====== eventlet/eventlet#842 * [bug] Fix memory leak in greendns eventlet/eventlet#810 * [infra] Fix OIDC authentication failure eventlet/eventlet#855 * [bug] Ignore asyncore and asynchat for Python 3.12+ eventlet/eventlet#804 0.34.0 (Not released on Pypi but landed with 0.34.1) ==================================================== * Dropped support for Python 3.6 and earlier. * Fix Python 3.13 compat by adding missing attibute '_is_main_interpreter' eventlet/eventlet#847 * Add support of Python 3.12 eventlet/eventlet#817 * Drop unmaintained and unused stdlib tests eventlet/eventlet#820 * Fix tests and CI for Python 3.7 and higher eventlet/eventlet#831 and eventlet/eventlet#832 * Stop claiming to create universal wheels eventlet/eventlet#841 * Fix green logging locks for Python versions <= 3.10 eventlet/eventlet#754 Change-Id: I909be1d1812eaed574525866dbc123083684571d

pythonspeed added 10 commits January 3, 2024 09:38

A first draft hub implemented by Miguel Grinberg

b36a617

Original source: https://gist.github.com/miguelgrinberg/829a20792d7283ae27b1f6a390c378b9

Use the correct callback

62d5fcf

We control the loop

5ee9e54

Explanation

4b68bc6

Just don't support this for now, it's a terrible "feature"

964d9f4

Simplify, and make another test pass

76fc0b7

Pass another test

5ff5373

Unnecessary and also broke zmq

e76a9b6

Set asyncio loop for any asyncio code that might want to use it.

5e93bf7

Run test on asyncio

02c4eb2

itamarst linked an issue Jan 3, 2024 that may be closed by this pull request

An asyncio hub for eventlet #869

Closed

Merge branch 'master' into 869-an-asyncio-hub-for-eventlet

851fa7b

4383 mentioned this pull request Jan 10, 2024

Asyncio integration for eventlet #868

Open

10 tasks

4383 reviewed Jan 10, 2024

View reviewed changes

itamarst and others added 3 commits January 10, 2024 10:36

Merge branch 'master' into 869-an-asyncio-hub-for-eventlet

3c4e46c

Unnecessary.

8f90492

Docstrings.

000bc68

Refactor multiple-reader unsupportedness for better testing and bette…

884f7f7

…r detection.

itamarst requested a review from 4383 January 10, 2024 16:29

Add a skip for asyncio

457c9d9

4383 approved these changes Jan 11, 2024

View reviewed changes

itamarst merged commit df81d5d into master Jan 11, 2024
26 checks passed

itamarst deleted the 869-an-asyncio-hub-for-eventlet branch January 11, 2024 16:41

4383 mentioned this pull request Jan 18, 2024

[release] eventlet 0.35.0 #897

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

An asyncio hub for eventlet #870

An asyncio hub for eventlet #870

itamarst commented Jan 3, 2024

codecov bot commented Jan 3, 2024 •

edited

tipabu commented Jan 3, 2024

itamarst commented Jan 3, 2024

4383 Jan 10, 2024

itamarst Jan 10, 2024

4383 commented Jan 10, 2024

itamarst commented Jan 10, 2024

itamarst commented Jan 10, 2024

4383 commented Jan 10, 2024

itamarst commented Jan 10, 2024

4383 left a comment •

edited

4383 commented Jan 11, 2024

4383 commented Jan 11, 2024

An asyncio hub for eventlet #870

An asyncio hub for eventlet #870

Conversation

itamarst commented Jan 3, 2024

codecov bot commented Jan 3, 2024 • edited

Codecov Report

tipabu commented Jan 3, 2024

itamarst commented Jan 3, 2024

4383 Jan 10, 2024

Choose a reason for hiding this comment

itamarst Jan 10, 2024

Choose a reason for hiding this comment

4383 commented Jan 10, 2024

itamarst commented Jan 10, 2024

itamarst commented Jan 10, 2024

4383 commented Jan 10, 2024

itamarst commented Jan 10, 2024

4383 left a comment • edited

Choose a reason for hiding this comment

4383 commented Jan 11, 2024

4383 commented Jan 11, 2024

codecov bot commented Jan 3, 2024 •

edited

4383 left a comment •

edited