uzadude commented on PR #55: URL: https://github.com/apache/datafu/pull/55#issuecomment-5809629233
Thanks for this! One concern I wanted to check before we update the compatibility table: `build_and_test_spark.sh` recompiles datafu-spark for each Spark version, so the matrix proves *source* compatibility. But we publish a single `datafu-spark_2.12` jar, and Scala code that touches Spark internals (we use Catalyst's `Collect`, `DeclarativeAggregate`, `TypeUtils`, `PythonRunner`, etc.) can compile fine against 3.5 yet fail at runtime when the jar was compiled against an older Spark. So I tested the actual released artifact: - The published [2.1.0 POM](https://repo1.maven.org/maven2/org/apache/datafu/datafu-spark_2.12/2.1.0/datafu-spark_2.12-2.1.0.pom) shows it was compiled against **Spark 3.2.4** (the default `sparkVersion` in `gradle.properties`). - I took this PR's tree, removed `datafu-spark/src/main`, and added the published `datafu-spark_2.12-2.1.0.jar` from Maven Central as a dependency instead. Then I ran `./gradlew :datafu-spark:test -PscalaVersion=2.12 -PsparkVersion=3.5.9` on Java 8. `compileScala` was `NO-SOURCE`, and the test runtime classpath resolved spark-core/sql/catalyst/hive to 3.5.9. - **All 30 tests passed**: `DataFrameOpsTests` (16), `TestAggregators` (7), `UdafTests` (4), `TestScalaPythonBridge` (3). - `TypeUtils.checkForOrderingExpr(DataType, String)`, the internal API I was most worried about, has the same signature in 3.2.4, 3.3.4, 3.4.2 and 3.5.9. Also, `datafu-spark/src` and `datafu-spark/build.gradle` are unchanged since `release-2.1.0-rc0`. So I agree with @eyala that we can extend the **2.1.0** row to `3.0.x - 3.5.x` rather than add a 2.2.0 row (`gradle.properties` is still at 2.1.0 anyway). Caveats: I only ran this against 3.5.9, not 3.5.0–3.5.8, and tests only cover what they exercise. A couple of smaller things: - CI runs with `-q`, so only 3.5.9 was exercised by the new matrix entries. Did you run the full 3.5.0–3.5.8 list locally? - `site/source/docs/spark/getting-started.html.markdown.erb` and `site/source/docs/spark/guide.html.markdown.erb` still say 3.4.2, and `changes.md` could use a DATAFU-182 entry. Separately, and not blocking this PR: it might be worth a follow-up JIRA for a CI job that builds the jar once with the release `sparkVersion` and then runs the tests against the latest patch of each supported minor version using that jar, so the compatibility table reflects binary compatibility of what we actually ship. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
