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

Drop Python 3.8 support #7840

Merged
merged 25 commits into from May 24, 2023
Merged

Drop Python 3.8 support #7840

merged 25 commits into from May 24, 2023

Conversation

graingert
Copy link
Member

@graingert graingert commented May 17, 2023

Closes dask/community#315

  • Tests added / passed
  • Passes pre-commit run --all-files

@github-actions
Copy link
Contributor

github-actions bot commented May 17, 2023

Unit Test Results

See test report for an extended history of previous test failures. This is useful for diagnosing flaky tests.

       20 files   -          6         20 suites   - 6   12h 18m 58s ⏱️ - 3h 3m 36s
  3 623 tests +         1    3 516 ✔️ +         4     105 💤 ±    0  2  - 3 
35 037 runs   - 10 769  33 318 ✔️  - 10 314  1 717 💤  - 451  2  - 4 

For more details on these failures, see this check.

Results for commit dc76082. ± Comparison against base commit 6f70648.

♻️ This comment has been updated with latest results.

@graingert graingert marked this pull request as ready for review May 17, 2023 14:33
@graingert graingert requested a review from fjetter as a code owner May 17, 2023 14:33
@graingert graingert requested review from hendrikmakait, fjetter and crusaderky and removed request for fjetter May 17, 2023 14:34
Copy link
Member

@hendrikmakait hendrikmakait left a comment

Choose a reason for hiding this comment

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

Thanks, @graingert! I just have a small question and two nits that don't block approval. From what I understand, we have lazy consensus in dask/community#315, so this should be good to merge.

distributed/comm/tcp.py Outdated Show resolved Hide resolved
distributed/compatibility.py Outdated Show resolved Hide resolved
distributed/compatibility.py Outdated Show resolved Hide resolved
Copy link
Member

@jrbourbeau jrbourbeau 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 handling all this @graingert. Overall this looks great -- just left a few small comments / questions

Also, it looks like we can drop some compatibility code here too

except asyncio.CancelledError:
# Task is being closed. When we drop Python < 3.8 we can drop
# this check (since CancelledError is not a subclass of
# Exception then).
break

@jrbourbeau jrbourbeau changed the title drop Python 3.8 support Drop Python 3.8 support May 19, 2023
@hendrikmakait
Copy link
Member

We should change https://github.com/graingert/distributed/blob/18e5421c31b6d8ceadcfa6cfecf1228072c67606/docs/source/develop.rst#L23 as well (conda env create --file continuous_integration/environment-3.8.yaml -> conda env create --file continuous_integration/environment-3.9.yaml)

distributed/cluster_dump.py Outdated Show resolved Hide resolved
distributed/cluster_dump.py Outdated Show resolved Hide resolved
distributed/collections.py Outdated Show resolved Hide resolved
distributed/comm/tcp.py Show resolved Hide resolved
distributed/compatibility.py Outdated Show resolved Hide resolved
distributed/shuffle/_comms.py Outdated Show resolved Hide resolved
distributed/shuffle/_scheduler_extension.py Outdated Show resolved Hide resolved
distributed/shuffle/_scheduler_extension.py Outdated Show resolved Hide resolved
distributed/tests/test_steal.py Outdated Show resolved Hide resolved
distributed/tests/test_steal.py Outdated Show resolved Hide resolved
graingert and others added 2 commits May 24, 2023 13:30
Co-authored-by: Hendrik Makait <hendrik@makait.com>
Copy link
Member

@jrbourbeau jrbourbeau left a comment

Choose a reason for hiding this comment

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

Thanks @graingert. This looks good to go

EDIT: Waiting for CI to finish

@jrbourbeau
Copy link
Member

Actually, distributed/comm/tests/test_tcp.py::test_getaddrinfo_invalid_af, which is being added here, is failing in CI
https://github.com/dask/distributed/actions/runs/5069958811/jobs/9104314707?pr=7840
https://github.com/dask/distributed/actions/runs/5069958811/jobs/9104313352?pr=7840

@graingert
Copy link
Member Author

@jrbourbeau just pushed a fix - I skip it on windows

@jrbourbeau
Copy link
Member

Thanks! Though we got a failure on macOS too https://github.com/dask/distributed/actions/runs/5069958811/jobs/9104314707?pr=7840

@graingert
Copy link
Member Author

@jrbourbeau weird, I pushed a skip for that too

@graingert graingert requested a review from jrbourbeau May 24, 2023 16:40
@jrbourbeau
Copy link
Member

The gpuCI failures here look unrelated to this PR but I've not seen them before (cc @charlesbluca). Checking over in #7857.

Will merge this PR and dask/dask#10295 after we've confirmed they're not related

@jrbourbeau
Copy link
Member

Okay, seeing similar gpuCI failures over in #7857, so they're definitely unrelated to the changes here

@jrbourbeau jrbourbeau merged commit 28459a5 into dask:main May 24, 2023
23 of 28 checks passed
@jrbourbeau
Copy link
Member

Thanks @graingert @crusaderky @hendrikmakait

@graingert graingert deleted the drop-py-38 branch May 24, 2023 17:52
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.

Drop support for python 3.8
6 participants