Python 3.12 fixes by hroncok

[nat-goodspeed]

Credit to @hroncok and @vstinner for logic (#797).
This PR attempts to add compatibility for older Python versions.

We draw the line at Python versions that predate ssl.SSLContext, though. The
remaining Python 2.7 documentation doesn't even mention the version at which
that was introduced.

[codecov]

Codecov Report

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

Comparison is base (d78f8f6) 53% compared to head (4779936) 53%.

Files	Patch %	Lines
eventlet/green/ssl.py	82%	3 Missing and 5 partials ⚠️
eventlet/green/http/client.py	50%	4 Missing and 2 partials ⚠️

Additional details and impacted files

@@          Coverage Diff          @@
##           master   #817   +/-   ##
=====================================
  Coverage      53%    53%           
=====================================
  Files          88     88           
  Lines        9848   9865   +17     
  Branches     1841   1846    +5     
=====================================
+ Hits         5299   5323   +24     
+ Misses       4164   4156    -8     
- Partials      385    386    +1     

Flag	Coverage Δ
ipv6	22% <40%> (-1%)	⬇️
py310epolls	52% <66%> (+<1%)	⬆️
py310poll	52% <66%> (+<1%)	⬆️
py310selects	52% <66%> (+<1%)	⬆️
py311epolls	52% <66%> (+<1%)	⬆️
py312epolls	50% <74%> (?)
py38epolls	52% <66%> (+<1%)	⬆️
py38openssl	50% <66%> (+<1%)	⬆️
py38poll	52% <66%> (+<1%)	⬆️
py38selects	52% <66%> (+<1%)	⬆️
py39dnspython1	50% <66%> (+<1%)	⬆️
py39epolls	52% <66%> (+<1%)	⬆️
py39poll	52% <66%> (+<1%)	⬆️
py39selects	52% <66%> (+<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.

[vstinner]

FYI Fedora currently uses this patch for Python 3.12: https://src.fedoraproject.org/rpms/python-eventlet/blob/rawhide/f/python3.12.patch

[temoto]

@vstinner thanks, i hope few small changes here would not create conflict issue in fedora?

[vstinner]

@vstinner thanks, i hope few small changes here would not create conflict issue in fedora?

If eventlet is ported to Python 3.12, we will just remove our patch in Fedora and update the package to the new eventlet release.

[nat-goodspeed]

I'm sorry to say I don't know how to resolve the many failures of verify_hub_empty() or

ssl.SSLError: [SSL: KRB5_S_TKT_NYV] unexpected eof while reading (_ssl.c:2570)

I'm willing to work on a fix if I could have diagnostic help.

[temoto]

I'm sorry to say I don't know how to resolve the many failures of verify_hub_empty() or

ssl.SSLError: [SSL: KRB5_S_TKT_NYV] unexpected eof while reading (_ssl.c:2570)

I'm willing to work on a fix if I could have diagnostic help.

95% sure that is not related to this changeset.

KRB5_S_TKT_NYV seems like openssl compatibility, non empty hub is likely ref count (mem leak).

[nat-goodspeed]

Have you seen similar failures on other branches?

[temoto]

Have you seen similar failures on other branches?

verify_hub_empty occurred on nose branch. KRB5_S_TKT_NYV is new.

[nat-goodspeed]

I ran the tests locally on Python 3.10 against master and against this python3.12 branch, and there was only one difference (other than timestamps, object IDs and PIDs).

======================================================================
FAIL: test_024_expect_100_continue (tests.wsgi_test.TestHttpd)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/nat/linden/eventlet-nat/tests/wsgi_test.py", line 825, in test_024_expect_100_continue
    with self.assertRaises(socket.error) as caught:
AssertionError: OSError not raised

======================================================================

Really, the question is: if this unblocks Python 3.12, is it worth merging?

[jayofdoom]

Thanks for working on this.

A major downside to the approach taken here is the inability for library callers to configure the SSLContext. I wonder if there's a way to factor this where the application using the eventlet ssl module can offer some configuration -- or maybe even a preconfigured SSLContext -- in order to allow the application control over what TLS/SSL versions are supported, and certificates accepted.

[jayofdoom]

I know @itamarst was going to take a look at py 3.12 support in eventlet as well. I've linked this PR to him.

[itamarst]

I'm happy to leave this part to someone else, but I've been just trying to get tests to run on 3.12 and discovered nose3 is not compatible with 3.12. So I'm going to see if I can fix that (see #638 for previous discussion of nose).

[nat-goodspeed]

@jayofdoom

A major downside to the approach taken here is the inability for library callers to configure the SSLContext.

It looks as though you can instantiate GreenSSLSocket(..., _context=my_context). Maybe that should be more prominently exposed? I'm open to suggestion on this point.

[itamarst]

#823 has a bunch of fixes to make tests pass, so should be helpful in validating 3.12 support actually works.

[doko42]

python 3.12 support needs an update for the cpython 3.12 branch apparently, see
python/cpython#112826

[onlined]

Quoting from Python documentation:

Deprecated since version 3.7: Since Python 3.2 and 2.7.9, it is recommended to use the SSLContext.wrap_socket() instead of wrap_socket(). The top-level function is limited and creates an insecure client socket without server name indication or hostname matching.

I think it's not needed anymore to use ssl.wrap_socket and do unnecessary branching because eventlet doesn't support versions older than 3.2 and 2.7. You can only use SSLContext.wrap_socket. Am I missing something?

[4383]

Now that many CI fixes have been merged (thanks @itamarst), I think we can rerun checks. Normally, Py38 to Py311 jobs should be green.

[itamarst]

I got everything passing. Further steps... since we're supporting 3.8+, and 3.8 already has ssl.SSLContext, possibly the ssl.wrap_socket codepath can just be dropped altogether.

[itamarst]

Note there were some random irrelevant changes to test modules in the original PR.

[4383]

hmm... I'm not sure to understand your intentions here.
Your commit message said that we only support py38 ("Simplify given we only support 3.8+") but you removed the section related to py311 and earlier versions and you keep the code related to py312.

Should we not keep this condition to differentiate Py38-Py311 and Py312+.

Please can you give more details about your thinking.

[itamarst]

The original comment was wrong. It assumed the new API is new in 3.12, but it actually exists in 3.8 too.

[4383]

ack. thanks for details. Then LGTM

[4383]

LGTM

As this a significant step for eventlet I'd suggest release a new version now, when this patch will be merged.

@itamarst: Is it ok for you? If yes, do you want to proceed the new tag and the build/upload?