Small tweaks to make compatible with dask-mpi

[jacobtomlinson]

While working on GPU support in dask-mpi I ran into a few issues with CUDAWorker. This PR makes a few tweaks to ensure things are compatible.

1. A valid option for local_directory is an empty string representing the current directory. So added a check before trying to create the directory.

2. Integers are valid options for name so added an explicit cast to string when concatenating with GPU index.

3. In dask-mpi it uses async with for creating Worker and Nanny objects. This broke with CUDAWorker because __aenter__ and __aexit__ have not been implemented. Looking at distributed those methods are implemented in the base Server class, so I've updated CUDAWorker here to also use Server as a base.

   I was surprised this wasn't the case already, but perhaps that was an intentional decision? If so it would be quick to implement the async context manager methods directly here instead.

[madsbk]

LGTM @jacobtomlinson, thanks.

FYI: I am working on the CI error: #658

[madsbk]

3. I was surprised this wasn't the case already, but perhaps that was an intentional decision? If so it would be quick to implement the async context manager methods directly here instead.

Yeah, that is strange. I don't think there is any reason for that. @pentschev or @quasiben do you know if it is deliberate that CUDAWorker doesn't inherit from distributed.Server?

[pentschev]

Yeah, that is strange. I don't think there is any reason for that. @pentschev or @quasiben do you know if it is deliberate that CUDAWorker doesn't inherit from distributed.Server?

I think there's no actual reason for that besides it not being a class that's trying to reproduce the behavior of Nanny or Worker currently, but simply instantiates Nanny. Originally, I think there was only the dask-cuda-worker CLI and @jacobtomlinson wrote CUDAWorker based on that and changed dask-cuda-worker to call CUDAWorker instead, IIRC, the reason for that was to ensure we could use it with SSHCluster. With that said, I'm not sure whether there's a reason we can't/shouldn't be doing it.

3. Looking at distributed those methods are implemented in the base Server class, so I've updated CUDAWorker here to also use Server as a base.

I haven't looked at it in detail to figure whether there's a difference, but it seems that the inheritance is from ServerNode and not Server in both Nanny and Worker. Are Server and ServerNode the same?

[jacobtomlinson]

@jacobtomlinson wrote CUDAWorker based on that and changed dask-cuda-worker to call CUDAWorker instead

Heh it's probably my fault that it doesn't inherit from anything.

Are Server and ServerNode the same?

ServerNode adds some extra methods for starting up web server based services like the dashboard, but as you say we don't need any of that here. My main requirement is that CUDAWorker behaves like a Worker from an asyncio point of view, which inheriting from Server should cover.

[pentschev]

rerun tests

[codecov-commenter]

Codecov Report

❗ No coverage uploaded for pull request base (branch-21.08@f3220a4). Click here to learn what that means.
The diff coverage is n/a.

❗ Current head 21d9f44 differs from pull request most recent head 7377716. Consider uploading reports for the commit 7377716 to get more accurate results

@@               Coverage Diff               @@
##             branch-21.08     #656   +/-   ##
===============================================
  Coverage                ?   90.32%           
===============================================
  Files                   ?       15           
  Lines                   ?     1644           
  Branches                ?        0           
===============================================
  Hits                    ?     1485           
  Misses                  ?      159           
  Partials                ?        0           

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f3220a4...7377716. Read the comment docs.

[pentschev]

It seems that all tests now pass but the old CUDA 11.0 build is not running anymore, leaving CI in a failed state, we probably will need @rapidsai/ops-codeowners to force merge this.

[ajschmidt8]

@gpucibot merge

[jacobtomlinson]

Thanks @ajschmidt8