dask / distributed

A distributed task scheduler for Dask
https://distributed.dask.org
BSD 3-Clause "New" or "Revised" License
1.58k stars 719 forks source link

Raise helpful error when using the wrong plugin base classes #8893

Closed jacobtomlinson closed 1 month ago

jacobtomlinson commented 1 month ago

The Client.register_plugin() method uses @functools.singledispatchmethod to dispatch to the correct plugin registration method. However singledispatchmethod doesn't like the dask.distributed import shim and can't determine that dask.distributed.diagnostics.plugins.WorkerPlugin is the same as distributed.diagnostics.plugins.WorkerPlugin.

This means that plugins based on classes like dask.distributed.diagnostics.plugins.WorkerPlugin raise the TypeError("Registering duck-typed plugins is not allowed. Please inherit from NannyPlugin, WorkerPlugin, or SchedulerPlugin to create a plugin.") exception.

I explored importing the base classes from the dask.distributed path and registering those for dispatch too, but it's not possible to import them from dask.distributed.diagnostics.plugins while distributed is being imported. And it's not possible to register them at runtime, only at import time.

Therefore the best option here appears to be to improve the error message. So this PR checks for plugins based on classes from dask.distributed.diagnostics.plugins and makes the error message more helpful.

In [1]: from dask.distributed.diagnostics.plugin import WorkerPlugin
   ...: import dask.distributed as dd
   ...: 
   ...: class PolitePlugin(WorkerPlugin):
   ...:     def setup(self, worker):
   ...:         print("hi", worker)
   ...:         self.worker = worker
   ...: 
   ...:     def teardown(self, worker):
   ...:         print("bye", worker)
   ...: 
   ...: cluster = dd.LocalCluster()
   ...: client = dd.Client(cluster)
   ...: client.register_plugin(PolitePlugin())
---------------------------------------------------------------------------
TypeError: Importing plugin base classes from `dask.distributed.diagnostics.plugin` is not supported. Please import directly from `distributed.diagnostics.plugin` instead.

Closes #8778

github-actions[bot] commented 1 month ago

Unit Test Results

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

    25 files  ±0      25 suites  ±0   10h 18m 23s :stopwatch: - 5m 4s  4 130 tests ±0   4 016 :white_check_mark: +1    110 :zzz: ±0  4 :x:  - 1  47 708 runs  ±0  45 607 :white_check_mark:  - 1  2 096 :zzz: +1  5 :x: ±0 

For more details on these failures, see this check.

Results for commit 9be2d8db. ± Comparison against base commit 9bec841a.