-
Notifications
You must be signed in to change notification settings - Fork 1.9k
[PIO-210] Add support for Scala 2.12; Update dependencies #518
base: develop
Are you sure you want to change the base?
Conversation
@EmergentOrder All tests are being run with Scala 2.11 because you haven't fixed .travis.yml. |
storage/elasticsearch/build.sbt
Outdated
@@ -25,10 +25,10 @@ libraryDependencies ++= Seq( | |||
"org.apache.predictionio" %% "apache-predictionio-core" % version.value % "provided", | |||
"org.apache.spark" %% "spark-core" % sparkVersion.value % "provided", | |||
"org.elasticsearch.client" % "elasticsearch-rest-client" % elasticsearchVersion.value, | |||
"org.elasticsearch" %% "elasticsearch-spark-20" % elasticsearchVersion.value | |||
"org.elasticsearch" % "elasticsearch-spark-20_2.11" % elasticsearchVersion.value |
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.
Does this run on Spark 2.4.3 (Scala 2.12)?
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.
As you pointed out, tests currently only run under 2.11, so I'm not sure. I wanted to ensure the 2.11 build was working first. That's the only 2.11 dep that doesn't have a 2.12 equivalent.
I'll fix .travis.yml now to test under 2.12.
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.
Ahh, too good to be true, of course.
It can't work with the 2.11 version in 2.12, so that is a blocker.
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.
PR is here, so we can pick this back up once they've merged and published:
exclude("org.apache.spark", "*"), | ||
"org.elasticsearch" % "elasticsearch-hadoop-mr" % elasticsearchVersion.value, | ||
"org.specs2" %% "specs2" % "2.3.13" % "test") | ||
"org.specs2" %% "specs2-core" % "2.4.17" % "test") |
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.
Any reason why we choose not the latest version (4.5.1) but 2.4.17?
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.
Tests failed to compile under the latest version.
@@ -33,7 +33,7 @@ libraryDependencies ++= Seq( | |||
exclude("org.mortbay.jetty", "servlet-api-2.5") | |||
exclude("org.mortbay.jetty", "jsp-api-2.1") | |||
exclude("org.mortbay.jetty", "jsp-2.1"), | |||
"org.specs2" %% "specs2" % "2.3.13" % "test") | |||
"org.specs2" %% "specs2-core" % "2.4.17" % "test") |
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.
same as above
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.
same as above
@@ -24,7 +24,7 @@ libraryDependencies ++= Seq( | |||
"org.apache.spark" %% "spark-sql" % sparkVersion.value % "provided", | |||
"org.scalikejdbc" %% "scalikejdbc" % "3.1.0", | |||
"org.postgresql" % "postgresql" % "9.4-1204-jdbc41" % "test", | |||
"org.specs2" %% "specs2" % "2.3.13" % "test") | |||
"org.specs2" %% "specs2-core" % "2.4.17" % "test") |
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.
same as above
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.
same as above
@@ -26,6 +26,6 @@ name := "template-scala-parallel-recommendation" | |||
organization := "org.apache.predictionio" | |||
|
|||
libraryDependencies ++= Seq( | |||
"org.apache.predictionio" %% "apache-predictionio-core" % "0.14.0" % "provided", | |||
"org.apache.predictionio" % "apache-predictionio-core" % "0.15.0-SNAPSHOT" % "provided", |
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.
We need to use the current (local) SNAPSHOT here, otherwise things like 2.12 support will never be able to pass in Travis before being merged.
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.
@shimamoto Do you have any idea why this fails to resolve? It should have been published locally inside docker on make-distribution, no? We need to be testing the current snapshot here.
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.
@EmergentOrder You need to write the double %%
after the groupID.
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.
Ah of course. I removed that and then missed putting it back. Thanks.
Looks like Travis is having some difficulties downloading dependencies... |
No description provided.