New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Feature][Connector-V2][Translation] Support spark 3.3 #2574
Conversation
21424ad
to
b71d350
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add licenses according to Dependency licenses action
.
you can see https://seatunnel.apache.org/docs/contribution/new-license
Can you add e2e from spark3 and spark2? |
ok, I will add it. |
Thanks, I will do it. |
77412ae
to
09a6cca
Compare
a2baef9
to
a308365
Compare
Please help me review it. I only add e2e in spark 3.3 for FakeSourceToConsoleIT, will add more after merge it, because the module change frequently, I fixed conflicts repeatedly. @Hisoka-X |
Hi, Please fix UT |
The fail is related with flink. The pr don't update, can you help me rerun it? |
The fail is strange, I don't how to fix it. @Hisoka-X |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall look good for me, @ashulin Please check source part. Thanks
How user switch spark2.4 and spark3.3 use shell command? |
<dependencyManagement> | ||
<dependencies> | ||
<!--spark--> | ||
<dependency> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
move to spark-2.4 module?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because seatunnel-translation-spark-common depend on spark, if move it, the common module need add it.
@@ -74,7 +74,7 @@ | |||
|
|||
<dependency> | |||
<groupId>org.apache.seatunnel</groupId> | |||
<artifactId>seatunnel-translation-spark-2.4</artifactId> | |||
<artifactId>seatunnel-translation-spark-common</artifactId> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How user switch seatunnel-transform-spark(2.4、3.3) list?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How user switch seatunnel-transform-spark(2.4、3.3) list?
Our release package include them, user choose the jar according to their spark client version.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How to choose? Seem like you should add jar choose logic in seatunnel-spark-starter.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typically, users choose one spark version, so they only use one translation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I prefer a more automated way, such as by judging the version of SPARK_HOME or using SPARK2 by default, you can switch versions by adding parameters to the shell script
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good idea. The arg can control to choose which plugin dir. We can do it in a new pr. How about you
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok for me
According to spark client version, choose the jar. But, we can handle it later, the pr only add spark3.3 translation. There are frequent conflicts if we add more content. |
OK for me |
|
||
@Override | ||
protected String getTranslationJarTargetPath() { | ||
return Paths.get(SEATUNNEL_HOME, "plugins", "translation-spark-3.3", "lib", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you help me review it, merging is blocking, the conflicts are frequent. @ashulin @CalvinKirs |
39f5646
to
82bfaf5
Compare
Can we support all spark e2e run both 3.3 and 2.4 without configure anything? I find we need write same code twice if we want test job both on 3.3 and 2.4. If we do this, and all e2e passed. I think we can merge this PR. |
I only add one test for 3.3, I will fix conflicts later. |
spark version :3.3.1
|
Who can help me look at this problem @JinJiDeJinMu Hi, Support Spark3 not finished at now. But @nishuihanqiu have a branch for spark3 in his repository. #875 (comment) |
Who can help me look at this problem @Hisoka-X thank you |
@Hisoka-X I want to join in the translation of Spark3. x based on v2, What can I do? |
@JinJiDeJinMu Wonderful! You can continue do your work with this PR. Fix conflict and do some test, if have some problem then fix it. |
Purpose of this pull request
Check list
New License Guide