[SPARK-33346][CORE][SQL][MLLIB][DSTREAM][K8S] Change the never changed 'var' to 'val' by LuciferYang · Pull Request #31142 · apache/spark

LuciferYang · 2021-01-12T03:57:34Z

What changes were proposed in this pull request?

Some local variables are declared as var, but they are never reassigned and should be declared as val, so this pr turn these from var to val except for mockito related cases.

Why are the changes needed?

Use val instead of var when possible.

Does this PR introduce any user-facing change?

No

How was this patch tested?

Pass the Jenkins or GitHub Action

SparkQA · 2021-01-12T07:17:32Z

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

SparkQA · 2021-01-12T07:46:31Z

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

SparkQA · 2021-01-12T09:07:58Z

Test build #133961 has finished for PR 31142 at commit 3929b10.

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

jaceklaskowski

LGTM (non-binding)

SparkQA · 2021-01-12T12:52:51Z

Test build #133955 has finished for PR 31142 at commit 924358a.

This patch fails from timeout after a configured wait of 500m.
This patch merges cleanly.
This patch adds no public classes.

srowen

Seems fine. Are there more?

LuciferYang · 2021-01-13T02:41:01Z

@srowen Let me check test dir today

LuciferYang · 2021-01-13T12:56:52Z

@srowen Address 67e16b9 add test dir related files

SparkQA · 2021-01-13T14:22:49Z

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

SparkQA · 2021-01-13T14:27:09Z

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

SparkQA · 2021-01-13T16:23:36Z

Test build #134016 has finished for PR 31142 at commit 4c48d68.

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

srowen · 2021-01-13T16:39:32Z

core/src/test/scala/org/apache/spark/deploy/worker/WorkerSuite.scala

  import org.apache.spark.deploy.DeployTestUtils._

-  @Mock(answer = RETURNS_SMART_NULLS) private var shuffleService: ExternalShuffleService = _
+  @Mock(answer = RETURNS_SMART_NULLS) private val shuffleService: ExternalShuffleService = null


Hm, I was going to wonder if we have to leave mocked members as 'var' because the framework will change them. But it looks like tests pass.

https://github.com/mockito/mockito/blob/019830665f97c8ba6159148e3b10086cfb894900/src/main/java/org/mockito/internal/util/reflection/FieldSetter.java#L13-L18

mockito use reflection api to change them, so they can be val, although var seems easier to understand.

So for ease of understanding, we can also not change the case related to @Mock

Sure, maybe revert those just to be conservative.

SparkQA · 2021-01-14T15:48:19Z

Test build #134058 has started for PR 31142 at commit 06dbd94.

SparkQA · 2021-01-14T16:32:09Z

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

SparkQA · 2021-01-14T17:08:36Z

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

SparkQA · 2021-01-15T03:30:33Z

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

SparkQA · 2021-01-15T03:56:15Z

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

srowen · 2021-01-15T04:44:43Z

Jenkins retest this please

SparkQA · 2021-01-15T05:01:52Z

Test build #134068 has finished for PR 31142 at commit c599a05.

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

HyukjinKwon · 2021-01-15T06:09:13Z

retest this please

SparkQA · 2021-01-15T07:31:54Z

Test build #134077 has finished for PR 31142 at commit c599a05.

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

SparkQA · 2021-01-15T09:29:15Z

Test build #134088 has finished for PR 31142 at commit c599a05.

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

srowen · 2021-01-15T14:47:11Z

Merged to master

LuciferYang · 2021-01-18T02:15:16Z

thx~ @srowen and @HyukjinKwon

var to val

13ec7e1

LuciferYang changed the title ~~[SPARK-33346][CORE][SQL][MLLIB] Change the never changed 'var' to 'val'~~ [SPARK-33346][CORE][SQL][MLLIB] Use 'val' instead of 'var' when possible. Jan 12, 2021

revert Unnecessary changes

924358a

github-actions bot added CORE EXAMPLES MLLIB SQL STRUCTURED STREAMING labels Jan 12, 2021

LuciferYang changed the title ~~[SPARK-33346][CORE][SQL][MLLIB] Use 'val' instead of 'var' when possible.~~ [WIP][SPARK-33346][CORE][SQL][MLLIB] Change the never changed 'var' to 'val' Jan 12, 2021

LuciferYang marked this pull request as draft January 12, 2021 05:52

revert change of OpenHashMap

3929b10

LuciferYang changed the title ~~[WIP][SPARK-33346][CORE][SQL][MLLIB] Change the never changed 'var' to 'val'~~ [SPARK-33346][CORE][SQL][MLLIB] Change the never changed 'var' to 'val' Jan 12, 2021

LuciferYang marked this pull request as ready for review January 12, 2021 07:48

jaceklaskowski reviewed Jan 12, 2021

View reviewed changes

srowen approved these changes Jan 12, 2021

View reviewed changes

HyukjinKwon mentioned this pull request Jan 13, 2021

[SPARK-34053][INFRA][FOLLOW-UP] Disables canceling push/schedule workflows #31153

Closed

fix test dir

67e16b9

LuciferYang changed the title ~~[SPARK-33346][CORE][SQL][MLLIB] Change the never changed 'var' to 'val'~~ [SPARK-33346][CORE][SQL][MLLIB][STREAMING][K8S] Change the never changed 'var' to 'val' Jan 13, 2021

fix compile

4c48d68

github-actions bot added DSTREAM KUBERNETES ML labels Jan 13, 2021

LuciferYang changed the title ~~[SPARK-33346][CORE][SQL][MLLIB][STREAMING][K8S] Change the never changed 'var' to 'val'~~ [SPARK-33346][CORE][SQL][MLLIB][DSTREAM][K8S] Change the never changed 'var' to 'val' Jan 13, 2021

srowen reviewed Jan 13, 2021

View reviewed changes

LuciferYang added 3 commits January 14, 2021 23:05

revect mock releated cases

9993876

revect mock releated cases 2

aec1751

revect mock releated cases 3

06dbd94

LuciferYang added 2 commits January 15, 2021 10:28

trigger GA and jenkins

8a95ae4

trigger GA and jenkins

c599a05

srowen approved these changes Jan 15, 2021

View reviewed changes

srowen closed this in 9e33d49 Jan 15, 2021

LuciferYang deleted the SPARK-33346 branch June 6, 2022 03:45

Conversation

LuciferYang commented Jan 12, 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!

SparkQA commented Jan 12, 2021

Uh oh!

SparkQA commented Jan 12, 2021

Uh oh!

SparkQA commented Jan 12, 2021

Uh oh!

jaceklaskowski left a comment

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Jan 12, 2021

Uh oh!

srowen left a comment

Choose a reason for hiding this comment

Uh oh!

LuciferYang commented Jan 13, 2021

Uh oh!

LuciferYang commented Jan 13, 2021

Uh oh!

SparkQA commented Jan 13, 2021

Uh oh!

SparkQA commented Jan 13, 2021

Uh oh!

SparkQA commented Jan 13, 2021

Uh oh!

srowen Jan 13, 2021

Choose a reason for hiding this comment

Uh oh!

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

Uh oh!

Choose a reason for hiding this comment

Uh oh!

srowen Jan 14, 2021

Choose a reason for hiding this comment

Uh oh!

LuciferYang Jan 14, 2021

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Jan 14, 2021

Uh oh!

SparkQA commented Jan 14, 2021

Uh oh!

SparkQA commented Jan 14, 2021

Uh oh!

SparkQA commented Jan 15, 2021

Uh oh!

SparkQA commented Jan 15, 2021

Uh oh!

srowen commented Jan 15, 2021

Uh oh!

SparkQA commented Jan 15, 2021

Uh oh!

HyukjinKwon commented Jan 15, 2021

Uh oh!

SparkQA commented Jan 15, 2021

Uh oh!

SparkQA commented Jan 15, 2021

Uh oh!

srowen commented Jan 15, 2021

Uh oh!

LuciferYang commented Jan 18, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

LuciferYang commented Jan 12, 2021 •

edited

Loading

LuciferYang Jan 14, 2021 •

edited

Loading