[SPARK-33934][SQL][FOLLOW-UP] Use SubProcessor's exit code as assert condition to fix flaky test #31046

AngersZhuuuu · 2021-01-05T21:21:37Z

What changes were proposed in this pull request?

Follow comment and fix. flaky test #30973 (comment).
This flaky test is similar as #30896

Some task's failed with root cause but in driver may return error without root cause , change. UT to check with status exit code since different root cause's exit code is not same.

Why are the changes needed?

Fix flaky test

Does this PR introduce any user-facing change?

No

How was this patch tested?

Existed UT

…y PATH

AngersZhuuuu · 2021-01-05T21:21:56Z

FYI @dongjoon-hyun

dongjoon-hyun

Thank you for the quick follow-up, @AngersZhuuuu .

dongjoon-hyun · 2021-01-05T22:02:28Z

@AngersZhuuuu . Since you are one of the active contributors, I want to give some advice.
Could you revise the PR title? In the Apache project, we had better avoid repeating the original PR title in the follow-up PR because it doesn't make any sense. The commit title is a precious resource and you can utilize it more meaningly by describing the corresponding commit content.

maropu · 2021-01-05T22:44:12Z

The fix itself seems fine. As Dongjoon suggested above, I think the title and PR description need to be modified a bit though.

AngersZhuuuu · 2021-01-05T23:47:41Z

@AngersZhuuuu . Since you are one of the active contributors, I want to give some advice.
Could you revise the PR title? In the Apache project, we had better avoid repeating the original PR title in the follow-up PR because it doesn't make any sense. The commit title is a precious resource and you can utilize it more meaningly by describing the corresponding commit content.

Done

maropu · 2021-01-06T00:09:49Z

Done

Really? It seems you forgot to update the PR title...

HyukjinKwon · 2021-01-06T00:32:29Z

sql/core/src/main/scala/org/apache/spark/sql/execution/BaseScriptTransformationExec.scala

 package org.apache.spark.sql.execution

-import java.io._
+import java.io.{BufferedReader, File, InputStream, InputStreamReader, OutputStream}


Oh, I don't know why I thought it's more than 5+. It's more than 6+ (https://github.com/databricks/scala-style-guide#imports).

Oh, I don't know why I thought it's more than 5+. It's more than 6+ (https://github.com/databricks/scala-style-guide#imports).

Seems my local ideal's rule is 5, emmm I need to check this.

sql/core/src/test/resources/test_script.py

dongjoon-hyun · 2021-01-06T01:18:31Z

@AngersZhuuuu . You didn't change the PR title yet.

AngersZhuuuu · 2021-01-06T01:27:06Z

@AngersZhuuuu . You didn't change the PR title yet.

Sorry for my bad network when at home...re-edit the title but seems failed to change. Updated now, cc. @maropu

Co-authored-by: Hyukjin Kwon <[email protected]>

dongjoon-hyun

+1, LGTM.

SparkQA · 2021-01-06T02:57:25Z

Test build #133694 has finished for PR 31046 at commit 03627c2.

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

dongjoon-hyun · 2021-01-06T03:12:47Z

@AngersZhuuuu . Could you add three empty commits because this is a fix for flaky tests?

AngersZhuuuu · 2021-01-06T03:17:21Z

@AngersZhuuuu . Could you add three empty commits because this is a fix for flaky tests?

Done, but I am confused why we do this ==

dongjoon-hyun · 2021-01-06T03:20:41Z

Ur, you should push one by one. It will trigger 3 GitHub Action jobs.
Currently, the three commits seems to be pushed as a single push. So, only one GitHub Action is trigger. (The yellow circle)

AngersZhuuuu · 2021-01-06T03:22:30Z

Ur, you should push one by one. It will trigger 3 GitHub Action job.
Currently, the three commits seems to be pushed as a single push.

Emmmm, got your point, push some empty commit to trigger GA to test the UT's stability.
I will do this.

SparkQA · 2021-01-06T04:58:11Z

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

SparkQA · 2021-01-06T05:29:15Z

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

SparkQA · 2021-01-06T06:09:22Z

Test build #133697 has finished for PR 31046 at commit 1fbc91c.

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

dongjoon-hyun · 2021-01-06T06:32:56Z

Although the last GitHub Action job is in the queue, the last commit (and its empty commits) passed three times already.
Merged to master. Thank you, all!

SparkQA · 2021-01-06T07:01:18Z

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

SparkQA · 2021-01-06T07:31:34Z

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

SparkQA · 2021-01-06T08:29:15Z

Test build #133707 has finished for PR 31046 at commit e5cf288.

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

SparkQA · 2021-01-06T09:21:27Z

Test build #133716 has finished for PR 31046 at commit 404fb63.

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

SparkQA · 2021-01-06T09:44:55Z

Test build #133718 has finished for PR 31046 at commit db12e28.

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

[SPARK-33934][SQL][FOLLOW-UP] Add SparkFile's root dir to env propert…

03627c2

…y PATH

github-actions bot added the SQL label Jan 5, 2021

dongjoon-hyun reviewed Jan 5, 2021

View reviewed changes

HyukjinKwon reviewed Jan 6, 2021

View reviewed changes

sql/core/src/test/resources/test_script.py Outdated Show resolved Hide resolved

AngersZhuuuu changed the title ~~[SPARK-33934][SQL][FOLLOW-UP] Add SparkFile's root dir to env property rot~~ [SPARK-33934][SQL][FOLLOW-UP] Use SubProcessor's exit code as assert condition to fix flaky test Jan 6, 2021

Update sql/core/src/test/resources/test_script.py

1fbc91c

Co-authored-by: Hyukjin Kwon <[email protected]>

dongjoon-hyun approved these changes Jan 6, 2021

View reviewed changes

HyukjinKwon approved these changes Jan 6, 2021

View reviewed changes

maropu approved these changes Jan 6, 2021

View reviewed changes

AngersZhuuuu added 3 commits January 6, 2021 11:16

Empty

19f124f

Empty

7c2ce48

Empty

e5cf288

Empty

404fb63

Empty

db12e28

dongjoon-hyun closed this in c0d0dba Jan 6, 2021

hammertank mentioned this pull request Feb 8, 2021

[SPARK-33152][SQL] Improve the performance of constraint propagation for Project and Aggregate #30894

Closed

[SPARK-33934][SQL][FOLLOW-UP] Use SubProcessor's exit code as assert condition to fix flaky test #31046

[SPARK-33934][SQL][FOLLOW-UP] Use SubProcessor's exit code as assert condition to fix flaky test #31046

Uh oh!

Conversation

AngersZhuuuu commented Jan 5, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What changes were proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user-facing change?

How was this patch tested?

Uh oh!

AngersZhuuuu commented Jan 5, 2021

Uh oh!

dongjoon-hyun left a comment

Choose a reason for hiding this comment

Uh oh!

dongjoon-hyun commented Jan 5, 2021

Uh oh!

maropu commented Jan 5, 2021

Uh oh!

AngersZhuuuu commented Jan 5, 2021

Uh oh!

maropu commented Jan 6, 2021

Uh oh!

HyukjinKwon Jan 6, 2021

Choose a reason for hiding this comment

Uh oh!

AngersZhuuuu Jan 6, 2021

Choose a reason for hiding this comment

Uh oh!

Uh oh!

dongjoon-hyun commented Jan 6, 2021

Uh oh!

AngersZhuuuu commented Jan 6, 2021

Uh oh!

dongjoon-hyun left a comment

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Jan 6, 2021

Uh oh!

dongjoon-hyun commented Jan 6, 2021

Uh oh!

AngersZhuuuu commented Jan 6, 2021

Uh oh!

dongjoon-hyun commented Jan 6, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

AngersZhuuuu commented Jan 6, 2021

Uh oh!

SparkQA commented Jan 6, 2021

Uh oh!

SparkQA commented Jan 6, 2021

Uh oh!

SparkQA commented Jan 6, 2021

Uh oh!

dongjoon-hyun commented Jan 6, 2021

Uh oh!

SparkQA commented Jan 6, 2021

Uh oh!

SparkQA commented Jan 6, 2021

Uh oh!

SparkQA commented Jan 6, 2021

Uh oh!

SparkQA commented Jan 6, 2021

Uh oh!

SparkQA commented Jan 6, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

AngersZhuuuu commented Jan 5, 2021 •

edited

Loading

dongjoon-hyun commented Jan 6, 2021 •

edited

Loading