Can one of the admins verify this patch?

Admins can comment ok to test to allow this one PR to run or add to allowlist to allow all future PRs from the same author to run.

Unit Test Results

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

       21 files  ±    0         21 suites  ±0   9h 57m 38s ⏱️ - 1h 29m 15s
  3 784 tests +    7    3 669 ✔️ ±    0     107 💤  -   1    8 ❌ +  8 
35 540 runs   - 962  33 763 ✔️  - 927  1 745 💤  - 67  32 ❌ +32 

For more details on these failures, see this check.

Results for commit 1afd997. ± Comparison against base commit acb2809.

distributed.tests.test_client ‑ test_short_tracebacks

distributed.shuffle.tests.test_shuffle ‑ test_basic_integration_local_cluster[False]
distributed.shuffle.tests.test_shuffle ‑ test_basic_integration_local_cluster[True]
distributed.tests.test_client ‑ test_get_scheduler
distributed.tests.test_client ‑ test_short_tracebacks[gather]
distributed.tests.test_client ‑ test_short_tracebacks[result]
distributed.tests.test_client ‑ test_short_tracebacks_async[await]
distributed.tests.test_client ‑ test_short_tracebacks_async[gather]
distributed.tests.test_client ‑ test_short_tracebacks_async[result]

♻️ This comment has been updated with latest results.

I noticed some linting issues https://github.com/dask/distributed/actions/runs/5864563511/job/15899846304?pr=8106
I recommend install pre-commit hooks to avoid this and autoformat your code before committing. see also https://docs.dask.org/en/stable/develop.html#code-formatting

There are a couple of related tests failing. Let me know if you need help figuring out what's the correct behavior

Hi Florian, I've fixed the pre commit black issue. May I ask you to clarify what exactly do you think is right behavior. According to the way I understand, get_client method should try to get client.current() first and if that fails, _get_client() should return default first and if this fails, it will try to return worker client.

Hello, Florian. I'm still waiting for your response on the test I'm failing and what you believe is wrong with it. I'd also like to know what I might be able to do to fix it. Thank you!

Hi @minhnguyenxuan60, I'll take this over from @fjetter, who's unavailable this week. The best way to check if your behavior matches the desired behavior would be to add a test that mimics the reproducer in #7965.

According to the way I understand, get_client method should try to get client.current() first and if that fails, _get_client() should return default first and if this fails, it will try to return worker client.

From what I understand from #7965, get_client should return Client.current() if available. If not, return default_client() if this is available, otherwise, follow the previously existing fallback logic of returning a worker client or some newly client for the given address.

This breaks with the current behavior that focused on returning the worker client, so I assume it will break some tests. We can look into this once the reproducer from #7965 works as expected.