dask-kubernetes icon indicating copy to clipboard operation
dask-kubernetes copied to clipboard

Remove worker-template from dask-kubernetes config

Open jhamman opened this issue 6 years ago • 3 comments

I'd like to propose dask-kubernetes deprecate the current usage of worker-template and worker-template-path from the config file. I suggest this for a few reasons:

  • Specification of the full cli executable sequence is implemented as a template in other cluster managers. Typically (e.g. dask-jobqueue, dask-yarn), keys like cores, processes, and memory are provided to the Cluster constructor.
  • It is easy to end up with templates that have misaligned specifications (i.e. a user may edit dask's memory-limit without editing the resource requirements.

Here's an example of the default dask-kubernetes setup:

https://github.com/dask/dask-kubernetes/blob/e17accc9bca567161338991e9a01e24d9dd220fd/dask_kubernetes/kubernetes.yaml#L11-L38

Ideally, we can switch to something more concise:

kubernetes:
  worker:
    cores: 2
    processes: 1
    memory: 6G
    image: daskdev/dask:latest

Where the template and pod spec can be created on the fly.

jhamman avatar Jul 10 '19 22:07 jhamman

From my perspective the challenge here is that it's hard to wrap up all of the options in a comprehensive way. Someone will come by with a more custom configuation that they'd like to use.

I think it'd be fine to have an alternate approach to construct a pod spec template, (this already exists in a utility function I think) but I think that there is a lot of value to being able to pass through a template.

mrocklin avatar Jul 10 '19 22:07 mrocklin

I agree. I think it's important to be able to pass a full spec through for maximum flexibility and customization.

However I'm not opposed to also having a more convenient way to create specs like the one Joe proposes. This could even be the default in the config, but should be overridden by a full pod spec.

jacobtomlinson avatar Jul 11 '19 08:07 jacobtomlinson

Okay, I think I agree with you both now. As @mrocklin says, there is already a utility function to do this so we just need to pass config options through.

jhamman avatar Jul 11 '19 14:07 jhamman

The classic KubeCluster was removed in https://github.com/dask/dask-kubernetes/pull/890. All users will need to migrate to the Dask Operator. Closing.

jacobtomlinson avatar Apr 30 '24 15:04 jacobtomlinson