Skip to content

Conversation

@milesgranger
Copy link
Contributor

@milesgranger milesgranger commented Mar 7, 2023

Extension to #7606

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

  • Support returning workers who timed out, via raise_for_timeout=False flag.
  • Return ErrorMessage when a worker fails to restart.
  • Allow restarting workers by name, address or both. Returning the same keys which were passed in.

Example

client.restart_workers(["tcp://127.0.0.1:3456", "Bob", "Kim"], raise_for_timeout=False)
{
    "tcp://127.0.0.1:3456": "OK",
    "Bob": "timed out",
    "Kim":  {
        "status": "error",
        ...
    }
}

@milesgranger milesgranger changed the title Fix restart workers failing restart tests Further improvements to Client.restart_workers Mar 7, 2023
@milesgranger milesgranger force-pushed the FIX-restart_workers-failing-restart-tests branch from b5d6dce to cc35e76 Compare March 7, 2023 14:09
@github-actions
Copy link
Contributor

github-actions bot commented Mar 7, 2023

Unit Test Results

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

       26 files  ±  0         26 suites  ±0   12h 5m 29s ⏱️ + 55m 17s
  3 497 tests +  6    3 391 ✔️ +  6     103 💤 ±0  3 ±0 
44 201 runs  +78  42 130 ✔️ +71  2 068 💤 +7  3 ±0 

For more details on these failures, see this check.

Results for commit a93a121. ± Comparison against base commit 689b603.

This pull request removes 1 and adds 7 tests. Note that renamed tests count towards both.
distributed.tests.test_client ‑ test_restart_workers_timeout
distributed.tests.test_client ‑ test_restart_workers_by_name[False]
distributed.tests.test_client ‑ test_restart_workers_by_name[True]
distributed.tests.test_client ‑ test_restart_workers_exception[False]
distributed.tests.test_client ‑ test_restart_workers_exception[True]
distributed.tests.test_client ‑ test_restart_workers_timeout[False]
distributed.tests.test_client ‑ test_restart_workers_timeout[True]
distributed.tests.test_system_monitor ‑ test_gil_contention

♻️ This comment has been updated with latest results.

Option to suppress TimeoutError and return the timed out status
@milesgranger milesgranger force-pushed the FIX-restart_workers-failing-restart-tests branch from cc35e76 to afd812f Compare March 9, 2023 08:24
@milesgranger milesgranger force-pushed the FIX-restart_workers-failing-restart-tests branch from afd812f to c4595ad Compare March 9, 2023 08:33
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.

Looks great, thank you!

@fjetter fjetter merged commit aec77af into dask:main Mar 9, 2023
@milesgranger milesgranger deleted the FIX-restart_workers-failing-restart-tests branch March 9, 2023 17:06
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.

2 participants