-
-
Notifications
You must be signed in to change notification settings - Fork 740
Add MultiprocessingAuthkeyPlugin to propagate authkey to Dask workers #9124
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
I would still need help to write a useful test... |
Unit Test ResultsSee test report for an extended history of previous test failures. This is useful for diagnosing flaky tests. 27 files ± 0 27 suites ±0 9h 46m 19s ⏱️ + 3m 6s For more details on these failures, see this check. Results for commit e54b412. ± Comparison against base commit c9e7aca. ♻️ This comment has been updated with latest results. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Given that this is a PR to distributed we don't need to do this in a plugin, this could happen directly in the Cluster
object for handing off the key and the Worker
object for recieving it. We could use the Dask config as the transport for the key to avoid us just injecting it into the environment all the time.
For testing we could just ensure the key is successfully set, transmitted and recieved.
Could you give me a hint how to test this properly? The following works instantly (without any change), because the cluster started by def _get_authkey():
import multiprocessing.process
return multiprocessing.process.current_process().authkey
@gen_cluster(client=True)
async def test_authkey(c, s, a, b):
import multiprocessing.process
worker_authkey = await c.submit(_get_authkey)
assert worker_authkey == multiprocessing.process.current_process().authkey Is there a factory for a cluster that is started using, say, |
I don't think there is a factory, but you could use |
That seems to work, thanks!
Is this secure? Is the config visible or even serialized to disk? The Python developers make a big deal about the fact that the authkey must always remain inaccessible from outside... |
When you add it to the config in a Python process it would only be held in memory, we never write config back to the disk, it's only read once when When workers are launched it would be transferred via environment variables, which is the same as the original proposal here. |
Closes #9122
pre-commit run --all-files