Re: [PR] [SPARK-56364][BUILD][TESTS] Generate Scala-based test JARs dynamically instead of storing pre-built binaries [spark]

2026-04-06 Thread via GitHub


sarutak commented on PR #55218:
URL: https://github.com/apache/spark/pull/55218#issuecomment-4195920070

   Merged to `master`. Thank you @pan3793 and @HyukjinKwon for reviewing!
   
   > Is Kiro CLI quite good? :-)
   
   It used to have some usability issues, but it’s gradually getting better :D


-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]



Re: [PR] [SPARK-56364][BUILD][TESTS] Generate Scala-based test JARs dynamically instead of storing pre-built binaries [spark]

2026-04-06 Thread via GitHub


sarutak closed pull request #55218: [SPARK-56364][BUILD][TESTS] Generate 
Scala-based test JARs dynamically instead of storing pre-built binaries
URL: https://github.com/apache/spark/pull/55218


-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]



Re: [PR] [SPARK-56364][BUILD][TESTS] Generate Scala-based test JARs dynamically instead of storing pre-built binaries [spark]

2026-04-06 Thread via GitHub


HyukjinKwon commented on PR #55218:
URL: https://github.com/apache/spark/pull/55218#issuecomment-4195364044

   Is Kiro CLI quite good? :-)
   


-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]



Re: [PR] [SPARK-56364][BUILD][TESTS] Generate Scala-based test JARs dynamically instead of storing pre-built binaries [spark]

2026-04-06 Thread via GitHub


sarutak commented on code in PR #55218:
URL: https://github.com/apache/spark/pull/55218#discussion_r3040610287


##
core/src/test/scala/org/apache/spark/executor/ClassLoaderIsolationSuite.scala:
##
@@ -17,37 +17,38 @@
 
 package org.apache.spark.executor
 
-import java.io.File
+import java.io.{File, PrintWriter}
 import java.net.URL
 
-import scala.util.Properties
-
 import org.apache.spark.{JobArtifactSet, JobArtifactState, LocalSparkContext, 
SparkConf, SparkContext, SparkFunSuite, TestUtils}
 import org.apache.spark.util.{MutableURLClassLoader, Utils}
 
 
 class ClassLoaderIsolationSuite extends SparkFunSuite with LocalSparkContext  {
 
-  private val scalaVersion = Properties.versionNumberString
-.split("\\.")
-.take(2)
-.mkString(".")
-
   private val jarURL1 = 
TestUtils.createJarWithClasses(Seq("ClassLoaderIsolation_Dummy"))
   private lazy val jar1 = jarURL1.toString
 
-  // package com.example
-  // object Hello { def test(): Int = 2 }
-  // case class Hello(x: Int, y: Int)

Review Comment:
   If I understand correctly, only `object Hello` is used so I removed `case 
class Hello`. If I'm missing something, please let me know.



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]



Re: [PR] [SPARK-56364][BUILD][TESTS] Generate Scala-based test JARs dynamically instead of storing pre-built binaries [spark]

2026-04-06 Thread via GitHub


sarutak commented on code in PR #55218:
URL: https://github.com/apache/spark/pull/55218#discussion_r3040596425


##
dev/test-jars.txt:
##
@@ -1,10 +1,4 @@
-core/src/test/resources/TestHelloV2_2.13.jar
-core/src/test/resources/TestHelloV3_2.13.jar
 data/artifact-tests/junitLargeJar.jar
 data/artifact-tests/smallJar.jar
-sql/connect/client/jvm/src/test/resources/TestHelloV2_2.13.jar
-sql/connect/client/jvm/src/test/resources/udf2.13.jar
 sql/connect/common/src/test/resources/artifact-tests/junitLargeJar.jar
 sql/connect/common/src/test/resources/artifact-tests/smallJar.jar

Review Comment:
   I'll remove them in other PR like as mentioned in 
[JIRA](https://issues.apache.org/jira/browse/SPARK-56352).
   
   ```
   3. Artifact transfer test files (2 JARs + 7 .class + CRC files in connect 
module)
   ```



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]



Re: [PR] [SPARK-56364][BUILD][TESTS] Generate Scala-based test JARs dynamically instead of storing pre-built binaries [spark]

2026-04-06 Thread via GitHub


pan3793 commented on code in PR #55218:
URL: https://github.com/apache/spark/pull/55218#discussion_r3040516372


##
core/src/test/scala/org/apache/spark/executor/ClassLoaderIsolationSuite.scala:
##
@@ -17,37 +17,38 @@
 
 package org.apache.spark.executor
 
-import java.io.File
+import java.io.{File, PrintWriter}
 import java.net.URL
 
-import scala.util.Properties
-
 import org.apache.spark.{JobArtifactSet, JobArtifactState, LocalSparkContext, 
SparkConf, SparkContext, SparkFunSuite, TestUtils}
 import org.apache.spark.util.{MutableURLClassLoader, Utils}
 
 
 class ClassLoaderIsolationSuite extends SparkFunSuite with LocalSparkContext  {
 
-  private val scalaVersion = Properties.versionNumberString
-.split("\\.")
-.take(2)
-.mkString(".")
-
   private val jarURL1 = 
TestUtils.createJarWithClasses(Seq("ClassLoaderIsolation_Dummy"))
   private lazy val jar1 = jarURL1.toString
 
-  // package com.example
-  // object Hello { def test(): Int = 2 }
-  // case class Hello(x: Int, y: Int)

Review Comment:
   seems `case class Hello(x: Int, y: Int)` is silently removed?



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]



Re: [PR] [SPARK-56364][BUILD][TESTS] Generate Scala-based test JARs dynamically instead of storing pre-built binaries [spark]

2026-04-06 Thread via GitHub


pan3793 commented on code in PR #55218:
URL: https://github.com/apache/spark/pull/55218#discussion_r3040506672


##
dev/test-jars.txt:
##
@@ -1,10 +1,4 @@
-core/src/test/resources/TestHelloV2_2.13.jar
-core/src/test/resources/TestHelloV3_2.13.jar
 data/artifact-tests/junitLargeJar.jar
 data/artifact-tests/smallJar.jar
-sql/connect/client/jvm/src/test/resources/TestHelloV2_2.13.jar
-sql/connect/client/jvm/src/test/resources/udf2.13.jar
 sql/connect/common/src/test/resources/artifact-tests/junitLargeJar.jar
 sql/connect/common/src/test/resources/artifact-tests/smallJar.jar

Review Comment:
   What to do with the remaining ones?



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]