Skip to content

[SPARK-33198][CORE] getMigrationBlocks should not fail at missing files#30110

Closed
dongjoon-hyun wants to merge 3 commits intoapache:masterfrom
dongjoon-hyun:SPARK-33198
Closed

[SPARK-33198][CORE] getMigrationBlocks should not fail at missing files#30110
dongjoon-hyun wants to merge 3 commits intoapache:masterfrom
dongjoon-hyun:SPARK-33198

Conversation

@dongjoon-hyun
Copy link
Member

@dongjoon-hyun dongjoon-hyun commented Oct 20, 2020

What changes were proposed in this pull request?

This PR aims to fix getMigrationBlocks error handling and to add test coverage.

  1. getMigrationBlocks should not fail at indexFile only case.
  2. assert causes java.lang.AssertionError which is not an Exception.

Why are the changes needed?

To handle the exception correctly.

Does this PR introduce any user-facing change?

No.

How was this patch tested?

Pass the CI with the newly added test case.

Copy link
Contributor

@holdenk holdenk left a comment

Choose a reason for hiding this comment

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

LGTM pending CI

@dongjoon-hyun
Copy link
Member Author

Thank you for review and approval, @holdenk !

@SparkQA
Copy link

SparkQA commented Oct 20, 2020

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/34662/

@SparkQA
Copy link

SparkQA commented Oct 20, 2020

Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/34662/

@SparkQA
Copy link

SparkQA commented Oct 20, 2020

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/34664/

@SparkQA
Copy link

SparkQA commented Oct 20, 2020

Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/34664/

@dongjoon-hyun
Copy link
Member Author

GitHub Action and K8s IT passed. Merged to master.

@dongjoon-hyun dongjoon-hyun deleted the SPARK-33198 branch October 20, 2020 22:03
@SparkQA
Copy link

SparkQA commented Oct 20, 2020

Test build #130053 has finished for PR 30110 at commit 824d868.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented Oct 20, 2020

Test build #130054 has finished for PR 30110 at commit 69bf6c0.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented Oct 20, 2020

Test build #130055 has finished for PR 30110 at commit 73864b0.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

holdenk pushed a commit to holdenk/spark that referenced this pull request Oct 27, 2020
### What changes were proposed in this pull request?

This PR aims to fix `getMigrationBlocks` error handling and to add test coverage.
1. `getMigrationBlocks` should not fail at indexFile only case.
2. `assert` causes `java.lang.AssertionError` which is not an `Exception`.

### Why are the changes needed?

To handle the exception correctly.

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

No.

### How was this patch tested?

Pass the CI with the newly added test case.

Closes apache#30110 from dongjoon-hyun/SPARK-33198.

Authored-by: Dongjoon Hyun <dhyun@apple.com>
Signed-off-by: Dongjoon Hyun <dhyun@apple.com>
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.

3 participants