Skip to content
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

Remove allow-pickle option #8401

Merged
merged 1 commit into from
Dec 13, 2023

Conversation

fjetter
Copy link
Member

@fjetter fjetter commented Dec 13, 2023

This is no longer an actual option since #7564 since we can technically launch a cluster without pickle but we cannot do anything with it.

I decided to remove the option entirely. If a user is actually setting it, we are now raising an exception during start to notify the user. I decided to go down this path since this option was used for security and I prefer failing hard than letting users think a security feature was still in place.

Sincere apologies for not going through a proper deprecation cycle but I think that ship has sailed. This has already be released in 2023.4.0

Closes #8386

Copy link
Contributor

github-actions bot commented Dec 13, 2023

Unit Test Results

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

       27 files  ±0         27 suites  ±0   11h 54m 25s ⏱️ + 8m 44s
  3 937 tests  - 1    3 819 ✔️  - 4     112 💤 ±0    6 +3 
49 523 runs  +5  47 194 ✔️ +8  2 314 💤  - 6  15 +3 

For more details on these failures, see this check.

Results for commit 4e899a8. ± Comparison against base commit 05ba316.

This pull request removes 2 and adds 1 tests. Note that renamed tests count towards both.
distributed.diagnostics.tests.test_scheduler_plugin ‑ test_register_plugin_pickle_disabled
distributed.tests.test_scheduler ‑ test_run_on_scheduler_disabled
distributed.tests.test_scheduler ‑ test_allow_pickle_false

♻️ This comment has been updated with latest results.

Copy link
Member

@hendrikmakait hendrikmakait left a comment

Choose a reason for hiding this comment

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

Thanks, @fjetter!

@hendrikmakait hendrikmakait merged commit 67b8e7f into dask:main Dec 13, 2023
17 of 34 checks passed
@fjetter fjetter deleted the remove_allow_pickle_option branch December 13, 2023 14:49
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.

Setting distributed.scheduler.pickle=false seems to break everything
2 participants