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

Python 3.10 #5952

Merged
merged 14 commits into from Mar 30, 2022
Merged

Python 3.10 #5952

merged 14 commits into from Mar 30, 2022

Conversation

graingert
Copy link
Member

@graingert graingert commented Mar 16, 2022

combines #5778 and #5353

@github-actions
Copy link
Contributor

github-actions bot commented Mar 16, 2022

Unit Test Results

       18 files  +       18         18 suites  +18   9h 25m 41s ⏱️ + 9h 25m 41s
  2 699 tests +  2 699    2 616 ✔️ +  2 616       81 💤 +     81  2 +2 
24 127 runs  +24 127  22 839 ✔️ +22 839  1 286 💤 +1 286  2 +2 

For more details on these failures, see this check.

Results for commit b0308ac. ± Comparison against base commit b872d45.

♻️ This comment has been updated with latest results.

Copy link
Member

@jakirkham jakirkham left a comment

Choose a reason for hiding this comment

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

Thanks for working on this Thomas! 😄

Had a few questions below 🙂

.github/workflows/tests.yaml Show resolved Hide resolved
distributed/tests/test_client.py Outdated Show resolved Hide resolved
distributed/tests/test_client.py Outdated Show resolved Hide resolved
distributed/tests/test_client.py Show resolved Hide resolved
distributed/profile.py Outdated Show resolved Hide resolved
@graingert graingert force-pushed the python-3.10 branch 2 times, most recently from cb7548b to b3a5b38 Compare March 18, 2022 14:35
@graingert graingert force-pushed the python-3.10 branch 5 times, most recently from 2423e95 to 090f5ee Compare March 22, 2022 14:20
@graingert graingert marked this pull request as ready for review March 22, 2022 16:20
jrbourbeau and others added 10 commits March 23, 2022 13:49
In 3.10, the _internal_ `_loop` attribute of several asyncio classes has
been removed.
by interacting with the ac.condition we bind it to the loop on py3.10
ERROR    asyncio:base_events.py:1729 Exception in callback BaseAsyncIOLoop._handle_events(18, 1)
handle: <Handle BaseAsyncIOLoop._handle_events(18, 1)>
Traceback (most recent call last):
  File "/home/graingert/miniconda3/envs/dask-distributed-310/lib/python3.10/asyncio/events.py", line 80, in _run
    self._context.run(self._callback, *self._args)
  File "/home/graingert/miniconda3/envs/dask-distributed-310/lib/python3.10/site-packages/tornado/platform/asyncio.py", line 189, in _handle_events
    handler_func(fileobj, events)
  File "/home/graingert/miniconda3/envs/dask-distributed-310/lib/python3.10/site-packages/tornado/netutil.py", line 276, in accept_handler
    callback(connection, address)
  File "/home/graingert/miniconda3/envs/dask-distributed-310/lib/python3.10/site-packages/tornado/tcpserver.py", line 288, in _handle_connection
    connection = ssl_wrap_socket(
  File "/home/graingert/miniconda3/envs/dask-distributed-310/lib/python3.10/site-packages/tornado/netutil.py", line 615, in ssl_wrap_socket
    return context.wrap_socket(socket, server_hostname=server_hostname, **kwargs)
  File "/home/graingert/miniconda3/envs/dask-distributed-310/lib/python3.10/ssl.py", line 512, in wrap_socket
    return self.sslsocket_class._create(
  File "/home/graingert/miniconda3/envs/dask-distributed-310/lib/python3.10/ssl.py", line 1061, in _create
    self._sslobj = self._context._wrap_socket(
ssl.SSLError: Cannot create a server socket with a PROTOCOL_TLS_CLIENT context (_ssl.c:794)
distributed/utils_test.py Show resolved Hide resolved
@jcrist jcrist mentioned this pull request Mar 24, 2022
3 tasks
@jakirkham
Copy link
Member

Seeing 2 test failures. One with test_restart on Linux Python 3.9 and one with test_restart_sync_no_center on Windows Python 3.9.

The former seems to be due to a missing file and the latter due to contention around a file.

Not seeing other issues around these tests on the issue tracker (so don't know if they are known to be flaky). Though wouldn't be surprised if these are due to sporadic test failures. Did see PR ( #5994 ) modernizing test_restart_sync_no_center (maybe something similar is needed for test_restart?).

Idk if these should be blockers, but ok going ahead without them (if others agree) to get Python 3.10 included.

@mrocklin
Copy link
Member

test_restart_sync_no_center can be nuked. That's happening at #5994 . Taking a look at the test_restart report now.

@mrocklin
Copy link
Member

For test_restart I think it's not related to this PR. That being said, I think that folks in this PR might be able to help diagnose what's going on. I think that that failing test (and others) might boil down to this question: #6001

@graingert graingert requested a review from fjetter March 25, 2022 12:07
Copy link
Member

@sjperkins sjperkins left a comment

Choose a reason for hiding this comment

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

Some observations:

  • Pytorch is no longer tested on Python 3.10.
  • The default SSL Context purpose has changed from SERVER_AUTH to CLIENT_AUTH
  • Do the docs build OK?

continuous_integration/environment-3.10.yaml Show resolved Hide resolved
distributed/tests/test_profile.py Show resolved Hide resolved
distributed/node.py Show resolved Hide resolved
@jakirkham
Copy link
Member

For test_restart I think it's not related to this PR.

Yeah agree it is probably unrelated.

That being said, I think that folks in this PR might be able to help diagnose what's going on. I think that that failing test (and others) might boil down to this question: #6001

Maybe just wanted to avoid holding up Python 3.10 addition over this test failure, but if others want to debug and fix it that is fine as well

@mrocklin
Copy link
Member

mrocklin commented Mar 25, 2022 via email

@jakirkham
Copy link
Member

@fjetter did you have more thoughts here or is this good from your perspective? 🙂

@fjetter
Copy link
Member

fjetter commented Mar 28, 2022

The environment.yaml still includes too many dependencies. All the "only tested here" dependencies should be removed since they are tested in the py3.9 build

Copy link
Member Author

@graingert graingert left a comment

Choose a reason for hiding this comment

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

remove "# Only tested here" deps

continuous_integration/environment-3.10.yaml Outdated Show resolved Hide resolved
continuous_integration/environment-3.10.yaml Outdated Show resolved Hide resolved
continuous_integration/environment-3.10.yaml Outdated Show resolved Hide resolved
Copy link
Member

@fjetter fjetter left a comment

Choose a reason for hiding this comment

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

I'm also wondering why we are testing

  • paramiko
  • scikit-learn/scipy
  • prometheus
  • netcdf4
  • h5py
  • ...

in this build but we can clean up these build files in a follow up PR.

continuous_integration/environment-3.10.yaml Outdated Show resolved Hide resolved
continuous_integration/environment-3.10.yaml Outdated Show resolved Hide resolved
continuous_integration/environment-3.10.yaml Outdated Show resolved Hide resolved
continuous_integration/environment-3.10.yaml Outdated Show resolved Hide resolved
continuous_integration/environment-3.10.yaml Outdated Show resolved Hide resolved
Co-authored-by: Florian Jetter <fjetter@users.noreply.github.com>
@fjetter fjetter merged commit 4c4df91 into dask:main Mar 30, 2022
@graingert graingert deleted the python-3.10 branch March 30, 2022 08:18
mrocklin pushed a commit to mrocklin/distributed that referenced this pull request Mar 31, 2022
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.

None yet

8 participants