Prepare for Dask 2.3.0 release

[jrbourbeau]

This is just to check that things work with the current master branch of dask/dask and dask/distributed prior to releasing 2.3.0 upstream.

I think the CI already runs against upstream master branches, so I've included a trivial change to the README to trigger CI

[jrbourbeau]

It appears there's some interesting caching-related behavior happening with the CI. The normal build triggered when I push a commit times out (ref https://circleci.com/gh/dask/dask-kubernetes/394) while if I rebuild the job without a cache the CI completes (ref https://circleci.com/gh/dask/dask-kubernetes/395). Will look into this more tomorrow

[TomAugspurger]

I wonder if your cached builds are ending up with different versions of dask / distributed on the workers vs. the scheduler. I think the scheduler image is built against master, while the workers are from the cached conda env.

[mrocklin]

The errors in the most recent CI build are definitely due to a mismatch in versions between scheduler and worker. We added the extra attribute recently.

[mrocklin]

This failure seems possibly more concerning

Traceback (most recent call last):
  File "/home/circleci/miniconda/envs/dask-kubernetes-test/lib/python3.7/site-packages/tornado/ioloop.py", line 743, in _run_callback
    ret = callback()
  File "/home/circleci/miniconda/envs/dask-kubernetes-test/lib/python3.7/site-packages/tornado/ioloop.py", line 767, in _discard_future_result
    future.result()
  File "/home/circleci/miniconda/envs/dask-kubernetes-test/lib/python3.7/site-packages/distributed/deploy/adaptive_core.py", line 182, in adapt
    recommendations = await self.recommendations(target)
  File "/home/circleci/miniconda/envs/dask-kubernetes-test/lib/python3.7/site-packages/distributed/deploy/adaptive_core.py", line 133, in recommendations
    plan = self.plan
  File "/home/circleci/miniconda/envs/dask-kubernetes-test/lib/python3.7/site-packages/distributed/deploy/adaptive.py", line 104, in plan
    return set(self.cluster.workers)
TypeError: unhashable type: 'V1Pod'

We recently changed adaptive to track sets of launched workers (plan/requested) and a set of arrived workers (observed). We implement those for most clusters here:

https://github.com/dask/distributed/blob/6302175056742d101b8af52336bc6bbf4227da35/distributed/deploy/adaptive.py#L99-L112

Currently these are expected to get a collection that is keyed by worker name in order to handle comparisons. One solution would be to specify the worker name as we launch a pod, and keep that name->pod mapping around.

[mrocklin]

As an alternative, we might also ...

1. change requirements in dask-kubernetes to avoid the newest release for now
2. bring back the old Adaptive code into this library

[mrocklin]

In case we want to go with option 2, the PR that made this change is here: dask/distributed#2904

We would probably walk one commit back from there, copy the adaptive.py file into here, and then overwrite the adapt method to use that class rather than the one in distributed (there is an Adaptive= keyword we can use).

[jrbourbeau]

@mrocklin thanks for all the feedback. Do you have a preference for any of the three options? You're more familiar with the current development practices of dask-kubernetes.

For context, is option 2 intended to be a workaround until a more long term fix for plan/requested/observed is added?

[mrocklin]

They're all workarounds. I think that option 2 is probably the most straightforward for someone not familiar with Kubernetes to handle quickly (I hope).

[jrbourbeau]

Alright, I'll give option 2 a go

[mrocklin]

I can also do it if things get too involved. From my perspective it's also nice to increase exposure to these parts of the codebase to others.

…

On Thu, Aug 15, 2019 at 12:09 PM James Bourbeau ***@***.***> wrote: Alright, I'll give option 2 a go — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#171?email_source=notifications&email_token=AACKZTHEE3VMWPTWNYVWG5LQEV5UZA5CNFSM4ILZ6WMKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD4MHX3I#issuecomment-521698285>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AACKZTF2QWMGM34NIFQWPQ3QEV5UZANCNFSM4ILZ6WMA> .

[jrbourbeau]

Agreed, knowledge transfer is definitely a good thing. Thanks for guiding here.

[jrbourbeau]

Came across a configuration setting change in dask/dask that impacted dask_kubernetes/tests/test_core.py::test_maximum. I fixed the issue here and raised an issue upstream (ref dask/dask#5282)

[jrbourbeau]

CI is passing now

FWIW I'm in favor of keeping the removal of CI caching. The conda environment setup doesn't seem to take that long (~90 seconds) and caching is causing some issues with this and other PRs (ref https://circleci.com/gh/dask/dask-kubernetes/396). That said, I will add it back if other feel differently.

[TomAugspurger]

Agreed w.r.t. caching.

…

On Thu, Aug 15, 2019 at 2:19 PM James Bourbeau ***@***.***> wrote: CI is passing now FWIW I'm in favor of keeping the removal of CI caching. The conda environment setup doesn't seem to take that long (~90 seconds) and caching is causing some issues with this and other PRs (ref https://circleci.com/gh/dask/dask-kubernetes/396). That said, I will add it back if other feel differently. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#171?email_source=notifications&email_token=AAKAOIWTCSIPF46NBHG2CI3QEWT5ZA5CNFSM4ILZ6WMKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD4MXW2A#issuecomment-521763688>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAKAOIVS5UZQPZI7T5K7QNTQEWT5ZANCNFSM4ILZ6WMA> .

[jacobtomlinson]

This all looks reasonable to me. Thanks for picking this up.

Are we ready to go with this?

[jrbourbeau]

I think so. @mrocklin any comments or concerns?

[mrocklin]

Yes, this seems fine to me

[jrbourbeau]

Great, merging then. Thanks all for reviewing!