[SPARK-49428][SQL] Move Connect Scala Client from Connector to SQL #49695

hvanhovell · 2025-01-27T17:29:03Z

What changes were proposed in this pull request?

This PR moves the connect Scala JVM client project to sql. It also moves the connect/bin and connect/doc to sql.

Why are the changes needed?

Connect is part of the sql project now. It is weird to keep these seperate.

Does this PR introduce any user-facing change?

No.

How was this patch tested?

Existing tests.

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

No.

LuciferYang · 2025-01-28T05:21:34Z

spark/.github/workflows/maven_test.yml

Line 197 in aa24a9a

    
                       ./build/mvn $MAVEN_CLI_OPTS -Dtest.exclude.tags="$EXCLUDED_TAGS" -Djava.version=${JAVA_VERSION/-ea} -pl connector/connect/client/jvm,sql/connect/common,sql/connect/server test -fae

We need to correct some parts of the maven_test.yml file, and at the same time, consider how to ensure compatibility if there are differences in code structure between the master and branch-4.0 now, as this yml file will also be used for the Maven daily test in branch-4.0.

LuciferYang · 2025-01-28T06:20:17Z

spark/assembly/pom.xml

Line 195 in 04f862a

    
           <argument>${basedir}/../connector/connect/client/jvm/target/connect-repl</argument>

spark/assembly/pom.xml

Line 209 in 04f862a

    
           <argument>${basedir}/../connector/connect/client/jvm/target/spark-connect-client-jvm_${scala.binary.version}-${project.version}.jar</argument>

The configuration in the assembly/pom.xml also needs to be fixed, otherwise the following error will occur during the build process:

[INFO] --- exec:3.2.0:exec (copy-connect-client-repl-jars) @ spark-assembly_2.13 ---
cp: /Users/yangjie01/SourceCode/git/spark-mine-13/assembly/../connector/connect/client/jvm/target/connect-repl: No such file or directory
[ERROR] Command execution failed.
org.apache.commons.exec.ExecuteException: Process exited with an error: 1 (Exit value: 1)
    at org.apache.commons.exec.DefaultExecutor.executeInternal (DefaultExecutor.java:355)
    at org.apache.commons.exec.DefaultExecutor.execute (DefaultExecutor.java:253)
    at org.codehaus.mojo.exec.ExecMojo.executeCommandLine (ExecMojo.java:884)
    at org.codehaus.mojo.exec.ExecMojo.executeCommandLine (ExecMojo.java:844)
    at org.codehaus.mojo.exec.ExecMojo.execute (ExecMojo.java:450)
    at org.apache.maven.plugin.DefaultBuildPluginManager.executeMojo (DefaultBuildPluginManager.java:126)
    at org.apache.maven.lifecycle.internal.MojoExecutor.doExecute2 (MojoExecutor.java:328)
    at org.apache.maven.lifecycle.internal.MojoExecutor.doExecute (MojoExecutor.java:316)
    at org.apache.maven.lifecycle.internal.MojoExecutor.execute (MojoExecutor.java:212)
    at org.apache.maven.lifecycle.internal.MojoExecutor.execute (MojoExecutor.java:174)
    at org.apache.maven.lifecycle.internal.MojoExecutor.access$000 (MojoExecutor.java:75)
    at org.apache.maven.lifecycle.internal.MojoExecutor$1.run (MojoExecutor.java:162)
    at org.apache.maven.plugin.DefaultMojosExecutionStrategy.execute (DefaultMojosExecutionStrategy.java:39)
    at org.apache.maven.lifecycle.internal.MojoExecutor.execute (MojoExecutor.java:159)
    at org.apache.maven.lifecycle.internal.LifecycleModuleBuilder.buildProject (LifecycleModuleBuilder.java:105)
    at org.apache.maven.lifecycle.internal.LifecycleModuleBuilder.buildProject (LifecycleModuleBuilder.java:73)
    at org.apache.maven.lifecycle.internal.builder.singlethreaded.SingleThreadedBuilder.build (SingleThreadedBuilder.java:53)
    at org.apache.maven.lifecycle.internal.LifecycleStarter.execute (LifecycleStarter.java:118)
    at org.apache.maven.DefaultMaven.doExecute (DefaultMaven.java:261)
    at org.apache.maven.DefaultMaven.doExecute (DefaultMaven.java:173)
    at org.apache.maven.DefaultMaven.execute (DefaultMaven.java:101)
    at org.apache.maven.cli.MavenCli.execute (MavenCli.java:906)
    at org.apache.maven.cli.MavenCli.doMain (MavenCli.java:283)
    at org.apache.maven.cli.MavenCli.main (MavenCli.java:206)
    at jdk.internal.reflect.DirectMethodHandleAccessor.invoke (DirectMethodHandleAccessor.java:103)
    at java.lang.reflect.Method.invoke (Method.java:580)
    at org.codehaus.plexus.classworlds.launcher.Launcher.launchEnhanced (Launcher.java:255)
    at org.codehaus.plexus.classworlds.launcher.Launcher.launch (Launcher.java:201)
    at org.codehaus.plexus.classworlds.launcher.Launcher.mainWithExitCode (Launcher.java:361)
    at org.codehaus.plexus.classworlds.launcher.Launcher.main (Launcher.java:314)

hvanhovell · 2025-01-29T01:29:21Z

@LuciferYang yeah I was afraid of that. Thanks for the heads-up!

hvanhovell · 2025-01-29T14:01:54Z

@LuciferYang do you think this is good to go? I will port this to 4.0 as well.

LuciferYang · 2025-01-29T14:14:49Z

Let me double-check that

LuciferYang · 2025-01-29T14:29:23Z

There may still be some code that needs to be fixed:

spark/sql/connect/server/README.md

Line 4 in 6bbfa2d

    
           See [Spark Connect Client](https://github.com/apache/spark/tree/master/connector/connect) directory

spark/sql/connect/common/README.md

Line 4 in 6bbfa2d

    
           See [Spark Connect Client](https://github.com/apache/spark/tree/master/connector/connect) directory

spark/docs/spark-connect-overview.md

Line 250 in 6bbfa2d

    
           [reference](https://github.com/apache/spark/blob/master/connector/connect/docs/client-connection-string.md).

spark/docs/_plugins/build_api_docs.rb

Lines 149 to 156 in 6bbfa2d

    
           # Copy over the unified ScalaDoc for all projects to api/scala. 
        
           # This directory will be copied over to _site when `jekyll` command is run. 
        
           copy_and_update_scala_docs("../target/scala-2.13/unidoc", "api/scala") 
        
           # copy_and_update_scala_docs("../connector/connect/client/jvm/target/scala-2.13/unidoc", "api/connect/scala") 
        
           # Copy over the unified JavaDoc for all projects to api/java. 
        
           copy_and_update_java_docs("../target/javaunidoc", "api/java", "api/scala") 
        
           # copy_and_update_java_docs("../connector/connect/client/jvm/target/javaunidoc", "api/connect/java", "api/connect/scala")

spark/dev/lint-scala

Lines 37 to 44 in 6bbfa2d

    
               -pl connector/connect/client/jvm \ 
        
               2>&1 | grep -e "Unformatted files found" \ 
        
           ) 
        
           if test ! -z "$ERRORS"; then 
        
             echo -e "The scalafmt check failed on sql/connect or connector/connect at following occurrences:\n\n$ERRORS\n" 
        
             echo "Before submitting your change, please make sure to format your code using the following command:" 
        
             echo "./build/mvn scalafmt:format -Dscalafmt.skip=false -Dscalafmt.validateOnly=false -Dscalafmt.changedOnly=false -pl sql/api -pl sql/connect/common -pl sql/connect/server -pl connector/connect/client/jvm"

spark/.github/labeler.yml

Lines 222 to 227 in 6bbfa2d

    
           CONNECT: 
        
             - changed-files: 
        
               - any-glob-to-any-file: [ 
        
                'sql/connect/**/*', 
        
                'connector/connect/**/*', 
        
                'python/**/connect/**/*'

spark/dev/protobuf-breaking-changes-check.sh

Lines 36 to 39 in 6bbfa2d

    
           pushd sql/connect/common/src/main && 
        
           echo "Start protobuf breaking changes checking against $BRANCH" && 
        
           buf breaking --against "https://github.com/apache/spark.git#branch=$BRANCH,subdir=connector/connect/common/src/main" && 
        
           echo "Finsh protobuf breaking changes checking: SUCCESS"

spark/dev/sparktestsupport/modules.py

Lines 332 to 343 in 6bbfa2d

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

@hvanhovell Could you please review the above file again? Of course, I don't object to fixing some of them in separate pull requests.

LuciferYang · 2025-01-29T14:31:04Z

There may still be some code that needs to be fixed:

spark/sql/connect/server/README.md

Line 4 in 6bbfa2d

See [Spark Connect Client](https://github.com/apache/spark/tree/master/connector/connect) directory

spark/sql/connect/common/README.md

Line 4 in 6bbfa2d

See [Spark Connect Client](https://github.com/apache/spark/tree/master/connector/connect) directory

spark/docs/spark-connect-overview.md

Line 250 in 6bbfa2d

[reference](https://github.com/apache/spark/blob/master/connector/connect/docs/client-connection-string.md).

spark/docs/_plugins/build_api_docs.rb

Lines 149 to 156 in 6bbfa2d

# Copy over the unified ScalaDoc for all projects to api/scala.

# This directory will be copied over to _site when `jekyll` command is run.

copy_and_update_scala_docs("../target/scala-2.13/unidoc", "api/scala")

# copy_and_update_scala_docs("../connector/connect/client/jvm/target/scala-2.13/unidoc", "api/connect/scala")

# Copy over the unified JavaDoc for all projects to api/java.

copy_and_update_java_docs("../target/javaunidoc", "api/java", "api/scala")

# copy_and_update_java_docs("../connector/connect/client/jvm/target/javaunidoc", "api/connect/java", "api/connect/scala")

spark/dev/lint-scala

Lines 37 to 44 in 6bbfa2d

-pl connector/connect/client/jvm \

2>&1 | grep -e "Unformatted files found" \

)

if test ! -z "$ERRORS"; then

echo -e "The scalafmt check failed on sql/connect or connector/connect at following occurrences:\n\n$ERRORS\n"

echo "Before submitting your change, please make sure to format your code using the following command:"

echo "./build/mvn scalafmt:format -Dscalafmt.skip=false -Dscalafmt.validateOnly=false -Dscalafmt.changedOnly=false -pl sql/api -pl sql/connect/common -pl sql/connect/server -pl connector/connect/client/jvm"

spark/.github/labeler.yml

Lines 222 to 227 in 6bbfa2d

CONNECT:

- changed-files:

- any-glob-to-any-file: [

'sql/connect/**/*',

'connector/connect/**/*',

'python/**/connect/**/*'

spark/dev/protobuf-breaking-changes-check.sh

Lines 36 to 39 in 6bbfa2d

pushd sql/connect/common/src/main &&

echo "Start protobuf breaking changes checking against $BRANCH" &&

buf breaking --against "https://github.com/apache/spark.git#branch=$BRANCH,subdir=connector/connect/common/src/main" &&

echo "Finsh protobuf breaking changes checking: SUCCESS"

spark/dev/sparktestsupport/modules.py

Lines 332 to 343 in 6bbfa2d

connect = Module(

name="connect",

dependencies=[hive, avro, protobuf],

source_file_regexes=[

"sql/connect",

"connector/connect",

],

sbt_test_goals=[

"connect/test",

"connect-client-jvm/test",

],

)

@hvanhovell Could you please review the above file again? Of course, I don't object to fixing some of them in separate pull requests.

It looks like you've fixed them in the latest commit :), let's wait for the latest CI results.

LuciferYang · 2025-01-29T14:35:45Z

dev/protobuf-breaking-changes-check.sh

@@ -35,7 +35,7 @@ fi

 pushd sql/connect/common/src/main &&
 echo "Start protobuf breaking changes checking against $BRANCH" &&
-buf breaking --against "https://github.com/apache/spark.git#branch=$BRANCH,subdir=connector/connect/common/src/main" &&
+buf breaking --against "https://github.com/apache/spark.git#branch=$BRANCH,subdir=sql/connect/common/src/main" &&


Should we perform a branch version check here? If BRANCH is branch-3.x, should we keep comparing it with connector/connect/common/src/main ?

Do we run that against 3.5/3.4?

branch-4.0 will against branch-3.5, right?

Where do we run that? I grepped through to the code base and I can't find us using it.

It was used in build_and_test.yml, but no more.

@HyukjinKwon is this a dead file?

It shouldn't be used in GitHub Actions, because GitHub Actions uses the following configuration:

spark/.github/workflows/build_and_test.yml

Lines 752 to 756 in 6bbfa2d

- name: Breaking change detection against branch-4.0

uses: bufbuild/buf-breaking-action@v1

with:

input: sql/connect/common/src/main

against: 'https://github.com/apache/spark.git#branch=branch-4.0,subdir=sql/connect/common/src/main'

Judging from the commit history, this seems more like a tool for local use by developers? It would be best to have @grundprinzip, @HyukjinKwon , or @zhengruifeng , who have modified this file, confirm it.

LuciferYang

LGTM
Thanks @hvanhovell

I have locally checked the Maven compilation and tested the three modules related to Connect, including connect-common, connect, and connect-client-jvm, and they are all ok.

hvanhovell added 3 commits January 27, 2025 13:12

Move connect/jvm/client into sql

604652f

Fix imports

e4e574d

Fix imports

11a90cd

github-actions bot added SQL STRUCTURED STREAMING BUILD DOCS CONNECT labels Jan 27, 2025

hvanhovell changed the title ~~Spark 49428~~ [SPARK-49428][SQL] Move Connect Scala Client from Connector to SQL Jan 27, 2025

hvanhovell requested a review from HyukjinKwon January 27, 2025 19:52

HyukjinKwon approved these changes Jan 28, 2025

View reviewed changes

Merge remote-tracking branch 'apache/master' into SPARK-49428

748f3da

hvanhovell added 2 commits January 28, 2025 21:32

Fix assembly

1696fea

Fix build yml...

93f2afb

github-actions bot added the INFRA label Jan 29, 2025

One more location

265a794

fix more locations

248fb93

LuciferYang reviewed Jan 29, 2025

View reviewed changes

LuciferYang approved these changes Jan 29, 2025

View reviewed changes

hvanhovell added 2 commits January 30, 2025 13:09

Merge remote-tracking branch 'apache/master' into SPARK-49428

2fde7e1

last one?

4d96960

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[SPARK-49428][SQL] Move Connect Scala Client from Connector to SQL #49695

[SPARK-49428][SQL] Move Connect Scala Client from Connector to SQL #49695

hvanhovell commented Jan 27, 2025

LuciferYang commented Jan 28, 2025 •

edited

Loading

LuciferYang commented Jan 28, 2025 •

edited

Loading

hvanhovell commented Jan 29, 2025

hvanhovell commented Jan 29, 2025

LuciferYang commented Jan 29, 2025

LuciferYang commented Jan 29, 2025

LuciferYang commented Jan 29, 2025

LuciferYang Jan 29, 2025

hvanhovell Jan 29, 2025

LuciferYang Jan 29, 2025 •

edited

Loading

hvanhovell Jan 29, 2025

hvanhovell Jan 29, 2025

LuciferYang Jan 29, 2025 •

edited

Loading

LuciferYang left a comment •

edited

Loading

	- name: Breaking change detection against branch-4.0
	uses: bufbuild/buf-breaking-action@v1
	with:
	input: sql/connect/common/src/main
	against: 'https://github.com/apache/spark.git#branch=branch-4.0,subdir=sql/connect/common/src/main'

[SPARK-49428][SQL] Move Connect Scala Client from Connector to SQL #49695

Are you sure you want to change the base?

[SPARK-49428][SQL] Move Connect Scala Client from Connector to SQL #49695

Conversation

hvanhovell commented Jan 27, 2025

What changes were proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user-facing change?

How was this patch tested?

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

LuciferYang commented Jan 28, 2025 • edited Loading

LuciferYang commented Jan 28, 2025 • edited Loading

hvanhovell commented Jan 29, 2025

hvanhovell commented Jan 29, 2025

LuciferYang commented Jan 29, 2025

LuciferYang commented Jan 29, 2025

LuciferYang commented Jan 29, 2025

LuciferYang Jan 29, 2025

Choose a reason for hiding this comment

hvanhovell Jan 29, 2025

Choose a reason for hiding this comment

LuciferYang Jan 29, 2025 • edited Loading

Choose a reason for hiding this comment

hvanhovell Jan 29, 2025

Choose a reason for hiding this comment

hvanhovell Jan 29, 2025

Choose a reason for hiding this comment

LuciferYang Jan 29, 2025 • edited Loading

Choose a reason for hiding this comment

LuciferYang left a comment • edited Loading

Choose a reason for hiding this comment

LuciferYang commented Jan 28, 2025 •

edited

Loading

LuciferYang commented Jan 28, 2025 •

edited

Loading

LuciferYang Jan 29, 2025 •

edited

Loading

LuciferYang Jan 29, 2025 •

edited

Loading

LuciferYang left a comment •

edited

Loading