You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "HyukjinKwon (via GitHub)" <gi...@apache.org> on 2024/05/21 07:25:18 UTC
[PR] [SPARK-48367][CONNECT] Fix lint-scala for scalafmt to detect files to format properly [spark]
HyukjinKwon opened a new pull request, #46679:
URL: https://github.com/apache/spark/pull/46679
### What changes were proposed in this pull request?
Seems like scalafmt upgrade (https://github.com/apache/spark/pull/44845) changed its output format. This PR proposes to fix `./dev/lint-scala` script to detect files format properly.
### Why are the changes needed?
To fix the regression in CI.
### Does this PR introduce _any_ user-facing change?
No, dev-only.
### How was this patch tested?
Manually ran the script.
### Was this patch authored or co-authored using generative AI tooling?
No.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-48367][CONNECT] Fix lint-scala for scalafmt to detect files to format properly [spark]
Posted by "panbingkun (via GitHub)" <gi...@apache.org>.
panbingkun commented on PR #46679:
URL: https://github.com/apache/spark/pull/46679#issuecomment-2122142979
I analyzed it and found that it was not caused by https://github.com/apache/spark/pull/44845, it was caused by this https://github.com/apache/spark/pull/46216
- In version `1.1.1713302731.c3d0074`, its output is:
```
[INFO] --- scalafmt:1.1.1713302731.c3d0074:format (default-cli) @ spark-connect-common_2.13 ---
[INFO] parsed config (v3.7.13): dev/.scalafmt.conf
[ERROR] unformatted file at: /Users/panbingkun/Developer/spark/spark-community/connector/connect/common/src/main/scala/org/apache/spark/sql/connect/ConnectProtoUtils.scala
[INFO] Scalafmt results: 1 of 36 were unformatted
[INFO] Details:
[INFO] - Requires formatting: ConnectProtoUtils.scala
[INFO] - Formatted: UdfUtils.scala
[INFO] - Formatted: DataTypeProtoConverter.scala
[INFO] - Formatted: ConnectCommon.scala
[INFO] - Formatted: ProtoUtils.scala
[INFO] - Formatted: Abbreviator.scala
[INFO] - Formatted: ProtoDataTypes.scala
[INFO] - Formatted: LiteralValueProtoConverter.scala
[INFO] - Formatted: InvalidPlanInput.scala
[INFO] - Formatted: ForeachWriterPacket.scala
[INFO] - Formatted: StreamingListenerPacket.scala
[INFO] - Formatted: StorageLevelProtoConverter.scala
[INFO] - Formatted: UdfPacket.scala
[INFO] - Formatted: ClassFinder.scala
[INFO] - Formatted: SparkConnectClient.scala
[INFO] - Formatted: GrpcRetryHandler.scala
[INFO] - Formatted: GrpcExceptionConverter.scala
[INFO] - Formatted: ArrowEncoderUtils.scala
[INFO] - Formatted: ScalaCollectionUtils.scala
[INFO] - Formatted: ArrowDeserializer.scala
[INFO] - Formatted: ArrowVectorReader.scala
[INFO] - Formatted: ArrowSerializer.scala
[INFO] - Formatted: ConcatenatingArrowStreamReader.scala
[INFO] - Formatted: RetryPolicy.scala
[INFO] - Formatted: SparkConnectStubState.scala
[INFO] - Formatted: ArtifactManager.scala
[INFO] - Formatted: SparkResult.scala
[INFO] - Formatted: RetriesExceeded.scala
[INFO] - Formatted: CloseableIterator.scala
[INFO] - Formatted: package.scala
[INFO] - Formatted: ExecutePlanResponseReattachableIterator.scala
[INFO] - Formatted: ResponseValidator.scala
[INFO] - Formatted: SparkConnectClientParser.scala
[INFO] - Formatted: CustomSparkConnectStub.scala
[INFO] - Formatted: CustomSparkConnectBlockingStub.scala
[INFO] - Formatted: TestUDFs.scala
[ERROR]
org.apache.maven.plugin.MojoExecutionException: Scalafmt: Unformatted files found
at org.antipathy.mvn_scalafmt.FormatMojo.execute (FormatMojo.java:91)
at org.apache.maven.plugin.DefaultBuildPluginManager.executeMojo (DefaultBuildPluginManager.java:126)
at org.apache.maven.lifecycle.internal.MojoExecutor.doExecute2 (MojoExecutor.java:328)
at org.apache.maven.lifecycle.internal.MojoExecutor.doExecute (MojoExecutor.java:316)
at org.apache.maven.lifecycle.internal.MojoExecutor.execute (MojoExecutor.java:212)
at org.apache.maven.lifecycle.internal.MojoExecutor.execute (MojoExecutor.java:174)
at org.apache.maven.lifecycle.internal.MojoExecutor.access$000 (MojoExecutor.java:75)
at org.apache.maven.lifecycle.internal.MojoExecutor$1.run (MojoExecutor.java:162)
at org.apache.maven.plugin.DefaultMojosExecutionStrategy.execute (DefaultMojosExecutionStrategy.java:39)
at org.apache.maven.lifecycle.internal.MojoExecutor.execute (MojoExecutor.java:159)
at org.apache.maven.lifecycle.internal.LifecycleModuleBuilder.buildProject (LifecycleModuleBuilder.java:105)
at org.apache.maven.lifecycle.internal.LifecycleModuleBuilder.buildProject (LifecycleModuleBuilder.java:73)
at org.apache.maven.lifecycle.internal.builder.singlethreaded.SingleThreadedBuilder.build (SingleThreadedBuilder.java:53)
at org.apache.maven.lifecycle.internal.LifecycleStarter.execute (LifecycleStarter.java:118)
at org.apache.maven.DefaultMaven.doExecute (DefaultMaven.java:261)
at org.apache.maven.DefaultMaven.doExecute (DefaultMaven.java:173)
at org.apache.maven.DefaultMaven.execute (DefaultMaven.java:101)
at org.apache.maven.cli.MavenCli.execute (MavenCli.java:906)
at org.apache.maven.cli.MavenCli.doMain (MavenCli.java:283)
at org.apache.maven.cli.MavenCli.main (MavenCli.java:206)
at jdk.internal.reflect.NativeMethodAccessorImpl.invoke0 (Native Method)
at jdk.internal.reflect.NativeMethodAccessorImpl.invoke (NativeMethodAccessorImpl.java:77)
at jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke (DelegatingMethodAccessorImpl.java:43)
at java.lang.reflect.Method.invoke (Method.java:568)
at org.codehaus.plexus.classworlds.launcher.Launcher.launchEnhanced (Launcher.java:283)
at org.codehaus.plexus.classworlds.launcher.Launcher.launch (Launcher.java:226)
at org.codehaus.plexus.classworlds.launcher.Launcher.mainWithExitCode (Launcher.java:407)
at org.codehaus.plexus.classworlds.launcher.Launcher.main (Launcher.java:348)
[INFO] ------------------------------------------------------------------------
[INFO] Reactor Summary for Spark Project Connect Common 4.0.0-SNAPSHOT:
[INFO]
[INFO] Spark Project Connect Common ....................... FAILURE [ 2.371 s]
[INFO] Spark Project Connect Server ....................... SKIPPED
[INFO] Spark Project Connect Client ....................... SKIPPED
[INFO] ------------------------------------------------------------------------
[INFO] BUILD FAILURE
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 2.917 s
[INFO] Finished at: 2024-05-21T16:43:12+08:00
[INFO] ------------------------------------------------------------------------
```
<img width="1199" alt="image" src="https://github.com/apache/spark/assets/15246973/da2da3fc-a1f5-465f-abe8-caedb2ce8e46">
<img width="1397" alt="image" src="https://github.com/apache/spark/assets/15246973/f54f9c39-d4a7-4e1e-aefa-4afa5e518e0b">
- In version `1.1.1684076452.9f83818`, its output is:
```
[INFO] --- scalafmt:1.1.1684076452.9f83818:format (default-cli) @ spark-connect-common_2.13 ---
https://repo1.maven.org/maven2/org/scala-lang/scala-reflect/2.13.11/scala-reflect-2.13.11.jar
100.0% [##########] 3.6 MiB (653.1 KiB / s)
https://repo1.maven.org/maven2/org/scala-lang/scala-library/2.13.11/scala-library-2.13.11.jar
100.0% [##########] 5.7 MiB (1018.3 KiB / s)
[INFO] parsed config (v3.7.13): dev/.scalafmt.conf
[ERROR] unformatted file at: /Users/panbingkun/Developer/spark/spark-community/connector/connect/common/src/main/scala/org/apache/spark/sql/connect/ConnectProtoUtils.scala
[INFO] Scalafmt results: 1 of 36 were unformatted
Details:
Requires formatting: ConnectProtoUtils.scala
Formatted: UdfUtils.scala
Formatted: DataTypeProtoConverter.scala
Formatted: ConnectCommon.scala
Formatted: ProtoUtils.scala
Formatted: Abbreviator.scala
Formatted: ProtoDataTypes.scala
Formatted: LiteralValueProtoConverter.scala
Formatted: InvalidPlanInput.scala
Formatted: ForeachWriterPacket.scala
Formatted: StreamingListenerPacket.scala
Formatted: StorageLevelProtoConverter.scala
Formatted: UdfPacket.scala
Formatted: ClassFinder.scala
Formatted: SparkConnectClient.scala
Formatted: GrpcRetryHandler.scala
Formatted: GrpcExceptionConverter.scala
Formatted: ArrowEncoderUtils.scala
Formatted: ScalaCollectionUtils.scala
Formatted: ArrowDeserializer.scala
Formatted: ArrowVectorReader.scala
Formatted: ArrowSerializer.scala
Formatted: ConcatenatingArrowStreamReader.scala
Formatted: RetryPolicy.scala
Formatted: SparkConnectStubState.scala
Formatted: ArtifactManager.scala
Formatted: SparkResult.scala
Formatted: RetriesExceeded.scala
Formatted: CloseableIterator.scala
Formatted: package.scala
Formatted: ExecutePlanResponseReattachableIterator.scala
Formatted: ResponseValidator.scala
Formatted: SparkConnectClientParser.scala
Formatted: CustomSparkConnectStub.scala
Formatted: CustomSparkConnectBlockingStub.scala
Formatted: TestUDFs.scala
[ERROR]
org.apache.maven.plugin.MojoExecutionException: Scalafmt: Unformatted files found
at org.antipathy.mvn_scalafmt.FormatMojo.execute (FormatMojo.java:91)
at org.apache.maven.plugin.DefaultBuildPluginManager.executeMojo (DefaultBuildPluginManager.java:126)
at org.apache.maven.lifecycle.internal.MojoExecutor.doExecute2 (MojoExecutor.java:328)
at org.apache.maven.lifecycle.internal.MojoExecutor.doExecute (MojoExecutor.java:316)
at org.apache.maven.lifecycle.internal.MojoExecutor.execute (MojoExecutor.java:212)
at org.apache.maven.lifecycle.internal.MojoExecutor.execute (MojoExecutor.java:174)
at org.apache.maven.lifecycle.internal.MojoExecutor.access$000 (MojoExecutor.java:75)
at org.apache.maven.lifecycle.internal.MojoExecutor$1.run (MojoExecutor.java:162)
at org.apache.maven.plugin.DefaultMojosExecutionStrategy.execute (DefaultMojosExecutionStrategy.java:39)
at org.apache.maven.lifecycle.internal.MojoExecutor.execute (MojoExecutor.java:159)
at org.apache.maven.lifecycle.internal.LifecycleModuleBuilder.buildProject (LifecycleModuleBuilder.java:105)
at org.apache.maven.lifecycle.internal.LifecycleModuleBuilder.buildProject (LifecycleModuleBuilder.java:73)
at org.apache.maven.lifecycle.internal.builder.singlethreaded.SingleThreadedBuilder.build (SingleThreadedBuilder.java:53)
at org.apache.maven.lifecycle.internal.LifecycleStarter.execute (LifecycleStarter.java:118)
at org.apache.maven.DefaultMaven.doExecute (DefaultMaven.java:261)
at org.apache.maven.DefaultMaven.doExecute (DefaultMaven.java:173)
at org.apache.maven.DefaultMaven.execute (DefaultMaven.java:101)
at org.apache.maven.cli.MavenCli.execute (MavenCli.java:906)
at org.apache.maven.cli.MavenCli.doMain (MavenCli.java:283)
at org.apache.maven.cli.MavenCli.main (MavenCli.java:206)
at jdk.internal.reflect.NativeMethodAccessorImpl.invoke0 (Native Method)
at jdk.internal.reflect.NativeMethodAccessorImpl.invoke (NativeMethodAccessorImpl.java:77)
at jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke (DelegatingMethodAccessorImpl.java:43)
at java.lang.reflect.Method.invoke (Method.java:568)
at org.codehaus.plexus.classworlds.launcher.Launcher.launchEnhanced (Launcher.java:283)
at org.codehaus.plexus.classworlds.launcher.Launcher.launch (Launcher.java:226)
at org.codehaus.plexus.classworlds.launcher.Launcher.mainWithExitCode (Launcher.java:407)
at org.codehaus.plexus.classworlds.launcher.Launcher.main (Launcher.java:348)
[INFO] ------------------------------------------------------------------------
[INFO] Reactor Summary for Spark Project Connect Common 4.0.0-SNAPSHOT:
[INFO]
[INFO] Spark Project Connect Common ....................... FAILURE [ 8.170 s]
[INFO] Spark Project Connect Server ....................... SKIPPED
[INFO] Spark Project Connect Client ....................... SKIPPED
[INFO] ------------------------------------------------------------------------
[INFO] BUILD FAILURE
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 8.729 s
[INFO] Finished at: 2024-05-21T16:49:04+08:00
[INFO] ------------------------------------------------------------------------
[WARNING] The requested profile "scala-2.13" could not be activated because it does not exist.
[ERROR] Failed to execute goal org.antipathy:mvn-scalafmt_2.13:1.1.1684076452.9f83818:format (default-cli) on project spark-connect-common_2.13: Error formatting Scala files: Scalafmt: Unformatted files found -> [Help 1]
[ERROR]
[ERROR] To see the full stack trace of the errors, re-run Maven with the -e switch.
[ERROR] Re-run Maven using the -X switch to enable full debug logging.
[ERROR]
[ERROR] For more information about the errors and possible solutions, please read the following articles:
[ERROR] [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/MojoExecutionException
```
<img width="360" alt="image" src="https://github.com/apache/spark/assets/15246973/10309d09-a3a3-4466-9746-3c29415c3eac">
<img width="1397" alt="image" src="https://github.com/apache/spark/assets/15246973/de2d89cd-a61b-43f5-86fb-1539f9464ff4">
- Since the above subsequent changes may still cause failure, do we need to
https://github.com/apache/spark/blob/255e7408d95be65682b3fe5af0d5c13b77697282/dev/lint-scala#L35
```
2>&1 | grep -e "Requires formatting" \
```
Change to:
```
2>&1 | grep -e "Unformatted files found" \
```
@HyukjinKwon WDYT?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-48367][CONNECT] Fix lint-scala for scalafmt to detect files to format properly [spark]
Posted by "HyukjinKwon (via GitHub)" <gi...@apache.org>.
HyukjinKwon commented on PR #46679:
URL: https://github.com/apache/spark/pull/46679#issuecomment-2122353063
re: https://github.com/apache/spark/pull/46679#issuecomment-2122142979
Yeah please go ahead.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-48367][CONNECT] Fix lint-scala for scalafmt to detect files to format properly [spark]
Posted by "HyukjinKwon (via GitHub)" <gi...@apache.org>.
HyukjinKwon closed pull request #46679: [SPARK-48367][CONNECT] Fix lint-scala for scalafmt to detect files to format properly
URL: https://github.com/apache/spark/pull/46679
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-48367][CONNECT] Fix lint-scala for scalafmt to detect files to format properly [spark]
Posted by "HyukjinKwon (via GitHub)" <gi...@apache.org>.
HyukjinKwon commented on PR #46679:
URL: https://github.com/apache/spark/pull/46679#issuecomment-2121952499
Merged to master.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-48367][CONNECT] Fix lint-scala for scalafmt to detect files to format properly [spark]
Posted by "panbingkun (via GitHub)" <gi...@apache.org>.
panbingkun commented on PR #46679:
URL: https://github.com/apache/spark/pull/46679#issuecomment-2122360412
> re: [#46679 (comment)](https://github.com/apache/spark/pull/46679#issuecomment-2122142979)
>
> Yeah please go ahead.
Yeah, let me do it, thank you very much! ❤️
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-48367][CONNECT] Fix lint-scala for scalafmt to detect files to format properly [spark]
Posted by "yaooqinn (via GitHub)" <gi...@apache.org>.
yaooqinn commented on code in PR #46679:
URL: https://github.com/apache/spark/pull/46679#discussion_r1608004912
##########
connector/connect/client/jvm/src/main/scala/org/apache/spark/sql/functions.scala:
##########
@@ -7209,8 +7209,8 @@ object functions {
* Returns length of array or map.
*
* This function returns -1 for null input only if spark.sql.ansi.enabled is false and
- * spark.sql.legacy.sizeOfNull is true. Otherwise, it returns null for null input.
- * With the default settings, the function returns null for null input.
+ * spark.sql.legacy.sizeOfNull is true. Otherwise, it returns null for null input. With the
Review Comment:
Why do we make changes here while keeping AS-IS for `org.apache.spark.sql.functions#size`?
##########
connector/connect/client/jvm/src/main/scala/org/apache/spark/sql/functions.scala:
##########
@@ -7687,8 +7687,8 @@ object functions {
* Returns length of array or map. This is an alias of `size` function.
*
* This function returns -1 for null input only if spark.sql.ansi.enabled is false and
- * spark.sql.legacy.sizeOfNull is true. Otherwise, it returns null for null input.
- * With the default settings, the function returns null for null input.
+ * spark.sql.legacy.sizeOfNull is true. Otherwise, it returns null for null input. With the
Review Comment:
ditto
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-48367][CONNECT] Fix lint-scala for scalafmt to detect files to format properly [spark]
Posted by "panbingkun (via GitHub)" <gi...@apache.org>.
panbingkun commented on code in PR #46679:
URL: https://github.com/apache/spark/pull/46679#discussion_r1608112552
##########
connector/connect/client/jvm/src/main/scala/org/apache/spark/sql/functions.scala:
##########
@@ -7209,8 +7209,8 @@ object functions {
* Returns length of array or map.
*
* This function returns -1 for null input only if spark.sql.ansi.enabled is false and
- * spark.sql.legacy.sizeOfNull is true. Otherwise, it returns null for null input.
- * With the default settings, the function returns null for null input.
+ * spark.sql.legacy.sizeOfNull is true. Otherwise, it returns null for null input. With the
Review Comment:
I guess it is the result of executing the following command to format automatically
```
./build/mvn scalafmt:format -Dscalafmt.skip=false -Dscalafmt.validateOnly=false -Dscalafmt.changedOnly=false -pl connector/connect/common -pl connector/connect/server -pl connector/connect/client/jvm
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org