-
Notifications
You must be signed in to change notification settings - Fork 3k
Flink: Support configurable Scala version. #4157
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -17,18 +17,11 @@ | |
| * under the License. | ||
| */ | ||
|
|
||
| def flinkProjects = [ | ||
| project(':iceberg-flink:iceberg-flink-1.14'), | ||
| project(':iceberg-flink:iceberg-flink-runtime-1.14') | ||
| ] | ||
|
|
||
| configure(flinkProjects) { | ||
| project.ext { | ||
| flinkVersion = '1.14.0' | ||
| } | ||
| } | ||
| String flinkVersion = '1.14.0' | ||
| String flinkMajorVersion = '1.14' | ||
| String scalaVersion = System.getProperty("scalaVersion") != null ? System.getProperty("scalaVersion") : System.getProperty("defaultScalaVersion") | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We’ve recently also had work for supporting Scala 2.13 in Spark, so it might be wise to name
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As mentioned, Spark is also using Very interested in people’s thoughts on how to handle this concern of
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good question, I also considered this issue before. The current available scala version for flink are So in fact, the supported scala version ranges are quite different between spark & flink. Besides, the supported scala version ranges are also different between different spark versions. Let's see the following table:
So even if we separate the scala version between spark & flink, the same scala version still could not be applied to build all the iceberg spark runtime jars I think. The correct approach is: try to package related artifacts in one build process as much as possible. If we can't build in one build, we can only choose to build again.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since there can only be one Scala version in the build, I think using the same |
||
|
|
||
| project(':iceberg-flink:iceberg-flink-1.14') { | ||
| project(":iceberg-flink:iceberg-flink-${flinkMajorVersion}_${scalaVersion}") { | ||
|
|
||
| dependencies { | ||
| implementation project(path: ':iceberg-bundled-guava', configuration: 'shadow') | ||
|
|
@@ -40,10 +33,10 @@ project(':iceberg-flink:iceberg-flink-1.14') { | |
| implementation project(':iceberg-parquet') | ||
| implementation project(':iceberg-hive-metastore') | ||
|
|
||
| compileOnly "org.apache.flink:flink-streaming-java_2.12:${flinkVersion}" | ||
| compileOnly "org.apache.flink:flink-streaming-java_2.12:${flinkVersion}:tests" | ||
| compileOnly "org.apache.flink:flink-table-api-java-bridge_2.12:${flinkVersion}" | ||
| compileOnly "org.apache.flink:flink-table-planner_2.12:${flinkVersion}" | ||
| compileOnly "org.apache.flink:flink-streaming-java_${scalaVersion}:${flinkVersion}" | ||
| compileOnly "org.apache.flink:flink-streaming-java_${scalaVersion}:${flinkVersion}:tests" | ||
| compileOnly "org.apache.flink:flink-table-api-java-bridge_${scalaVersion}:${flinkVersion}" | ||
| compileOnly "org.apache.flink:flink-table-planner_${scalaVersion}:${flinkVersion}" | ||
| compileOnly "org.apache.hadoop:hadoop-hdfs" | ||
| compileOnly "org.apache.hadoop:hadoop-common" | ||
| compileOnly("org.apache.hadoop:hadoop-minicluster") { | ||
|
|
@@ -72,7 +65,7 @@ project(':iceberg-flink:iceberg-flink-1.14') { | |
| testImplementation ("org.apache.flink:flink-test-utils-junit:${flinkVersion}") { | ||
| exclude group: 'junit' | ||
| } | ||
| testImplementation("org.apache.flink:flink-test-utils_2.12:${flinkVersion}") { | ||
| testImplementation("org.apache.flink:flink-test-utils_${scalaVersion}:${flinkVersion}") { | ||
| exclude group: "org.apache.curator", module: 'curator-test' | ||
| exclude group: 'junit' | ||
| } | ||
|
|
@@ -116,7 +109,7 @@ project(':iceberg-flink:iceberg-flink-1.14') { | |
| } | ||
| } | ||
|
|
||
| project(':iceberg-flink:iceberg-flink-runtime-1.14') { | ||
| project(":iceberg-flink:iceberg-flink-runtime-${flinkMajorVersion}_${scalaVersion}") { | ||
| apply plugin: 'com.github.johnrengelman.shadow' | ||
|
|
||
| tasks.jar.dependsOn tasks.shadowJar | ||
|
|
@@ -136,7 +129,7 @@ project(':iceberg-flink:iceberg-flink-runtime-1.14') { | |
| } | ||
|
|
||
| dependencies { | ||
| implementation project(':iceberg-flink:iceberg-flink-1.14') | ||
| implementation project(":iceberg-flink:iceberg-flink-${flinkMajorVersion}_${scalaVersion}") | ||
| implementation project(':iceberg-aws') | ||
| implementation(project(':iceberg-aliyun')) { | ||
| exclude group: 'edu.umd.cs.findbugs', module: 'findbugs' | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -87,30 +87,30 @@ if (!flinkVersions.isEmpty()) { | |
| } | ||
|
|
||
| if (flinkVersions.contains("1.12")) { | ||
| include ':iceberg-flink:flink-1.12' | ||
| include ':iceberg-flink:flink-runtime-1.12' | ||
| project(':iceberg-flink:flink-1.12').projectDir = file('flink/v1.12/flink') | ||
| project(':iceberg-flink:flink-1.12').name = 'iceberg-flink-1.12' | ||
| project(':iceberg-flink:flink-runtime-1.12').projectDir = file('flink/v1.12/flink-runtime') | ||
| project(':iceberg-flink:flink-runtime-1.12').name = 'iceberg-flink-runtime-1.12' | ||
| include ":iceberg-flink:flink-1.12_${scalaVersion}" | ||
| include ":iceberg-flink:flink-runtime-1.12_${scalaVersion}" | ||
| project(":iceberg-flink:flink-1.12_${scalaVersion}").projectDir = file('flink/v1.12/flink') | ||
| project(":iceberg-flink:flink-1.12_${scalaVersion}").name = "iceberg-flink-1.12_${scalaVersion}" | ||
| project(":iceberg-flink:flink-runtime-1.12_${scalaVersion}").projectDir = file('flink/v1.12/flink-runtime') | ||
| project(":iceberg-flink:flink-runtime-1.12_${scalaVersion}").name = "iceberg-flink-runtime-1.12_${scalaVersion}" | ||
| } | ||
|
|
||
| if (flinkVersions.contains("1.13")) { | ||
| include ':iceberg-flink:flink-1.13' | ||
| include ':iceberg-flink:flink-runtime-1.13' | ||
| project(':iceberg-flink:flink-1.13').projectDir = file('flink/v1.13/flink') | ||
| project(':iceberg-flink:flink-1.13').name = 'iceberg-flink-1.13' | ||
| project(':iceberg-flink:flink-runtime-1.13').projectDir = file('flink/v1.13/flink-runtime') | ||
| project(':iceberg-flink:flink-runtime-1.13').name = 'iceberg-flink-runtime-1.13' | ||
| include ":iceberg-flink:flink-1.13_${scalaVersion}" | ||
| include ":iceberg-flink:flink-runtime-1.13_${scalaVersion}" | ||
| project(":iceberg-flink:flink-1.13_${scalaVersion}").projectDir = file('flink/v1.13/flink') | ||
| project(":iceberg-flink:flink-1.13_${scalaVersion}").name = "iceberg-flink-1.13_${scalaVersion}" | ||
| project(":iceberg-flink:flink-runtime-1.13_${scalaVersion}").projectDir = file('flink/v1.13/flink-runtime') | ||
| project(":iceberg-flink:flink-runtime-1.13_${scalaVersion}").name = "iceberg-flink-runtime-1.13_${scalaVersion}" | ||
| } | ||
|
|
||
| if (flinkVersions.contains("1.14")) { | ||
| include ':iceberg-flink:flink-1.14' | ||
| include ':iceberg-flink:flink-runtime-1.14' | ||
| project(':iceberg-flink:flink-1.14').projectDir = file('flink/v1.14/flink') | ||
| project(':iceberg-flink:flink-1.14').name = 'iceberg-flink-1.14' | ||
| project(':iceberg-flink:flink-runtime-1.14').projectDir = file('flink/v1.14/flink-runtime') | ||
| project(':iceberg-flink:flink-runtime-1.14').name = 'iceberg-flink-runtime-1.14' | ||
| include ":iceberg-flink:flink-1.14_${scalaVersion}" | ||
| include ":iceberg-flink:flink-runtime-1.14_${scalaVersion}" | ||
| project(":iceberg-flink:flink-1.14_${scalaVersion}").projectDir = file('flink/v1.14/flink') | ||
| project(":iceberg-flink:flink-1.14_${scalaVersion}").name = "iceberg-flink-1.14_${scalaVersion}" | ||
| project(":iceberg-flink:flink-runtime-1.14_${scalaVersion}").projectDir = file('flink/v1.14/flink-runtime') | ||
| project(":iceberg-flink:flink-runtime-1.14_${scalaVersion}").name = "iceberg-flink-runtime-1.14_${scalaVersion}" | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Correct me if I’m wrong, but I think we need to be able to publish / build all combinations of Flink / Scala versions here. For example, when publishing a new release to maven we need to build all of the artifacts. Is it possible to do that in a way that doesn’t require two build calls (one for each Scala version)? I’m not a highly experienced Scala with gradle user, so I might be mistaken or it might be necessary to use two build calls. But it makes the release process more cumbersome and particular to have to call build multiple times, which is something we’ve been trying to make more streamlined (the build / release process). I’m ok with iterating on things and would not block on this, but it is a concern for me.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Looking at the Spark on Scala 2.13 support PR, it looks like I’m not sure if the release scripts have been updated to support this yet though.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
As far as I know, the (In general, I think it's a good question)
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, we can't have multiple Scala versions in the build without strange errors. We'll have to update the binary release script to publish Scala 2.13 modules. |
||
| } | ||
|
|
||
| if (sparkVersions.contains("3.0")) { | ||
|
|
||
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.
@rdblue @kbendick Seems I made a mistake here ? Since we apache iceberg flink & flink-runtime modules don't introduce any scala dependencies, so in theory the iceberg-flink-1.14.jar & iceberg-flink-runtime-1.14.jar can work fine on both flink-1.14.0-bin-scala_2.11 & flink-1.14.0-bin-scala_2.12 release. So we don't need to add any
_${scalaVersion}suffix foriceberg-flink&iceberg-flink-runtimejar.(But we still need this PR to make iceberg-flink/iceber-flink-runtime compile on different scala 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.
➜ iceberg git:(master) ✗ ./gradlew :iceberg-flink:iceberg-flink-runtime-1.14_2.12:dependencies --configuration runtimeClasspath
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.
PR is here: #4193