Change spark-tensorflow-connector dependency to be spark 3.0.0 preview#141
Change spark-tensorflow-connector dependency to be spark 3.0.0 preview#141WeichenXu123 wants to merge 6 commits intotensorflow:masterfrom
Conversation
|
Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). 📝 Please visit https://cla.developers.google.com/ to sign. Once you've signed (or fixed any issues), please reply here with What to do if you already signed the CLAIndividual signers
Corporate signers
ℹ️ Googlers: Go here for more info. |
| <recompileMode>incremental</recompileMode> | ||
| <useZincServer>true</useZincServer> | ||
| <scalaVersion>${scala.binary.version}</scalaVersion> | ||
| <scalaVersion>${scala.version}</scalaVersion> |
There was a problem hiding this comment.
We need specify scala version "2.12.10" instead of "2.12" here, otherwise it will cause some compatibility issue inside maven scala plugin.
...flow-connector/src/main/scala/org/tensorflow/spark/datasources/tfrecords/DefaultSource.scala
Show resolved
Hide resolved
|
@googlebot I signed it! |
|
CLAs look good, thanks! ℹ️ Googlers: Go here for more info. |
|
@jhseu Could you help review it ? Thanks! |
|
When I try to build this, I'm hitting: It looks like this tries to get a tensorflow-hadoop version which matches the spark-tensorflow-connector version. Is that intentional (given that tensorflow-hadoop is on version 1.14.0, whereas spark-tensorflow-connector is on version 1.10.0)? |
|
@jkbradley The default maven repo only include tensorflow-hadoop version >= 1.11, |
|
Whoops, my bad, did not realize it's in the same project & is a manually handled dependency. Thanks! |
|
Since this project's CI isn't running, I tested this PR locally. It may have some flakiness in the impl or tests right now. I ran the tests once (mvn clean install) and hit the following failure. But then I ran them again (mvn test) & they passed. I ran a 3rd time (mvn clean install) and they passed. Failure in LocalWriteSuite: Also, one nit: the name of the artifact in the pom should be updated to 2.12: spark-tensorflow-connector_2.11 |
|
I'm not opposed to this, but wouldn't it be better to wait until Spark 3.0.0 is released? |
|
@jhseu After we verify correctness, we can keep this PR open so less work for users who want to try out Spark 3.0 preview with spark-tensorflow-connector. |
|
Yeah, I don't mind keeping this open. |
|
@jkbradley Flaky test fixed. You could retest it. And pom artifact is updated to 2_12. |
|
@WeichenXu123 Could you explain the test flakiness? Is it relevant to Spark 3.0 upgrade? If not, let's submit another PR so the fix can go in. |
This reverts commit 0a6c412.
|
@jhseu If we do not plan to make a new release that is 2.4 compatible, shall we review and merge this PR? |
|
Hi, we would like to use this library with spark 2.4 and scala 2.12.10. Would it be possible to support multiple versions with multiple profiles? I should probably create an issue but just wanted to ask here as well. |
|
Now Spark 3.0.0 is released. And we need https://mvnrepository.com/artifact/org.tensorflow/spark-tensorflow-connector_2.12 to be released, I think. |
Change spark-tensorflow-connector to be spark-3.0.0-preview2
Test: