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

WIP Python 3.12 fixes #797

Closed
wants to merge 4 commits into from
Closed

WIP Python 3.12 fixes #797

wants to merge 4 commits into from

Conversation

hroncok
Copy link
Contributor

@hroncok hroncok commented May 4, 2023

This makes the tests pass in Fedora. I have no ide what I am doing, so it may be entirely wrong.

This probably breaks with older Pythons.

Fixes #795

@codecov
Copy link

codecov bot commented May 4, 2023

Codecov Report

Patch coverage has no change and project coverage change: -27 ⚠️

Comparison is base (dd2f0ea) 38% compared to head (293065c) 12%.

Additional details and impacted files
@@           Coverage Diff            @@
##           master    #797     +/-   ##
========================================
- Coverage      38%     12%    -27%     
========================================
  Files          88      88             
  Lines       13084   10629   -2455     
  Branches     1830    1329    -501     
========================================
- Hits         5074    1338   -3736     
- Misses       7586    9158   +1572     
+ Partials      424     133    -291     
Flag Coverage Δ
ipv6 12% <0%> (-1%) ⬇️
py36epolls ?

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

Impacted Files Coverage Δ
eventlet/green/http/client.py <1% <0%> (-30%) ⬇️
eventlet/green/ssl.py 11% <ø> (-27%) ⬇️
eventlet/green/thread.py 42% <0%> (+2%) ⬆️

... and 68 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@hroncok
Copy link
Contributor Author

hroncok commented May 17, 2023

I've split this into 4 individual commits for easier review.

)
if ca_certs:
context.load_verify_locations(ca_certs)
if ciphers := kw.get('ciphers'):
Copy link
Member

Choose a reason for hiding this comment

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

sorry, this is 3.8+ construct, not going to happen anytime soon

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I know. My aim for this draft was to fix this for 3.12. Maintaining compatibility with previous Python versions was out of scope. If we can determine this approach is correct, we can add a giant if around the code. If the walrus would cause a problem even inside an if, no problem removing it.

Copy link
Contributor

Choose a reason for hiding this comment

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

sorry, this is 3.8+ construct, not going to happen anytime soon

It can happen now! (#827)

gizmoguy added a commit to faucetsdn/eventlet that referenced this pull request Oct 5, 2023
Python 3.12 compatibility fix based on eventlet#797, absent use of :=
@itamarst
Copy link
Contributor

Hi, what needs to be done to get this merged?

@temoto
Copy link
Member

temoto commented Oct 27, 2023

Hi, what needs to be done to get this merged?

py3.7 compat (no :=) is a good start

nat-goodspeed added a commit to nat-goodspeed/eventlet that referenced this pull request Oct 31, 2023
Credit to @hroncok and Victor Stinner <vstinner@python.org> for logic (eventlet#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.
@itamarst
Copy link
Contributor

Superseded by #817, which builds on this. Thank you for all your work!

@itamarst itamarst closed this Dec 15, 2023
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.

[Python 3.12] AttributeError: module 'ssl' has no attribute 'wrap_socket'
4 participants