You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "juliuszsompolski (via GitHub)" <gi...@apache.org> on 2023/08/11 21:18:04 UTC
[GitHub] [spark] juliuszsompolski opened a new pull request, #42461: [CONNECT][POC] Have real server and real simple client in tests - connect-client-jvm-shaded
juliuszsompolski opened a new pull request, #42461:
URL: https://github.com/apache/spark/pull/42461
### What changes were proposed in this pull request?
Try to create a `connect-client-jvm-shaded` so that I could pull a shaded version of the client into connect
### Why are the changes needed?
### Does this PR introduce _any_ user-facing change?
No
### How was this patch tested?
--
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] juliuszsompolski commented on pull request #42461: [CONNECT][POC] Have real server and real simple client in tests - connect-client-jvm-shaded
Posted by "juliuszsompolski (via GitHub)" <gi...@apache.org>.
juliuszsompolski commented on PR #42461:
URL: https://github.com/apache/spark/pull/42461#issuecomment-1675415483
@LuciferYang here I try to create a connect-client-jvm-shaded module that will relocated the client code into org.apache.sparkconnectclient. I'm not sure if maven works, trying now...
But in sbt while I get the shading done in the assembly jar, I can't figure out a way to put the relocated classes into the package jar, or to make the server depend on the assembly jar for testing...
cc @HyukjinKwon @hvanhovell
--
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] juliuszsompolski commented on pull request #42461: [CONNECT][POC] Have real server and real simple client in tests - connect-client-jvm-shaded
Posted by "juliuszsompolski (via GitHub)" <gi...@apache.org>.
juliuszsompolski commented on PR #42461:
URL: https://github.com/apache/spark/pull/42461#issuecomment-1677056265
This approach seems unfeasible. Closing.
--
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] juliuszsompolski closed pull request #42461: [CONNECT][POC] Have real server and real simple client in tests - connect-client-jvm-shaded
Posted by "juliuszsompolski (via GitHub)" <gi...@apache.org>.
juliuszsompolski closed pull request #42461: [CONNECT][POC] Have real server and real simple client in tests - connect-client-jvm-shaded
URL: https://github.com/apache/spark/pull/42461
--
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] juliuszsompolski commented on a diff in pull request #42461: [CONNECT][POC] Have real server and real simple client in tests - connect-client-jvm-shaded
Posted by "juliuszsompolski (via GitHub)" <gi...@apache.org>.
juliuszsompolski commented on code in PR #42461:
URL: https://github.com/apache/spark/pull/42461#discussion_r1291888298
##########
connector/connect/server/pom.xml:
##########
@@ -113,6 +113,12 @@
<version>${project.version}</version>
<scope>provided</scope>
</dependency>
+ <dependency>
+ <groupId>org.apache.spark</groupId>
+ <artifactId>spark-connect-client-jvm-shaded_${scala.binary.version}</artifactId>
+ <version>${project.version}</version>
+ <scope>test</scope>
+ </dependency>
Review Comment:
instead of shading, if I could somehow define in the server that I want to filter spark-connect-client-jvm, and include classes from org.apache.spark.sql.connect, but exclude the rest that could also maybe work...
--
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 #42461: [CONNECT][POC] Have real server and real simple client in tests - connect-client-jvm-shaded
Posted by "HyukjinKwon (via GitHub)" <gi...@apache.org>.
HyukjinKwon commented on PR #42461:
URL: https://github.com/apache/spark/pull/42461#issuecomment-1675535111
cc @LuciferYang too
--
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] juliuszsompolski commented on pull request #42461: [CONNECT][POC] Have real server and real simple client in tests - connect-client-jvm-shaded
Posted by "juliuszsompolski (via GitHub)" <gi...@apache.org>.
juliuszsompolski commented on PR #42461:
URL: https://github.com/apache/spark/pull/42461#issuecomment-1675461873
I can't seem a way to do it. There are ways to shade stuff in the assembly output, but I can't figure out how to include an already shaded dependency at an earlier phase...
--
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 #42461: [CONNECT][POC] Have real server and real simple client in tests - connect-client-jvm-shaded
Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang commented on PR #42461:
URL: https://github.com/apache/spark/pull/42461#issuecomment-1675936370
hmm... If the code moved to `connect-client-jvm-internal` does not involve the user Api part, I prefer to continue modifying https://github.com/apache/spark/pull/42441.
--
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] juliuszsompolski commented on pull request #42461: [CONNECT][POC] Have real server and real simple client in tests - connect-client-jvm-shaded
Posted by "juliuszsompolski (via GitHub)" <gi...@apache.org>.
juliuszsompolski commented on PR #42461:
URL: https://github.com/apache/spark/pull/42461#issuecomment-1675538185
Hm, supposedly Maven is supposed to respect the order of dependencies, so if the connect-jvm is first, it should be picked up first?
https://stackoverflow.com/questions/2612734/maven-multiple-class-with-the-same-path-implemented-in-different-jar
--
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