Skip to content

Conversation

@jacobtomlinson
Copy link
Member

The Scheduler.add_plugin has an idempotent flag which can be set to True to ensure a plugin is only registered once the first time. The default behaviour is to overwrite the plugin with new ones from subsequent attempts.

This flag is not exposed to the register_scheduler_plugin RPC method. This PR simply passes that flag through.

Copy link
Member

@jrbourbeau jrbourbeau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @jacobtomlinson. Exposing the idempotent keyword seems totally reasonable. Would you mind adding a small test for this change?

Also, this exposes an issue where we'll still call the plugin's .start() method even if idempotent=True. Perhaps we should have some extra logic to return early in that case?

@jacobtomlinson
Copy link
Member Author

Thanks for the review @jrbourbeau.

To address the starting of plugins I added another check an even earlier return. Also added tests for idempotent and non-idempotent plugins.

Copy link
Member

@jrbourbeau jrbourbeau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @jacobtomlinson!

@jrbourbeau jrbourbeau merged commit b1e06a0 into dask:main Nov 30, 2021
@jacobtomlinson jacobtomlinson deleted the idempotent-scheduler-pugin branch November 30, 2021 16:22
asford added a commit to asford/distributed that referenced this pull request May 11, 2022
Updates client-side interface of register_scheduler_plugin to pass through idempotent flag.

Extending dask#5545

Finalizing removal of kwargs support
deprecated in dask#5699
and removed in dask#6144
mrocklin pushed a commit that referenced this pull request May 13, 2022
Updates client-side interface of register_scheduler_plugin to pass through idempotent flag.

Extending #5545

Finalizing removal of kwargs support
deprecated in #5699
and removed in #6144
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants