Add preload script to conf#2325
Conversation
mrocklin
left a comment
mrocklin
left a comment
There was a problem hiding this comment.
There may be some tests here (though I haven't explored much):
$ git grep test.*preload
distributed/cli/tests/test_dask_scheduler.py:def test_preload_file(loop):
distributed/cli/tests/test_dask_scheduler.py:def test_preload_module(loop):
distributed/cli/tests/test_dask_scheduler.py:def test_preload_command(loop):
distributed/cli/tests/test_dask_scheduler.py:def test_preload_command_default(loop):
distributed/tests/test_preload.py:def test_worker_preload_file(loop):
distributed/tests/test_preload.py:def test_worker_preload_module(loop):
I agree that it's odd the preload is only in the CLI. I think that it would be good to move this within the Scheduler and Worker classes. That change would be welcome here, or we could mark it as future work in an issue.
| if not preload: | ||
| preload = dask.config.get('distributed.scheduler.preloads') | ||
| if not preload_argv: | ||
| preload_argv = dask.config.get('distributed.scheduler.preloads-argv') |
There was a problem hiding this comment.
Thoughts on preload vs preloads? I'm not sure if we should pluralize this word, given the single-ness of the --preload CLI keyword.
There was a problem hiding this comment.
Fine by me to go with preload, I'll update the PR accordingly.
For Worker this is already the case. For Scheduler, there seems to be some reason about it, thus I'm not sure this is a trivial change. I did not find a simple way to do it when I looked at it. |
|
Test failures here seem unrelated. See #2331 |
|
Sorry for the delay on this. TLS issues were fixed in other PRs. Things are green now. Merging. |
|
No problem, I was planning to look at the tests you mentioned to see if I can add something. But this seems simple enough to merge it as it is, hopefully I did not messed up with default kwargs 🙂 |
2 participants
Fixes #2121.
Still trying to dive inside codebase, so I pick up some simple issues. Two questions for this one: