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

Support for Python 3.10 #9930

Merged
merged 40 commits into from
Apr 26, 2022
Merged

Support for Python 3.10 #9930

merged 40 commits into from
Apr 26, 2022

Conversation

altendky
Copy link
Contributor

@altendky altendky commented Jan 24, 2022

This is just a duplicate of #9765 to start. Feel free to create PRs against this one for 3.10 fixes that can't go into main. If they can go into main then submit them against main. After they get merged you can close and reopen this PR (just buttons at the bottom) and it will trigger a rebuild including merging with the latest main so we can see the results of your fixes.

If you know some explicit categories that need fixed or discussed, please add them to the list of reasons this is still draft.

Notable points:

  • This drops testing of macOS with Python 3.7
  • Wheel availability is significantly incomplete for macOS Arm with Python 3.8.
  • Wheel availability is not being checked for macOS Arm or Intel for Python 3.7.
  • Python versions used for installers is not being changed.

Draft for:

Maybe followup:

  • Use the same version for all installers? 3.9? 3.10?

@altendky altendky mentioned this pull request Jan 24, 2022
arvidn
arvidn previously approved these changes Jan 24, 2022
Copy link
Contributor

@arvidn arvidn left a comment

Choose a reason for hiding this comment

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

is it OK to drop python 3.8 from CI?

setup.py Outdated
"blspy==1.0.9", # Signature library
"chiavdf==1.0.4", # timelord and vdf verification
"chiabip158==1.1", # bip158-style wallet filters
"chiapos==1.0.8", # proof of space
Copy link
Contributor

Choose a reason for hiding this comment

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

bumping these dependencies seem unrelated to supporting python 3.10. are there other python-3.10 fixes in those dependencies?

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 think these are updates for these to provide wheels. I'm not sure if there are other changes (nor entirely sure about that). They are being updated in #9808 separately anyways and presumably that goes in first.

@altendky
Copy link
Contributor Author

I'm guessing Gene was dropping 3.8 to keep just two CI runs for macOS. You get lower runner count and they are slower. But, I don't know my way around the macOS community and what is available and actually used etc. For example, why did we already not have 3.7 on macOS?

@hoffmang9
Copy link
Member

I was indeed trying to keep the total Mac runners limited. All of our sub repos had to be version bumped to build 3.10 wheels.

@hoffmang9
Copy link
Member

The last two MacOS releases are 3.8+ and Silicon requires 3.9+ (though Silicon was back ported into 3.8 later). Brew installs 3.9 by default right now. As such 3.7 really isn't a factor on any mostly up to date Mac.

@altendky altendky self-assigned this Feb 5, 2022
@altendky
Copy link
Contributor Author

We'll want to hit up the loop parameter warnings being ignored at https://github.com/Chia-Network/chia-blockchain/pull/10240/files#diff-338af70c6877cd271f4d5d1c05bbfc692ad15c5dbff0f7ea3c0e578f3f59c850R19. The loop argument is deprecated since Python 3.8, and scheduled for removal in Python 3.10

@altendky
Copy link
Contributor Author

Both the client and server SSL context creation use purpose=ssl.Purpose.SERVER_AUTH with the private function ssl._create_unverified_context(). Changing the server to use CLIENT_AUTH in 8106e4f seems to have fixed the connection issues showing up in 3.10. There were a few places where we were also using our server context creation helper where it should have been the client helper.

def ssl_context_for_server(
ca_cert: Path,
ca_key: Path,
private_cert_path: Path,
private_key_path: Path,
*,
check_permissions: bool = True,
log: Optional[logging.Logger] = None,
) -> ssl.SSLContext:
if check_permissions:
verify_ssl_certs_and_keys([ca_cert, private_cert_path], [ca_key, private_key_path], log)
ssl_context = ssl._create_unverified_context(purpose=ssl.Purpose.SERVER_AUTH, cafile=str(ca_cert))

def ssl_context_for_client(
ca_cert: Path,
ca_key: Path,
private_cert_path: Path,
private_key_path: Path,
*,
check_permissions: bool = True,
log: Optional[logging.Logger] = None,
) -> ssl.SSLContext:
if check_permissions:
verify_ssl_certs_and_keys([ca_cert, private_cert_path], [ca_key, private_key_path], log)
ssl_context = ssl._create_unverified_context(purpose=ssl.Purpose.SERVER_AUTH, cafile=str(ca_cert))

Here is the latest 3.9 and 3.10 source for the private function ssl._create_unverified_context() that we are using as well as a shortcut to a diff of the two.

https://github.com/python/cpython/blob/v3.9.11/Lib/ssl.py#L758-L802
https://github.com/python/cpython/blob/v3.10.4/Lib/ssl.py#L778-L830
Diff: https://www.diffchecker.com/Xa1EaTvY

I'm guessing there was a copy/paste error in 3d27a7d#diff-e0bb43e77afdc23f85aefba41e3d8fda1554c6402c8e08536508ed6246d8068f that introduced this. Since we were passing in cafile= the purpose had no effect. Now it does in 3.10.

mariano54 and others added 3 commits April 20, 2022 14:09
* 2.7 seconds -> 0.45 seconds

* Merge

* Work on create_plots refactor

* Try to fix tests

* Try to fix tests

* Use new functions

* Fix block_tools by adding dir

* Extra argument

* Try to fix cyclic import

* isort

* Drop warning

* Some cleanups around `exclude_final_dir` and directory adding

* Cleanup `min_mainnet_k_size` checks

* Drop unrelated changes

* Fixes after rebase

* Fix cyclic import

* Update tests/block_tools.py

Co-authored-by: dustinface <[email protected]>

* Update tests/block_tools.py

Co-authored-by: dustinface <[email protected]>

Co-authored-by: xdustinface <[email protected]>
Co-authored-by: dustinface <[email protected]>
hoffmang9
hoffmang9 previously approved these changes Apr 20, 2022
Copy link
Member

@hoffmang9 hoffmang9 left a comment

Choose a reason for hiding this comment

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

lgtm pending CI

@cmmarslender
Copy link
Contributor

cmmarslender commented Apr 23, 2022

Trying to install on 22.04, I'm getting the following error, even though I had pre-installed python 3.10:

E: Unable to locate package python3.9-venv
E: Couldn't find any package by glob 'python3.9-venv'
E: Couldn't find any package by regex 'python3.9-venv'

Seems like apt is attempting to install python 3.9 even though 3.10 is supported, in the 2110+ block. I think we need to add an additional 2204 check

@altendky altendky marked this pull request as ready for review April 26, 2022 18:48
Copy link
Contributor

@cmmarslender cmmarslender left a comment

Choose a reason for hiding this comment

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

lgtm. Tested all of the following with Python 3.10, and everything worked fine with the install scripts

Linux arm64
Linux amd64
Mac arm64
Mac intel
Windows

@wjblanke wjblanke merged commit 7892148 into main Apr 26, 2022
@wjblanke wjblanke deleted the python-310-altendky branch April 26, 2022 19:37
wallentx pushed a commit that referenced this pull request Apr 28, 2022
* Support for Python 3.10

* Update install.sh to block Python 3.11

* websockets to 10.1

* Update workflows for Python 3.10

* single quote 3.10

* Enable fedora:35 (py3.10) installer script testing

* rebuild workflows

* fixup test-install-scripts.yml

* add ignore for distutils deprecation in tests for now

* asyncio.get_event_loop().run_until_complete() -> asyncio.run()

* aiohttp==3.8.1 for python 3.10 support

* use ssl.Purpose.CLIENT_AUTH for ssl_context_for_server()

* rebuild workflows

* use ssl_context_for_client() in BlockTools.get_daemon_ssl_context()

* create a client context for the RpcServer to connect to the daemon

* go back to asyncio.get_event_loop().run_until_complete() for now to recover 3.7

* ignore:There is no current event loop:DeprecationWarning

* Ms.plot load perf2 (#10978)

* 2.7 seconds -> 0.45 seconds

* Merge

* Work on create_plots refactor

* Try to fix tests

* Try to fix tests

* Use new functions

* Fix block_tools by adding dir

* Extra argument

* Try to fix cyclic import

* isort

* Drop warning

* Some cleanups around `exclude_final_dir` and directory adding

* Cleanup `min_mainnet_k_size` checks

* Drop unrelated changes

* Fixes after rebase

* Fix cyclic import

* Update tests/block_tools.py

Co-authored-by: dustinface <[email protected]>

* Update tests/block_tools.py

Co-authored-by: dustinface <[email protected]>

Co-authored-by: xdustinface <[email protected]>
Co-authored-by: dustinface <[email protected]>

* remove 3.10 avoidance step from debian:bookworm installer testing

* add 3.10 to wheel availability check workflow

* add 3.10 to Install.ps1 supported Python versions for Windows

* add jammy jellyfish to the install script test matrix

* correct ubuntu:jammy job name

* add 22.04 with Python 3.10 to install.sh

Co-authored-by: Gene Hoffman <[email protected]>
Co-authored-by: Yostra <[email protected]>
Co-authored-by: Mariano Sorgente <[email protected]>
Co-authored-by: xdustinface <[email protected]>
Co-authored-by: dustinface <[email protected]>
justinengland pushed a commit that referenced this pull request May 12, 2022
* Support for Python 3.10

* Update install.sh to block Python 3.11

* websockets to 10.1

* Update workflows for Python 3.10

* single quote 3.10

* Enable fedora:35 (py3.10) installer script testing

* rebuild workflows

* fixup test-install-scripts.yml

* add ignore for distutils deprecation in tests for now

* asyncio.get_event_loop().run_until_complete() -> asyncio.run()

* aiohttp==3.8.1 for python 3.10 support

* use ssl.Purpose.CLIENT_AUTH for ssl_context_for_server()

* rebuild workflows

* use ssl_context_for_client() in BlockTools.get_daemon_ssl_context()

* create a client context for the RpcServer to connect to the daemon

* go back to asyncio.get_event_loop().run_until_complete() for now to recover 3.7

* ignore:There is no current event loop:DeprecationWarning

* Ms.plot load perf2 (#10978)

* 2.7 seconds -> 0.45 seconds

* Merge

* Work on create_plots refactor

* Try to fix tests

* Try to fix tests

* Use new functions

* Fix block_tools by adding dir

* Extra argument

* Try to fix cyclic import

* isort

* Drop warning

* Some cleanups around `exclude_final_dir` and directory adding

* Cleanup `min_mainnet_k_size` checks

* Drop unrelated changes

* Fixes after rebase

* Fix cyclic import

* Update tests/block_tools.py

Co-authored-by: dustinface <[email protected]>

* Update tests/block_tools.py

Co-authored-by: dustinface <[email protected]>

Co-authored-by: xdustinface <[email protected]>
Co-authored-by: dustinface <[email protected]>

* remove 3.10 avoidance step from debian:bookworm installer testing

* add 3.10 to wheel availability check workflow

* add 3.10 to Install.ps1 supported Python versions for Windows

* add jammy jellyfish to the install script test matrix

* correct ubuntu:jammy job name

* add 22.04 with Python 3.10 to install.sh

Co-authored-by: Gene Hoffman <[email protected]>
Co-authored-by: Yostra <[email protected]>
Co-authored-by: Mariano Sorgente <[email protected]>
Co-authored-by: xdustinface <[email protected]>
Co-authored-by: dustinface <[email protected]>
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.

7 participants