-
Notifications
You must be signed in to change notification settings - Fork 5.8k
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
Streaming executor fixes #4 #32882
Streaming executor fixes #4 #32882
Changes from all commits
edc51bd
61f4d6d
a33a943
36ebe52
ce6763e
0e2c29e
f2b6ed0
bb6c5c4
540fe79
edad7d0
60cc079
a3d3980
001579c
8aeed6c
7a9a49b
ef97167
6f0563c
bcec4d6
ddef4e5
fc9a175
f0e90b7
999d1de
d8159e3
d81cd02
bc831bb
c444395
642da6f
f713f2f
d416a73
da5acee
ab64cb6
7a5d5e3
47010ca
b9fcb5f
9cb959d
3f26b7c
d841b33
0ce805c
0258409
846cc65
7edecbd
e4e8d38
367354d
f6af0a9
f7069c9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -382,12 +382,13 @@ def test_zip_different_num_blocks_split_smallest( | |
[{str(i): i for i in range(num_cols1, num_cols1 + num_cols2)}] * n, | ||
parallelism=num_blocks2, | ||
) | ||
ds = ds1.zip(ds2) | ||
ds = ds1.zip(ds2).fully_executed() | ||
num_blocks = ds._plan._snapshot_blocks.executed_num_blocks() | ||
assert ds.take() == [{str(i): i for i in range(num_cols1 + num_cols2)}] * n | ||
if should_invert: | ||
assert ds.num_blocks() == num_blocks2 | ||
assert num_blocks == num_blocks2 | ||
else: | ||
assert ds.num_blocks() == num_blocks1 | ||
assert num_blocks == num_blocks1 | ||
Comment on lines
+385
to
+391
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. curious as #32860 is merged, would the change here is still needed? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Have you tested that PR with streaming enabled for this test? Also note this is to handle issue introduced in #32132. |
||
|
||
|
||
def test_zip_pandas(ray_start_regular_shared): | ||
|
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.
This test is called
test_bulk_executor
so shall we just force bulk execution mode instead?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.
Makes sense!