[#3555] feat(spark-connector): support multi scala version #3608
Conversation
FANNG1
changed the title
| @@ -57,6 +57,7 @@ jobs: | |||
| matrix: | |||
| architecture: [linux/amd64] | |||
| java-version: [ 8, 11, 17 ] | |||
| scala-version: [ 2.12, 2.13 ] | |||
There was a problem hiding this comment.
will change to 2.12 when every thing is ready to merge
There was a problem hiding this comment.
I think 2.12 only should be enough, otherwise, we will have too many CI pipeline.
| @@ -239,6 +239,7 @@ public org.apache.spark.sql.connector.expressions.Transform[] toSparkTransform( | |||
| return sparkTransforms.toArray(new org.apache.spark.sql.connector.expressions.Transform[0]); | |||
| } | |||
|
|
|||
| @SuppressWarnings("deprecation") | |||
There was a problem hiding this comment.
JavaConverters.seqAsJavaList is deprecated in scala 2.13
|
@jerryshao @qqqttt123 please help to review when you have time |
|
Generally LGTM, @FANNG1 can you address one comment I mentioned above. |
| ./gradlew --rerun-tasks -PskipTests -PtestMode=${{ matrix.test-mode }} -PjdkVersion=${{ matrix.java-version }} :spark-connector:spark-3.3:test --tests "com.datastrato.gravitino.spark.connector.integration.test.**" | ||
| ./gradlew --rerun-tasks -PskipTests -PtestMode=${{ matrix.test-mode }} -PjdkVersion=${{ matrix.java-version }} :spark-connector:spark-3.4:test --tests "com.datastrato.gravitino.spark.connector.integration.test.**" | ||
| ./gradlew --rerun-tasks -PskipTests -PtestMode=${{ matrix.test-mode }} -PjdkVersion=${{ matrix.java-version }} :spark-connector:spark-3.5:test --tests "com.datastrato.gravitino.spark.connector.integration.test.**" | ||
| if [ "${{ matrix.scala-version }}" == "2.12" ];then |
There was a problem hiding this comment.
if only support 2.12 in ci, seems unnecessary to add this check.
There was a problem hiding this comment.
I prefer to keep this, maybe we need to support 2.13 latter
|
ci failed for |
settings.gradle.kts
Outdated
| @@ -8,6 +8,10 @@ plugins { | |||
|
|
|||
| rootProject.name = "gravitino" | |||
|
|
|||
| val scalaVersion: String = gradle.startParameter.projectProperties["scalaVersion"]?.toString() | |||
| ?: settings.extra["defaultScalaVersion"]?.toString() | |||
| ?: "2.12" | |||
There was a problem hiding this comment.
Why not just define defaultScalaVersion directly in the settings file, it seems unnecessary to hardcode a 2.12 here.
There was a problem hiding this comment.
defaultScalaVersion is defined in gradle.properties, removed 2.12
|
@caican00 comments are addressed, please help to review again |
LGTM from my side. |
|
@jerryshao do you have any comments? |
I don't have further comment. |
### What changes were proposed in this pull request? support multi scala version ### Why are the changes needed? Fix: #3555 ### Does this PR introduce _any_ user-facing change? no ### How was this patch tested? 1. existing IT and UT 2. in my local machine, tested spark sqls on spark3.4-2.13 and spark3.5-2.13
…che#3608) ### What changes were proposed in this pull request? support multi scala version ### Why are the changes needed? Fix: apache#3555 ### Does this PR introduce _any_ user-facing change? no ### How was this patch tested? 1. existing IT and UT 2. in my local machine, tested spark sqls on spark3.4-2.13 and spark3.5-2.13
What changes were proposed in this pull request?
support multi scala version
Why are the changes needed?
Fix: #3555
Does this PR introduce any user-facing change?
no
How was this patch tested?