Skip to content

Clean up deprecations in CLI#9240

Merged
crusaderky merged 1 commit into
dask:mainfrom
crusaderky:deprecated-cli
May 18, 2026
Merged

Clean up deprecations in CLI#9240
crusaderky merged 1 commit into
dask:mainfrom
crusaderky:deprecated-cli

Conversation

@crusaderky
Copy link
Copy Markdown
Collaborator

@crusaderky crusaderky commented Apr 30, 2026

No description provided.

@crusaderky crusaderky added the deprecation Something is being removed label Apr 30, 2026
@crusaderky crusaderky marked this pull request as draft April 30, 2026 11:19
@crusaderky crusaderky marked this pull request as ready for review April 30, 2026 13:07
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 30, 2026

Unit Test Results

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

    30 files  ± 0      30 suites  ±0   11h 1m 5s ⏱️ - 1m 52s
 4 112 tests  -  3   4 005 ✅  -  2    104 💤 ±0  3 ❌  - 1 
59 713 runs   - 42  57 274 ✅  - 37  2 436 💤  - 4  3 ❌  - 1 

For more details on these failures, see this check.

Results for commit 0f70b55. ± Comparison against base commit d9c0010.

This pull request removes 3 tests. 
distributed.cli.tests.test_dask_scheduler ‑ test_single_executable_deprecated
distributed.cli.tests.test_dask_worker ‑ test_single_executable_deprecated
distributed.cli.tests.test_dask_worker ‑ test_single_executable_works

@crusaderky crusaderky mentioned this pull request May 1, 2026
Open
@crusaderky crusaderky changed the title Deprecations in CLI Clean up deprecations in CLI May 1, 2026
@crusaderky crusaderky mentioned this pull request May 1, 2026
Closed
@crusaderky crusaderky merged commit 81b9308 into dask:main May 18, 2026
34 of 39 checks passed
@crusaderky crusaderky deleted the deprecated-cli branch May 18, 2026 18:45
@rjzamora rjzamora mentioned this pull request May 18, 2026
Closed
rjzamora
rjzamora reviewed May 18, 2026
View reviewed changes
Comment thread distributed/cli/utils.py
CLICK_VERSION = tuple(map(int, importlib.metadata.version("click").split(".")[:2]))


def install_signal_handlers(loop=None, cleanup=None):
Copy link
Copy Markdown
Member

@rjzamora rjzamora May 18, 2026

Choose a reason for hiding this comment

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

This is currently used in Dask-CUDA. Was this API deprecated? cc @TomAugspurger @pentschev for viz

Copy link
Copy Markdown
Member

@TomAugspurger TomAugspurger May 18, 2026

Choose a reason for hiding this comment

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

Yeah. Dunno how we missed it :/

Copy link
Copy Markdown
Member

@rjzamora rjzamora May 18, 2026

Choose a reason for hiding this comment

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

Maybe rapidsai/dask-cuda#1651 takes care of this?

Copy link
Copy Markdown
Member

@pentschev pentschev May 19, 2026

Choose a reason for hiding this comment

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

Thanks for catching this, Rick. I think you're right, we should be more aligned with how Distributed is currently handling signals and thus rapidsai/dask-cuda#1651 looks like the proper solution.

Copy link
Copy Markdown
Collaborator Author

@crusaderky crusaderky May 19, 2026

Choose a reason for hiding this comment

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

Is it acceptable to have this PR in the next distributed release, or do you prefer reverting it and doing it again in a few months?

Copy link
Copy Markdown
Member

@pentschev pentschev May 19, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

I'll let @TomAugspurger answer that definitively, but AFAIK the latest Dask release (2026.3.0) will be used for the upcoming RAPIDS 25.06 release and thus we do not need to make changes for this release. For the next RAPIDS release expected only for August, it should be ok for us to use the changes from rapidsai/dask-cuda#1651 that drop the use of install_signal_handlers unless we find out any issues with that within the next ~2 months.

Copy link
Copy Markdown
Member

@TomAugspurger TomAugspurger May 19, 2026

Choose a reason for hiding this comment

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

Yeah we should be good.

@rjzamora rjzamora mentioned this pull request May 18, 2026
Merged
rapids-bot Bot pushed a commit to rapidsai/dask-cuda that referenced this pull request May 19, 2026
@rjzamora
Drop usage of install_signal_handlers (#1651)
c0f9a3f 
The `install_signal_handlers` utility was unused in distributed, and was removed in dask/distributed#9240

This PR simply follows the pattern used in dask/distributed#6205 to migrate to `wait_for_signals`.

Authors:
  - Richard (Rick) Zamora (https://github.com/rjzamora)

Approvers:
  - Peter Andreas Entschev (https://github.com/pentschev)

URL: #1651
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@TomAugspurger TomAugspurger TomAugspurger left review comments

@jacobtomlinson jacobtomlinson Awaiting requested review from jacobtomlinson jacobtomlinson is a code owner

@fjetter fjetter Awaiting requested review from fjetter fjetter is a code owner

+2 more reviewers

@pentschev pentschev pentschev left review comments

@rjzamora rjzamora rjzamora left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

deprecation Something is being removed

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@crusaderky @TomAugspurger @pentschev @rjzamora