[SPARK-45629][CORE][SQL][CONNECT][ML][STREAMING][BUILD][EXAMPLES]Fix `Implicit definition should have explicit type` #43526

laglangyue · 2023-10-25T09:11:13Z

What changes were proposed in this pull request?

This PR aims to fix Implicit definition should have explicit type in Scala 2.13.

This pr includes:

Declaration types for global variables of implicit
Add @scala.annotation.warn

Why are the changes needed?

For implicit global variables without explicit type declaration, will get warnning :
warning: Implicit definition should have explicit type (inferred String) [quickfixable]
No modifications are required for local variables.
Additionally, to handle cases involving reflection-related types like ClassTag in implicit variables, the @scala.annotation.warn annotation is used to suppress the warning.

Furthermore, warnings generated in Spark will be treated as errors:

[error] ... Implicit definition should have explicit type (inferred org.json4s.DefaultFormats.type) [quickfixable]
...
[error] implicit val formats = org.json4s.DefaultFormats

Jira link:
SPARK-45314: https://issues.apache.org/jira/browse/SPARK-45629

Related issue link about implicit :
scala/bug#5265

Does this PR introduce any user-facing change?

no

How was this patch tested?

Most of the testing is completed through CI, and the example module is locally compiled and tested in IDEA Additionally, there are some writing changes that are verified through demo code

Was this patch authored or co-authored using generative AI tooling?

no

connector/kafka-0-10-sql/src/main/scala/org/apache/spark/sql/kafka010/JsonUtils.scala

laglangyue · 2023-11-01T04:30:24Z

Hi, brother
@LuciferYang
How can I reproduce this? I would like to check if I have made all the necessary modifications

[error] /Users/yangjie01/SourceCode/git/spark-mine-sbt/core/src/main/scala/org/apache/spark/deploy/FaultToleranceTest.scala:343:16: Implicit definition should have explicit type (inferred org.json4s.DefaultFormats.type) [quickfixable]
[error] Applicable -Wconf / @nowarn filters for this fatal warning: msg=<part of the message>, cat=other-implicit-type, site=org.apache.spark.deploy.TestMasterInfo.formats
[error]   implicit val formats = org.json4s.DefaultFormats
[error]

LuciferYang · 2023-11-01T05:33:14Z

spark/project/SparkBuild.scala

Line 251 in e1bc48b

"-Wconf:msg=Implicit definition should have explicit type:s",

@laglangyue You can delete the above line and then compile with

build/sbt -Phadoop-3 -Pdocker-integration-tests -Pspark-ganglia-lgpl -Pkinesis-asl -Pkubernetes -Phive-thriftserver -Pconnect -Pyarn -Phive -Phadoop-cloud -Pvolcano -Pkubernetes-integration-tests Test/package streaming-kinesis-asl-assembly/assembly connect/assembly

If no further compilation errors occur, we can consider the task completed.

laglangyue · 2023-11-03T06:43:21Z

spark/project/SparkBuild.scala

Line 251 in e1bc48b

"-Wconf:msg=Implicit definition should have explicit type:s",

@laglangyue You can delete the above line and then compile with
build/sbt -Phadoop-3 -Pdocker-integration-tests -Pspark-ganglia-lgpl -Pkinesis-asl -Pkubernetes -Phive-thriftserver -Pconnect -Pyarn -Phive -Phadoop-cloud -Pvolcano -Pkubernetes-integration-tests Test/package streaming-kinesis-asl-assembly/assembly connect/assembly
If no further compilation errors occur, we can consider the task completed.

Exciting for building successful,I has finished.

laglangyue · 2023-11-03T06:44:06Z

LuciferYang

Can you update the pr title to add the corresponding module label? Can you update the pr description to specifically explain why this work needs to be done (I don't think the subtask of SPARK-45314 is a rigorous reason)? Also, is it still a compile warning in Scala 3, or will it turn into a compile error?

project/SparkBuild.scala

core/src/main/scala/org/apache/spark/resource/ResourceInformation.scala

core/src/main/scala/org/apache/spark/executor/CoarseGrainedExecutorBackend.scala

LuciferYang · 2023-11-04T04:29:25Z

core/src/main/scala/org/apache/spark/deploy/FaultToleranceTest.scala

@@ -340,7 +341,7 @@ private object FaultToleranceTest extends App with Logging {
 private class TestMasterInfo(val ip: String, val dockerId: DockerId, val logFile: File)


FaultToleranceTest is a strange file, it's a test class, but it's in the production directory (so it probably has never been run by GA task). Can we directly delete this file? @srowen @dongjoon-hyun @HyukjinKwon

connector/kafka-0-10-sql/src/main/scala/org/apache/spark/sql/kafka010/JsonUtils.scala

...ftserver/src/test/scala/org/apache/spark/sql/hive/thriftserver/HiveThriftServer2Suites.scala

...e-managers/yarn/src/main/scala/org/apache/spark/scheduler/cluster/YarnSchedulerBackend.scala

LuciferYang · 2023-11-04T04:49:43Z

@laglangyue Additionally, GA's testing has not yet passed, we need to make them all pass

laglangyue · 2023-11-28T03:30:39Z

@LuciferYang there is a CI error，I don't know why.

LuciferYang · 2023-11-28T07:24:25Z

I think we can just retry the failed task

LuciferYang · 2023-11-28T08:16:38Z

Is this one ready to review?

laglangyue · 2023-11-28T11:05:50Z

Is this one ready to review?

yes.

LuciferYang · 2023-11-28T15:19:02Z

core/src/main/scala/org/apache/spark/deploy/StandaloneResourceUtils.scala

@@ -23,7 +23,7 @@ import java.nio.file.Files
 import scala.collection.mutable
 import scala.util.control.NonFatal

-import org.json4s.{DefaultFormats, Extraction}
+import org.json4s._


Suggested change

import org.json4s._

import org.json4s.{DefaultFormats, Extraction, Formats}

hmm... seems like this has been revert again?

LuciferYang · 2023-11-28T15:25:41Z

sql/core/src/main/scala/org/apache/spark/sql/execution/streaming/CompactibleFileStreamLog.scala


  /** Needed to serialize type T into JSON when using Jackson */
+  @nowarn


Suggested change

@nowarn

@scala.annotation.nowarn

When this annotation is only needed in one place in this class, it is customary to use xx directly.

thanks, get it firstly.

LuciferYang · 2023-11-28T15:27:14Z

sql/core/src/main/scala/org/apache/spark/sql/execution/streaming/FileStreamSourceLog.scala

@@ -36,8 +33,8 @@ class FileStreamSourceLog(
    path: String)
  extends CompactibleFileStreamLog[FileEntry](metadataLogVersion, sparkSession, path) {

-  import CompactibleFileStreamLog._
-  import FileStreamSourceLog._
+  import org.apache.spark.sql.execution.streaming.CompactibleFileStreamLog._


Why is it necessary to change to import with the full name here?

maybe changed by Idea. I have reverted

LuciferYang · 2023-11-28T15:27:39Z

sql/core/src/main/scala/org/apache/spark/sql/execution/streaming/HDFSMetadataLog.scala


  /** Needed to serialize type T into JSON when using Jackson */
+  @nowarn


Suggested change

@nowarn

@scala.annotation.nowarn

LuciferYang · 2023-11-28T15:28:03Z

...re/src/main/scala/org/apache/spark/sql/execution/streaming/state/OperatorStateMetadata.scala


+  @nowarn


Suggested change

@nowarn

@scala.annotation.nowarn

LuciferYang

+1, LGTM , pending test

LuciferYang · 2023-11-29T19:06:38Z

@laglangyue Could you modify the description of this PR again? Need to focus on the description of the Why are the changes needed? part.

In addition to fixing a compilation warning in Scala 2.13, you may need to summarize the key points in #43526 (comment), and add it to the pr description so that the changes in this pr can be considered necessary. Note, this is just an example, if you have a more solid reason to describe Why are the changes needed?, that would be better.

dongjoon-hyun

Sorry for being late for this PR. Feel free to proceed to merge, @LuciferYang .

LuciferYang · 2023-12-01T03:52:35Z

Merged into master for Spark 4.0. Thanks @laglangyue and @dongjoon-hyun ~

… `Implicit definition should have explicit type` ### What changes were proposed in this pull request? This PR aims to fix `Implicit definition should have explicit type` in Scala 2.13. This pr includes: 1. Declaration types for global variables of implicit 2. Add scala.annotation.warn ### Why are the changes needed? - For implicit global variables without explicit type declaration, will get warnning : warning: Implicit definition should have explicit type (inferred String) [quickfixable] - No modifications are required for local variables. Additionally, to handle cases involving reflection-related types like ClassTag in implicit variables, the [scala.annotation.warn](https://github.com/scala.annotation.warn) annotation is used to suppress the warning. Furthermore, warnings generated in Spark will be treated as errors: [error] ... Implicit definition should have explicit type (inferred org.json4s.DefaultFormats.type) [quickfixable] ... [error] implicit val formats = org.json4s.DefaultFormats Jira link: SPARK-45314: https://issues.apache.org/jira/browse/SPARK-45629 Related issue link about `implicit` : scala/bug#5265 ### Does this PR introduce _any_ user-facing change? no ### How was this patch tested? Most of the testing is completed through CI, and the example module is locally compiled and tested in IDEA Additionally, there are some writing changes that are verified through demo code ### Was this patch authored or co-authored using generative AI tooling? no Closes apache#43526 from laglangyue/SPARK-45629. Lead-authored-by: tangjiafu <jiafu.tang@qq.com> Co-authored-by: tangjiafu <tangjiafu@corp.netease.com> Signed-off-by: yangjie01 <yangjie01@baidu.com>

github-actions bot added SQL STRUCTURED STREAMING EXAMPLES CORE DSTREAM CONNECT labels Oct 25, 2023

LuciferYang reviewed Oct 25, 2023

View reviewed changes

connector/kafka-0-10-sql/src/main/scala/org/apache/spark/sql/kafka010/JsonUtils.scala Outdated Show resolved Hide resolved

laglangyue force-pushed the SPARK-45629 branch 2 times, most recently from 841a381 to ed749cf Compare October 26, 2023 08:32

github-actions bot removed the CORE label Oct 27, 2023

laglangyue force-pushed the SPARK-45629 branch from efa03f4 to 1701eef Compare October 31, 2023 04:47

github-actions bot removed the DSTREAM label Oct 31, 2023

laglangyue force-pushed the SPARK-45629 branch from 1701eef to a826ec7 Compare November 2, 2023 06:33

github-actions bot added BUILD ML MLLIB WEB UI YARN CORE PYTHON DSTREAM labels Nov 2, 2023

laglangyue changed the title ~~[WIP][SPARK-45629]Fix Implicit definition should have explicit type~~ [SPARK-45629]Fix Implicit definition should have explicit type Nov 3, 2023

LuciferYang reviewed Nov 4, 2023

View reviewed changes

laglangyue force-pushed the SPARK-45629 branch from aa97726 to c140705 Compare November 7, 2023 09:16

github-actions bot added the R label Nov 7, 2023

laglangyue and others added 11 commits November 27, 2023 16:37

wip

7e7bf8e

sql context

cd35fe4

wip: remove some fotmats

bafe785

wip

b655d8c

improve

e60fc0c

improve

efefd0e

style

890ce70

unify format

9114019

wip

8479852

wip

40b7a74

wip

3e391bc

laglangyue force-pushed the SPARK-45629 branch from 09cd042 to 3e391bc Compare November 27, 2023 08:39

tangjiafu added 2 commits November 27, 2023 20:14

reconvert some change

7d2ca49

remove

3f18a3e

LuciferYang reviewed Nov 28, 2023

View reviewed changes

modify as review

995c82e

laglangyue force-pushed the SPARK-45629 branch from 3ea9d4b to 995c82e Compare November 29, 2023 02:30

revert the org.json4s._

7c4bb5d

LuciferYang approved these changes Nov 29, 2023

View reviewed changes

dongjoon-hyun reviewed Nov 30, 2023

View reviewed changes

LuciferYang closed this in a186324 Dec 1, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[SPARK-45629][CORE][SQL][CONNECT][ML][STREAMING][BUILD][EXAMPLES]Fix `Implicit definition should have explicit type` #43526

[SPARK-45629][CORE][SQL][CONNECT][ML][STREAMING][BUILD][EXAMPLES]Fix `Implicit definition should have explicit type` #43526

laglangyue commented Oct 25, 2023 •

edited

laglangyue commented Nov 1, 2023

LuciferYang commented Nov 1, 2023 •

edited

laglangyue commented Nov 3, 2023

laglangyue commented Nov 3, 2023

LuciferYang left a comment

LuciferYang Nov 4, 2023

LuciferYang commented Nov 4, 2023

laglangyue commented Nov 28, 2023

LuciferYang commented Nov 28, 2023

LuciferYang commented Nov 28, 2023

laglangyue commented Nov 28, 2023

LuciferYang Nov 28, 2023

LuciferYang Nov 29, 2023

LuciferYang Nov 28, 2023

LuciferYang Nov 28, 2023

laglangyue Nov 29, 2023

LuciferYang Nov 28, 2023

laglangyue Nov 29, 2023

LuciferYang Nov 28, 2023

LuciferYang Nov 28, 2023

laglangyue Nov 29, 2023

LuciferYang left a comment

LuciferYang commented Nov 29, 2023

dongjoon-hyun left a comment

LuciferYang commented Dec 1, 2023

		@@ -340,7 +341,7 @@ private object FaultToleranceTest extends App with Logging {
		private class TestMasterInfo(val ip: String, val dockerId: DockerId, val logFile: File)

	import org.json4s._
	import org.json4s.{DefaultFormats, Extraction, Formats}


		/** Needed to serialize type T into JSON when using Jackson */
		@nowarn

[SPARK-45629][CORE][SQL][CONNECT][ML][STREAMING][BUILD][EXAMPLES]Fix Implicit definition should have explicit type #43526

[SPARK-45629][CORE][SQL][CONNECT][ML][STREAMING][BUILD][EXAMPLES]Fix Implicit definition should have explicit type #43526

Conversation

laglangyue commented Oct 25, 2023 • edited

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?

Was this patch authored or co-authored using generative AI tooling?

laglangyue commented Nov 1, 2023

LuciferYang commented Nov 1, 2023 • edited

laglangyue commented Nov 3, 2023

laglangyue commented Nov 3, 2023

LuciferYang left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

LuciferYang commented Nov 4, 2023

laglangyue commented Nov 28, 2023

LuciferYang commented Nov 28, 2023

LuciferYang commented Nov 28, 2023

laglangyue commented Nov 28, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

LuciferYang left a comment

Choose a reason for hiding this comment

LuciferYang commented Nov 29, 2023

dongjoon-hyun left a comment

Choose a reason for hiding this comment

LuciferYang commented Dec 1, 2023

[SPARK-45629][CORE][SQL][CONNECT][ML][STREAMING][BUILD][EXAMPLES]Fix `Implicit definition should have explicit type` #43526

[SPARK-45629][CORE][SQL][CONNECT][ML][STREAMING][BUILD][EXAMPLES]Fix `Implicit definition should have explicit type` #43526

laglangyue commented Oct 25, 2023 •

edited

LuciferYang commented Nov 1, 2023 •

edited