[GitHub] [spark] LuciferYang commented on a diff in pull request #40605: [SPARK-42958][CONNECT] Refactor `connect-jvm-client-mima-check` to support mima check with avro module

[via GitHub]

LuciferYang commented on code in PR #40605:
URL: https://github.com/apache/spark/pull/40605#discussion_r1153013745


##
dev/connect-jvm-client-mima-check:
##
@@ -34,20 +34,18 @@ fi
 
 rm -f .connect-mima-check-result
 
-echo "Build sql module, connect-client-jvm module and connect-client-jvm test 
jar..."
-build/sbt 
"sql/package;connect-client-jvm/assembly;connect-client-jvm/Test/package"
+echo "Build required modules..."
+build/sbt "assembly/package;connect-client-jvm/Test/package"

Review Comment:
   In the future, we need to add mllib, protobuf and other modules, and use 
their classpath at the same time. This will lead to continuous expansion of the 
classpath, so instead, execute `assembly/package` and use the classpath of 
assembly
   
   



-- 
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-unsubscr...@spark.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] [spark] LuciferYang commented on a diff in pull request #40605: [SPARK-42958][CONNECT] Refactor `connect-jvm-client-mima-check` to support mima check with avro module

[via GitHub]

LuciferYang commented on code in PR #40605:
URL: https://github.com/apache/spark/pull/40605#discussion_r1160542658


##
dev/connect-jvm-client-mima-check:
##
@@ -34,20 +34,18 @@ fi
 
 rm -f .connect-mima-check-result
 
-echo "Build sql module, connect-client-jvm module and connect-client-jvm test 
jar..."
-build/sbt 
"sql/package;connect-client-jvm/assembly;connect-client-jvm/Test/package"
+echo "Build required modules..."
+build/sbt 
"sql/package;connect-client-jvm/assembly;connect-client-jvm/Test/package;avro/package"
 
 CONNECT_TEST_CLASSPATH="$(build/sbt -DcopyDependencies=false "export 
connect-client-jvm/Test/fullClasspath" | grep jar | tail -n1)"
-CONNECT_CLASSPATH="$(build/sbt -DcopyDependencies=false "export 
connect-client-jvm/fullClasspath" | grep jar | tail -n1)"
 SQL_CLASSPATH="$(build/sbt -DcopyDependencies=false "export sql/fullClasspath" 
| grep jar | tail -n1)"
 
-
 echo "Do connect-client-jvm module mima check ..."
 
 $JAVA_CMD \
   -Xmx2g \
   -XX:+IgnoreUnrecognizedVMOptions 
--add-opens=java.base/java.util.jar=ALL-UNNAMED \
-  -cp "$CONNECT_CLASSPATH:$CONNECT_TEST_CLASSPATH:$SQL_CLASSPATH" \
+  -cp "$CONNECT_TEST_CLASSPATH:$SQL_CLASSPATH" \

Review Comment:
   @hvanhovell Yes, you are right. Only `checkDatasetApiCompatibility` needs 
`SQL_CLASSPATH ` because it is not a pure mima check.
   
   So we just need `CONNECT_TEST_CLASSPATH` for 
`CheckConnectJvmClientCompatibility ` and `SQL_CLASSPATH` for 
`checkDatasetApiCompatibility`
   
   



-- 
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-unsubscr...@spark.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] [spark] LuciferYang commented on a diff in pull request #40605: [SPARK-42958][CONNECT] Refactor `connect-jvm-client-mima-check` to support mima check with avro module

[via GitHub]

LuciferYang commented on code in PR #40605:
URL: https://github.com/apache/spark/pull/40605#discussion_r1160556535


##
connector/connect/client/jvm/src/test/scala/org/apache/spark/sql/connect/client/CheckConnectJvmClientCompatibility.scala:
##
@@ -62,15 +62,29 @@ object CheckConnectJvmClientCompatibility {
   "spark-connect-client-jvm-assembly",
   "spark-connect-client-jvm")
   val sqlJar: File = findJar("sql/core", "spark-sql", "spark-sql")
-  val problems = checkMiMaCompatibility(clientJar, sqlJar)
-  if (problems.nonEmpty) {
+  val problemsWithSqlModule = 
checkMiMaCompatibilityWithSqlModule(clientJar, sqlJar)
+  if (problemsWithSqlModule.nonEmpty) {
 resultWriter.write(s"ERROR: Comparing client jar: $clientJar and and 
sql jar: $sqlJar \n")
-resultWriter.write(s"problems: \n")
-resultWriter.write(s"${problems.map(p => 
p.description("client")).mkString("\n")}")
+resultWriter.write(s"problemsWithSqlModule: \n")

Review Comment:
   done



-- 
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-unsubscr...@spark.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org