ericm-db commented on PR #56907: URL: https://github.com/apache/spark/pull/56907#issuecomment-4918060984
> I am not a Connect expert, but it feels a bit off to be creating the discovery file from Python vs. via [`SparkConnectServer`](https://github.com/apache/spark/blob/8f9385a8b9c07c30b46ac748966ba7304e8b4da9/sql/connect/server/src/main/scala/org/apache/spark/sql/connect/service/SparkConnectServer.scala#L29) (the class that gets run when you call [start-connect-server.sh](https://github.com/apache/spark/blob/03b62c27862a5a592cec3ae46c0231a503a1d83c/sbin/start-connect-server.sh#L31)) or [`SparkConnectService`](https://github.com/apache/spark/blob/3df41f5f211e495103a1b934872a25683637944f/sql/connect/server/src/main/scala/org/apache/spark/sql/connect/service/SparkConnectService.scala#L59). > > Ideally, we want any language client to be able to leverage the Connect server discovery mechanism. Doesn't that mean the write/manage path for this mechanism should live in Scala? @nchammas Agreed on the end state: language-neutral discovery is the right goal. The natural design is SparkConnectService writing the discovery file itself when it binds (behind something like `spark.connect.discovery.file`) and removing it on stop. That gets three things this PR can't: any language client can discover servers, your spark CLI can read the same file, and servers started manually via `sbin/start-connect-server.sh` become discoverable too. Basically the "weaker solution" from the dev thread. I kept the write path in Python here for scope reasons. The opt-in replaces a spawn path that's already Python-owned, and a Python component has to exist regardless, something client-side needs to make the reuse decision, launch the detached server on first use, and run the idle-timeout reaper. Moving the write into the server doesn't remove that piece, it just thins it: Python (and every other client) becomes a pure reader/launcher. My proposal: land this as the Python opt-in, then file a follow-up to move the write/manage path into SparkConnectService. That change touches the server and effectively defines the discovery file as a cross-language contract (format, location, multi-server behavior), so it deserves Connect-reviewer attention and probably belongs alongside the CLI SPIP discussion. Happy to file that JIRA and take it. If you'd rather settle the contract first, I'm open to that too, but I'd rather not grow this PR into a server change. -- 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]
