Skip to content

Conversation

@attilapiros
Copy link
Contributor

What changes were proposed in this pull request?

The testTransformerByInterceptingException failed to catch the expected message on 2.3 during streaming tests as the feature generated message is not at the direct caused by exception but even one level deeper.

How was this patch tested?

Running the unit tests.

@SparkQA
Copy link

SparkQA commented Mar 18, 2018

Test build #88345 has finished for PR 20852 at commit 7544cb4.

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

@dongjoon-hyun
Copy link
Member

Hi, @yanboliang and @jkbradley .
branch-2.3 is broken. Could you review this PR?

exception.getCause != null && (
hasExpectedMessageDirectly(exception.getCause) || (
exception.getCause.getCause != null &&
hasExpectedMessageDirectly(exception.getCause.getCause))))
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not use a loop to check inner exception ? So it can check any deep level exception msg.
Maybe we cannot make sure the possible root exception max level (even in master version)
@jkbradley What do you think ?

Copy link
Member

Choose a reason for hiding this comment

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

I don't think we need to loop further. If the real message is buried this deep, that could be considered a problem in and of itself.

@jkbradley
Copy link
Member

Apologies for breaking it!

This LGTM

I'll go ahead and merge it to fix the build, but please comment further on this PR as needed.

@jkbradley
Copy link
Member

Merging with branch-2.3

asfgit pushed a commit that referenced this pull request Mar 19, 2018
…ng streaming tests

## What changes were proposed in this pull request?

The testTransformerByInterceptingException failed to catch the expected message on 2.3 during streaming tests as the feature generated message is not at the direct caused by exception but even one level deeper.

## How was this patch tested?

Running the unit tests.

Author: “attilapiros” <[email protected]>

Closes #20852 from attilapiros/SPARK-23728.
@dongjoon-hyun
Copy link
Member

Thank you, @jkbradley and @attilapiros .

@HyukjinKwon
Copy link
Member

(@attilapiros, just in case it should be manually closed)

@attilapiros
Copy link
Contributor Author

Closing manually as it was merged to branch-2.3.

@attilapiros attilapiros deleted the SPARK-23728 branch April 26, 2018 20:07
peter-toth pushed a commit to peter-toth/spark that referenced this pull request Oct 6, 2018
…ng streaming tests

## What changes were proposed in this pull request?

The testTransformerByInterceptingException failed to catch the expected message on 2.3 during streaming tests as the feature generated message is not at the direct caused by exception but even one level deeper.

## How was this patch tested?

Running the unit tests.

Author: “attilapiros” <[email protected]>

Closes apache#20852 from attilapiros/SPARK-23728.
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.

6 participants