-
Notifications
You must be signed in to change notification settings - Fork 20
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
Add support for PyPy 3.10 #113
Conversation
f6e8faf
to
3f40b6c
Compare
Oh my. Mostly I'm just impressed if it works. Good call on only changing it for non-CPython implementations, in case someone is already using isinstance to check if they've got our SSLContext. I'll wait for final review until the tests pass. |
3f40b6c
to
03c47c5
Compare
Looks like a bug in PyPy's |
Okay @davisagli tests are passing so ready for review. If you'd like to take a stab at PyPy support without the horrible hacks please do, I would like to not have them 😓 |
# Dirty hack to get around isinstance() checks | ||
# for ssl.SSLContext instances in aiohttp/trustme | ||
# when using non-CPython implementations. | ||
return _truststore_SSLContext_dunder_class or SSLContext |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you point me to a reproducer for why this was needed for PyPy? Is there something PyPy should be doing to make isinstance
work?
certfile=mkcert_certs.cert_file, | ||
keyfile=mkcert_certs.key_file, | ||
certfile=str(mkcert_certs.cert_file), | ||
keyfile=str(mkcert_certs.key_file), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry about that, is there a simple reproducer for this? PyPy passes the CPython ssl stdlib tests, maybe there is a case missing there?
Thanks for getting PyPy to work. I am not familiar with the project, but am a PyPy core dev. Please ping me (mattip) if you come across any other PyPy problems. |
I opened https://foss.heptapod.net/pypy/pypy/-/issues/4002 to continue the discussion. |
@mattip Thanks Matti! We'll keep you in mind if we run into future problems :) |
This is very much a hack, I don't like it, but attempting to add support to PyPy while maintaining the
SSLContext
subclassing is a huge pain from my initial attempt. The__class__
hack maintains compatibility for aiohttp and trustme.Closes #112