You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "LuciferYang (via GitHub)" <gi...@apache.org> on 2023/03/07 12:55:01 UTC

[GitHub] [spark] LuciferYang opened a new pull request, #40317: [SPARK-42700][BUILD] Add `h2` as test dependency of connect-server module

LuciferYang opened a new pull request, #40317:
URL: https://github.com/apache/spark/pull/40317

   ### What changes were proposed in this pull request?
   Run the following commands
   
   ```
   build/mvn clean install -DskipTests -pl connector/connect/server -am
   build/mvn test -pl connector/connect/server
   ```
   
   then we can see the following error message due to [SPARK-42555](https://issues.apache.org/jira/browse/SPARK-42555) A adds the loading `org.h2.Driver` in `beforeAll` of `ProtoToParsedPlanTestSuite`, but 
   
   ```
   ProtoToParsedPlanTestSuite:
   *** RUN ABORTED ***
     java.lang.ClassNotFoundException: org.h2.Driver
     at java.base/java.net.URLClassLoader.findClass(URLClassLoader.java:476)
     at java.base/java.lang.ClassLoader.loadClass(ClassLoader.java:589)
     at java.base/java.lang.ClassLoader.loadClass(ClassLoader.java:522)
     at java.base/java.lang.Class.forName0(Native Method)
     at java.base/java.lang.Class.forName(Class.java:398)
     at org.apache.spark.util.Utils$.classForName(Utils.scala:225)
     at org.apache.spark.sql.connect.ProtoToParsedPlanTestSuite.beforeAll(ProtoToParsedPlanTestSuite.scala:68)
     at org.scalatest.BeforeAndAfterAll.liftedTree1$1(BeforeAndAfterAll.scala:212)
     at org.scalatest.BeforeAndAfterAll.run(BeforeAndAfterAll.scala:210)
     at org.scalatest.BeforeAndAfterAll.run$(BeforeAndAfterAll.scala:208)
     ...
   ```
   
   So this pr add `h2` as test dependency of connect-server module to make maven test pass.
   
    
   
   ### Why are the changes needed?
   Add `h2` as test dependency of connect-server module to make maven test pass.
   
   
   ### Does this PR introduce _any_ user-facing change?
   No
   
   
   ### How was this patch tested?
   manual checkļ¼š
   ```
   build/mvn clean install -DskipTests -pl connector/connect/server -am
   build/mvn test -pl connector/connect/server
   ```
   with this pr, all test passed
   


-- 
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


[GitHub] [spark] LuciferYang commented on pull request #40317: [SPARK-42700][BUILD] Add `h2` as test dependency of connect-server module

Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang commented on PR #40317:
URL: https://github.com/apache/spark/pull/40317#issuecomment-1459646333

   Thanks @HyukjinKwon @hvanhovell @dongjoon-hyun @beliefer 


-- 
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


[GitHub] [spark] LuciferYang commented on pull request #40317: [SPARK-42700][BUILD] Add `h2` as test dependency of connect-server module

Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang commented on PR #40317:
URL: https://github.com/apache/spark/pull/40317#issuecomment-1458123804

   cc @HyukjinKwon fix a maven test failed of connect server module due to dependency loss
   
   


-- 
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


[GitHub] [spark] LuciferYang commented on pull request #40317: [SPARK-42700][BUILD] Add `h2` as test dependency of connect-server module

Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang commented on PR #40317:
URL: https://github.com/apache/spark/pull/40317#issuecomment-1458156395

   > this is the 101st we have broken the maven build in the last month alone. We don't test with it, but we do feel comfortable to release with it. Are we sure the dual build setup is still a thing we want to pursue? Isn't it time to start thinking about greener pastures...
   
   Personally, I think we should consider migrating the build to sbt in Spark 3.5.0, also cc @pan3793 
   
   


-- 
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


[GitHub] [spark] HyukjinKwon closed pull request #40317: [SPARK-42700][BUILD] Add `h2` as test dependency of connect-server module

Posted by "HyukjinKwon (via GitHub)" <gi...@apache.org>.
HyukjinKwon closed pull request #40317: [SPARK-42700][BUILD] Add `h2` as test dependency of connect-server module
URL: https://github.com/apache/spark/pull/40317


-- 
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


[GitHub] [spark] beliefer commented on pull request #40317: [SPARK-42700][BUILD] Add `h2` as test dependency of connect-server module

Posted by "beliefer (via GitHub)" <gi...@apache.org>.
beliefer commented on PR #40317:
URL: https://github.com/apache/spark/pull/40317#issuecomment-1459375825

   @LuciferYang Thank you for the job. https://github.com/apache/spark/pull/40291 need this one.


-- 
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


[GitHub] [spark] dongjoon-hyun commented on pull request #40317: [SPARK-42700][BUILD] Add `h2` as test dependency of connect-server module

Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on PR #40317:
URL: https://github.com/apache/spark/pull/40317#issuecomment-1458723866

   For the following @hvanhovell 's question, I'd ask @srowen 's advice.
   > this is the 101st we have broken the maven build in the last month alone. We don't test with it, but we do feel comfortable to release with it. Are we sure the dual build setup is still a thing we want to pursue? Isn't it time to start thinking about greener pastures...
   
   


-- 
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


[GitHub] [spark] hvanhovell commented on pull request #40317: [SPARK-42700][BUILD] Add `h2` as test dependency of connect-server module

Posted by "hvanhovell (via GitHub)" <gi...@apache.org>.
hvanhovell commented on PR #40317:
URL: https://github.com/apache/spark/pull/40317#issuecomment-1458151326

   @HyukjinKwon @gatorsmile @cloud-fan @dongjoon-hyun this is the 101st we have broken the maven build in the last month alone. We don't test with it, but we do feel comfortable to release with it. Are we sure the dual build setup is still a thing we want to pursue? Isn't it time to start thinking about greener pastures...


-- 
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


[GitHub] [spark] srowen commented on pull request #40317: [SPARK-42700][BUILD] Add `h2` as test dependency of connect-server module

Posted by "srowen (via GitHub)" <gi...@apache.org>.
srowen commented on PR #40317:
URL: https://github.com/apache/spark/pull/40317#issuecomment-1458757289

   While I'm a Maven person myself, I above all have also long preferred one build over two, and an SBT build is fine with me. I actually can't recall if there were strong reasons to keep one or the other in the past; seems like there were Reasons we needed both. We do use mvn in a few places in scripts to parse out stuff about the build, like to determine when new dependencies have been added. mvn and sbt dependency resolution aren't the same, but all the more reason to just pick one, even if we have to migrate some scripts


-- 
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


[GitHub] [spark] LuciferYang commented on pull request #40317: [SPARK-42700][BUILD] Add `h2` as test dependency of connect-server module

Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang commented on PR #40317:
URL: https://github.com/apache/spark/pull/40317#issuecomment-1459157408

   re-triggered GA


-- 
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


[GitHub] [spark] HyukjinKwon commented on pull request #40317: [SPARK-42700][BUILD] Add `h2` as test dependency of connect-server module

Posted by "HyukjinKwon (via GitHub)" <gi...@apache.org>.
HyukjinKwon commented on PR #40317:
URL: https://github.com/apache/spark/pull/40317#issuecomment-1459462492

   Merged to master and branch-3.4.


-- 
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