-
-
Notifications
You must be signed in to change notification settings - Fork 748
Fix tests for p2p by default #7595
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
Conversation
Unit Test ResultsSee test report for an extended history of previous test failures. This is useful for diagnosing flaky tests. 24 files ±0 24 suites ±0 11h 4m 44s ⏱️ + 12m 31s Results for commit 4554122. ± Comparison against base commit 40456b4. This pull request skips 1 and un-skips 1 tests.♻️ This comment has been updated with latest results. |
30d28da to
944e11a
Compare
| @pytest.mark.skipif( | ||
| ( | ||
| parse_version(dask.__version__) < parse_version("2023.2.2") | ||
| and parse_version(dask.__version__) >= parse_version("2023.2.1") | ||
| ), | ||
| reason="https://github.com/dask/dask/pull/10005", | ||
| ) | ||
| @gen_cluster(client=True) | ||
| async def test_bag_groupby_tasks_default(c, s, a, b): |
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.
I decided to skip this test to unblock our CI. dask/dask#10005 will introduce the same test in dask/dask, i.e. we could even remove it from distributed. keeping it here for early failures is still nice since the test is very cheap
| # These may change in the future but the selection below shoul dnot | ||
| distributed_default = "tasks" | ||
| # These may change in the future but the selection below should not | ||
| distributed_default = "p2p" if has_pyarrow else "tasks" |
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.
My guess is we'll also want to use check_minimal_arrow_version() to make sure we account for pyarrow<7 being installed
jrbourbeau
left a comment
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.
Thanks @fjetter
Closes #7591