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

   [SPARK-60504][CONNECT] Fix withLocalConnectServer ignoring explicit --remote
   
   ### What changes were proposed in this pull request?
   
   This PR extracts the condition used by `SparkSession.withLocalConnectServer` 
to decide
   whether to spawn an embedded local Spark Connect server into a new method,
   `shouldStartLocalConnectServer`, and fixes a bug in that condition: an 
explicitly
   configured `sc://host:port` remote (via `--remote`, `spark.remote`, or 
`SPARK_REMOTE`)
   was being ignored whenever `spark.api.mode=connect` (or 
`SPARK_CONNECT_MODE=1`) was also
   set, causing the client to silently redirect to a spawned local server 
instead of the
   requested remote endpoint.
   
   `shouldStartLocalConnectServer` now takes the resolved remote string and 
whether
   `spark.api.mode=connect` is in effect, and returns true in exactly two cases:
   
   1. The remote is the local convenience master (e.g. `local`, `local[*]`, 
`local[4]`).
   2. `spark.api.mode=connect` is set and the remote is some other classic 
master (e.g.
      `yarn`, `spark://host:port`, `k8s://...`) rather than an already-explicit 
Spark Connect
      endpoint, letting an existing classic application switch to Connect just 
by setting
      `spark.api.mode=connect`, without changing its master.
   
   A fully-specified `sc://host:port` remote is now never treated as a request 
to start a
   local server, regardless of `isAPIModeConnect`.
   
   ### Why are the changes needed?
   
   Running spark-shell while specifying a remote sc://host:port/ was previously 
broken.
   
   Before this change, `shouldStartLocalConnectServer` only looked at whether 
the remote
   string started with `local`, but `withLocalConnectServer` could resolve 
`remoteString` to
   a classic master (e.g. `spark.master`) when `spark.api.mode=connect` was 
set, including
   when the user had also explicitly set an `sc://` remote. The explicit 
`--remote` /
   `spark.remote` / `SPARK_REMOTE` value was being passed on for connecting, 
but the
   decision of *whether to start a local server* did not account for 
`isAPIModeConnect`,
   so a classic master could be misclassified. This PR makes the decision 
explicit and
   covers it with tests so the explicit `sc://` remote is never overridden by a 
spawned
   local server.
   
   ### Does this PR introduce _any_ user-facing change?
   
   Yes. When `spark.api.mode=connect` (or `SPARK_CONNECT_MODE=1`) is set 
together with an
   explicit `--remote` / `spark.remote` / `SPARK_REMOTE` pointing at 
`sc://host:port`, the
   client now connects to that endpoint as requested instead of silently 
starting and
   connecting to a local Spark Connect server.
   
   ### How was this patch tested?
   
   I ran the old spark-shell passing --remote sc://host:port/ and --conf 
spark.api.mode=connect to verify the broken behaviour
   I then ran the new spark-shell with the same arguments and verified the 
correct connection
   I ran spark shell with both old and new versions passing only --conf 
spark.api.mode=connect to verify a local Connect server was created
   
   ### Was this patch authored or co-authored using generative AI tooling?
   
   Yes, I used an LLM to help me generate the code and the tests.
   


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