[SPARK-48763][CONNECT][BUILD] Move connect server and common to builtin module

[HyukjinKwon]

What changes were proposed in this pull request?

This PR proposes to move the connect server to builtin module.

From:

connector/connect/server
connector/connect/common

To:

connect/server
connect/common

Why are the changes needed?

So the end users do not have to specify --packages when they start the Spark Connect server. Spark Connect client remains as a separate module. This was also pointed out in #39928 (comment).

Does this PR introduce any user-facing change?

Yes, users don't have to specify --packages anymore.

How was this patch tested?

CI in this PR should verify them.
Also manually tested several basic commands such as:

• Maven build

• SBT build

• Running basic Scala client commands

  cd connector/connect
  bin/spark-connect
  bin/spark-connect-scala-client

• Running basic PySpark client commands

  bin/pyspark --remote local

• Connecting to the server launched by ./sbin/start-connect-server.sh

  ./sbin/start-connect-server.sh
   bin/pyspark --remote "sc://localhost"

Was this patch authored or co-authored using generative AI tooling?

No.

[LuciferYang]

spark/dev/sparktestsupport/modules.py

Lines 323 to 334 in 0487d78

	connect = Module(
	name="connect",
	dependencies=[hive, avro, protobuf],
	source_file_regexes=[
	"connector/connect",
	],
	build_profile_flags=["-Pconnect"],
	sbt_test_goals=[
	"connect/test",
	"connect-client-jvm/test",
	],
	)

This place may need to be corrected.

[zhengruifeng]

qq: what about the client module?

[HyukjinKwon]

qq: what about the client module?

That stays as a non-builtin library for now because they cannot coexist with Spark Classic jars

[LuciferYang]

should not delete this line

[LuciferYang]

@HyukjinKwon I have a question. If we move the dependencies defined in the assembly's connect profile to the default dependency list, perhaps we won't need to make such extensive changes? Or is it a rule that the contents under the connector directory should not be packaged by default in the Spark distribution?

[HyukjinKwon]

Yeah we may unpack the shaded dependencies but I was thinking that could be done separately. Or if you meant something else, we could try.

[dongjoon-hyun]

Given the size of this change (2253 files) and the diff of spark-deps-hadoop-3-hive-2.3, I believe we need to be more careful about this change. Maybe, a discussion thread in dev@spark and possibly a vote to get a community bless, @HyukjinKwon .

[HyukjinKwon]

Sure, will do.

[HyukjinKwon]

Sent out [DISCUSS] Move Spark Connect server to builtin package (Client API layer stays external) 👍

[LuciferYang]

So the end users do not have to specify --packages when they start the Spark Connect server.

What I mean is, if we are just trying to address this issue, it would be sufficient to just modify this file, and it appears that there is no need to move the code of the connect server module to a new directory.

[HyukjinKwon]

I will reply in the dev mailing list

[LuciferYang]

Maybe We can add the following exclusions to spark-connect_${scala.binary.version, which will prevent any changes to spark-deps-hadoop-3-hive-2.3, since the current spark-connect module is still a shaded jar and does not require the addition of extra third-party dependencies to the project:

<exclusions>
        <exclusion>
          <groupId>org.apache.spark</groupId>
          <artifactId>spark-connect-common_${scala.binary.version}</artifactId>
        </exclusion>
        <exclusion>
          <groupId>io.grpc</groupId>
          <artifactId>*</artifactId>
        </exclusion>
        <exclusion>
          <groupId>com.google.code.gson</groupId>
          <artifactId>gson</artifactId>
        </exclusion>
        <exclusion>
          <groupId>com.google.guava</groupId>
          <artifactId>failureaccess</artifactId>
        </exclusion>
      </exclusions>

[pan3793]

FYI: we are already good to unify the Guava version in the whole Spark project, see #42493

[LuciferYang]

Since the scope is configured as provided, these two jars should not be included in the packaging result.

[pan3793]

leaving the connect client in a separate folder looks weird. I suppose there is no contract to leave all optional modules under connector? e.g. resource-managers/kubernetes/{docker,integration-tests}, hadoop-cloud. What about moving the whole connect folder to the top level?

[HyukjinKwon]

I actually plan to put the connect client at the top level too but it should be together with SPIP so it is there for now.

[HyukjinKwon]

Vote passed at https://lists.apache.org/thread/7pyfy90nbs8ltqyorpgzpl3cp24xrx8s. Thanks all!

[HyukjinKwon]

Merged to master.

[bjornjorgensen]

I looks like ./dev/make-distribution.sh --name custom-spark --pip --tgz -Pkubernetes does not work after this.
buildx failed with: ERROR: failed to solve: process "/bin/sh -c ./dev/make-distribution.sh --name custom-spark --pip --tgz -Pkubernetes" did not complete successfully: exit code: 1
but without --pip ./dev/make-distribution.sh --name custom-spark --tgz -Pkubernetes that works.

[HyukjinKwon]

Lemme take a look

[HyukjinKwon]

Hm, it works to me. Can you share the exact command and full error message?

[bjornjorgensen]

oh.. my fault there was something else that did not work as intended on my build system.
@HyukjinKwon thanks for double checking.