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

Make all LocalCluster tests async #1087

Open
pentschev opened this issue Jan 18, 2023 · 1 comment
Open

Make all LocalCluster tests async #1087

pentschev opened this issue Jan 18, 2023 · 1 comment

Comments

@pentschev
Copy link
Member

We should make all LocalCluster tests async as it's generally simpler to test (e.g., to have timeouts) and generally more in line with real use-cases. Parts of that were addressed in #1084 and #1086 .

Some of the tests are a bit challenging though, for instance attempting to move

with LocalCluster(
protocol="tcp",
dashboard_address=None,
n_workers=4,
threads_per_worker=5,
processes=True,
) as cluster:
to async had processes failing to connect, I'm not sure why.

Also most of the tests in https://github.com/rapidsai/dask-cuda/blob/branch-23.02/dask_cuda/tests/test_explicit_comms.py are executed in a child process but a quick glance makes me think that's not necessary. Could you remind me why that is that case @madsbk ?

@madsbk
Copy link
Member

madsbk commented Jan 20, 2023

Also most of the tests in https://github.com/rapidsai/dask-cuda/blob/branch-23.02/dask_cuda/tests/test_explicit_comms.py are executed in a child process but a quick glance makes me think that's not necessary. Could you remind me why that is that case @madsbk ?

We did it to make the tests handle UCX segfaulting, something that happen a lot in the early days :)
But I think you are right, we hopefully doesn't need it anymore :)

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

No branches or pull requests

2 participants