Is this the minimally-surprising behaviour? My feeling is that the narrower specification (shuffle="tasks" as a keyword argument) should take precedence over the configuration variable.

Before this PR, there was one way to enable an explicit-comms shuffle: set dask.config["explicit-comms"] to True, which changed the meaning of shuffle="tasks" to be shuffle="explicit-comms".

Now that we are hooking in one level higher up, I think we can (unless we want to maintain backwards-compat) switch to a setup where the only way to request the explicit-comms shuffle is via dask.config["shuffle"] = "explicit-comms" or shuffle="explicit-comms", and remove the dask.config["explicit-comms"] option (unless we anticipate needing it for other things as well).

Concretely, I'm suggesting:

use_explicit_comms = kwargs.get("shuffle", dask.config.get("shuffle", None)) == "explicit-comms"
if use_explicit_comms:
    try:
        import distributed.worker
        distributed.worker.get_client()
    except (ImportError, ValueError):
        warning("Requested explicit-comms shuffle, but no distributed client available, using task-based shuffle")
        # slight action-at-a-distance here because this affects the call to `func` below in the (implicit) "not use_explicit_comms" branch.
        kwargs["shuffle"] = "tasks"
    else:
        ... # do the explicit-comms thing
return func(*args, **kwargs)

Now that we are hooking in one level higher up, I think we can (unless we want to maintain backwards-compat) switch to a setup where...

Yes. I agree that we should move away from the "explicit-comms" config, but I was indeed trying to avoid a "breaking" change. With that said, I have no evidence that anyone is actually using the "explicit-comms" config in the wild. So, perhaps I was being too conservative.

Even if people are using the "explicit-comms" config, we should probably add a deprecation warning here, and tell them to use the "shuffle" option and config instead. What do you think about allowing the "explicit-comms"/"tasks" for now, but with a FutureWarning?

Yes, let's do that (and try and get it in for 22.10 so we can immediately remove :P)

Agree, this is a good solution.