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

   ### What changes were proposed in this pull request?
   
   Currently, Spark Connect has the following types of integration tests:
   * Client unit tests with a mock in process server (DummySparkConnectService 
with GRPC InProcessServerBuilder)
   * Client unit tests with a mock RPC server (DummySparkConnectService with 
GRPC NettyServerBuilder)
   * Server unit tests with in-process server and using various mocks
   * E2E tests of real client with a server started in another process (using 
RemoteSparkSession)
   
   What is lacking are E2E tests with an in-process Server (so that server 
state can be inspected asserted), and a real RPC client. This is impossible, 
because classes from `spark-connect-client-jvm` module include the client API 
which duplicates Spark SQL APIs of SparkSession, Dataset etc. When trying to 
pull a real client into the server module for testing, these classes clash.
   
   Move the `org.apache.spark.sql.connect.client` code into new 
`spark-connect-client-jvm-internal` module, so that the internal 
SparkConnectClient code is separated from the client public API, and can be 
pulled into testing of the server.
   
   Tried alternative approach in https://github.com/apache/spark/pull/42465. 
That doesn't work, because it also reorders the maven build in a way so that 
client is build before server, but client actually requires server to be build 
first to use tests with `RemoteSparkSession`
   Tried alternative approach to depend on a shaded/relocated version of client 
in https://github.com/apache/spark/pull/42461, but that's just not possible to 
do neither in maven nor sbt.
   Tried alternative approach to create client-jvm-internal module in 
https://github.com/apache/spark/pull/42441, moving things to connect-common was 
preferred to introducing new module by reviewers.
   Moved it together with tests in https://github.com/apache/spark/pull/42501, 
but moving tests isn't really needed.
   
   ### Why are the changes needed?
   
   For being able to use the internal client for testing of in-process server 
with an in-process client, but communicating over real RPC.
   
   ### Does this PR introduce _any_ user-facing change?
   
   No.
   
   ### How was this patch tested?
   
   All modules test and build.


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

Reply via email to