Skip to content

Conversation

@otterc
Copy link
Contributor

@otterc otterc commented Jun 21, 2021

What changes were proposed in this pull request?

It is a trivial change to remove the reference to an incorrect configuration for push-based shuffle from a test suite.
Ref: #30312
With SPARK-32917, ShuffleBlockPusher and its test suite was introduced. ShuffleBlockPusher is created only when push-based shuffle is enabled and the tests in ShuffleBlockPusherSuite are just testing the functionality in the pusher. So there is no need to have these configs enabled in these test.

Why are the changes needed?

This change removes an incorrect configuration from the test suite.

Does this PR introduce any user-facing change?

No

How was this patch tested?

This change just removes an incorrect configuration from the test suite so haven't added any UTs for it.

@github-actions github-actions bot added the CORE label Jun 21, 2021
@otterc
Copy link
Contributor Author

otterc commented Jun 21, 2021

@mridulm Could you please help review this trivial change

@AmplabJenkins
Copy link

Can one of the admins verify this patch?

@asfgit asfgit closed this in 1fe6daa Jun 21, 2021
@mridulm
Copy link
Contributor

mridulm commented Jun 21, 2021

Thanks for fixing this @otterc

@dongjoon-hyun dongjoon-hyun changed the title [SPARK-35836][SHUFFLE][CORE] Removed the reference to spark.shuffle.push.based.enabled in ShuffleBlockPusherSuite [SPARK-35836][SHUFFLE][CORE][TESTS] Removed the reference to spark.shuffle.push.based.enabled in ShuffleBlockPusherSuite Jun 21, 2021
domybest11 pushed a commit to domybest11/spark that referenced this pull request Jun 15, 2022
…ush.based.enabled in ShuffleBlockPusherSuite

### What changes were proposed in this pull request?
It is a trivial change to remove the reference to an incorrect configuration for push-based shuffle from a test suite.
Ref: apache#30312
With SPARK-32917, `ShuffleBlockPusher` and its test suite was introduced. `ShuffleBlockPusher` is created only when push-based shuffle is enabled and the tests in `ShuffleBlockPusherSuite` are just testing the functionality in the pusher. So there is no need to have these configs enabled in these test.

### Why are the changes needed?
This change removes an incorrect configuration from the test suite.

### Does this PR introduce _any_ user-facing change?
No

### How was this patch tested?
This change just removes an incorrect configuration from the test suite so haven't added any UTs for it.

Closes apache#32992 from otterc/SPARK-35836.

Authored-by: Chandni Singh <[email protected]>
Signed-off-by: Mridul Muralidharan <mridul<at>gmail.com>
wangyum pushed a commit that referenced this pull request May 26, 2023
…ush.based.enabled in ShuffleBlockPusherSuite

### What changes were proposed in this pull request?
It is a trivial change to remove the reference to an incorrect configuration for push-based shuffle from a test suite.
Ref: #30312
With SPARK-32917, `ShuffleBlockPusher` and its test suite was introduced. `ShuffleBlockPusher` is created only when push-based shuffle is enabled and the tests in `ShuffleBlockPusherSuite` are just testing the functionality in the pusher. So there is no need to have these configs enabled in these test.

### Why are the changes needed?
This change removes an incorrect configuration from the test suite.

### Does this PR introduce _any_ user-facing change?
No

### How was this patch tested?
This change just removes an incorrect configuration from the test suite so haven't added any UTs for it.

Closes #32992 from otterc/SPARK-35836.

Authored-by: Chandni Singh <[email protected]>
Signed-off-by: Mridul Muralidharan <mridul<at>gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants