[GitHub] spark pull request #19307: [SPARK-22087][SPARK-14650][WIP][BUILD][REPL][CORE...

[asfgit]

Github user asfgit closed the pull request at:

https://github.com/apache/spark/pull/19307


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request #19307: [SPARK-22087][SPARK-14650][WIP][BUILD][REPL][CORE...

[srowen]

Github user srowen commented on a diff in the pull request:

https://github.com/apache/spark/pull/19307#discussion_r140210945
  
--- Diff: core/src/main/scala/org/apache/spark/rdd/DoubleRDDFunctions.scala 
---
@@ -128,9 +128,9 @@ class DoubleRDDFunctions(self: RDD[Double]) extends 
Logging with Serializable {
 }
 // Compute the minimum and the maximum
 val (max: Double, min: Double) = self.mapPartitions { items =>
-  Iterator(items.foldRight(Double.NegativeInfinity,
-Double.PositiveInfinity)((e: Double, x: (Double, Double)) =>
-(x._1.max(e), x._2.min(e
+  Iterator(
+items.foldRight((Double.NegativeInfinity, Double.PositiveInfinity)
--- End diff --

More warnings about auto converting 2 args to a single tuple arg


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request #19307: [SPARK-22087][SPARK-14650][WIP][BUILD][REPL][CORE...

[srowen]

Github user srowen commented on a diff in the pull request:

https://github.com/apache/spark/pull/19307#discussion_r140211316
  
--- Diff: 
core/src/test/scala/org/apache/spark/scheduler/SchedulerIntegrationSuite.scala 
---
@@ -625,6 +625,8 @@ class BasicSchedulerIntegrationSuite extends 
SchedulerIntegrationSuite[SingleCor
   backend.taskFailed(taskDescription, fetchFailed)
 case (1, _, partition) =>
   backend.taskSuccess(taskDescription, 42 + partition)
+case unmatched =>
--- End diff --

Many new warnings about a match not being exhaustive; this is one case 
where it was clearly avoidable. I left other instances alone


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request #19307: [SPARK-22087][SPARK-14650][WIP][BUILD][REPL][CORE...

[srowen]

Github user srowen commented on a diff in the pull request:

https://github.com/apache/spark/pull/19307#discussion_r140211172
  
--- Diff: 
core/src/test/scala/org/apache/spark/deploy/SparkSubmitUtilsSuite.scala ---
@@ -83,11 +83,12 @@ class SparkSubmitUtilsSuite extends SparkFunSuite with 
BeforeAndAfterAll {
 val resolver = settings.getDefaultResolver.asInstanceOf[ChainResolver]
 assert(resolver.getResolvers.size() === 4)
 val expected = repos.split(",").map(r => s"$r/")
-resolver.getResolvers.toArray.zipWithIndex.foreach { case (resolver: 
AbstractResolver, i) =>
-  if (1 < i && i < 3) {
-assert(resolver.getName === s"repo-$i")
-assert(resolver.asInstanceOf[IBiblioResolver].getRoot === 
expected(i - 1))
-  }
+
resolver.getResolvers.toArray.map(_.asInstanceOf[AbstractResolver]).zipWithIndex.foreach
 {
--- End diff --

This ends up being interpreted as a partial function, which wasn't the 
intent, and generates a warning. Should be equivalent


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request #19307: [SPARK-22087][SPARK-14650][WIP][BUILD][REPL][CORE...

[srowen]

Github user srowen commented on a diff in the pull request:

https://github.com/apache/spark/pull/19307#discussion_r140210769
  
--- Diff: core/src/main/scala/org/apache/spark/deploy/worker/Worker.scala 
---
@@ -450,10 +450,9 @@ private[deploy] class Worker(
 }
   }(cleanupThreadExecutor)
 
-  cleanupFuture.onFailure {
-case e: Throwable =>
-  logError("App dir cleanup failed: " + e.getMessage, e)
-  }(cleanupThreadExecutor)
+  cleanupFuture.failed.foreach(e =>
--- End diff --

onFailure, onSuccess are deprecated in 2.12. This should be equivalent in 
2.11+2.12


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request #19307: [SPARK-22087][SPARK-14650][WIP][BUILD][REPL][CORE...

[srowen]

Github user srowen commented on a diff in the pull request:

https://github.com/apache/spark/pull/19307#discussion_r140211761
  
--- Diff: 
sql/core/src/test/scala/org/apache/spark/sql/streaming/EnsureStatefulOpPartitioningSuite.scala
 ---
@@ -20,6 +20,7 @@ package org.apache.spark.sql.streaming
 import java.util.UUID
 
 import org.apache.spark.rdd.RDD
+import org.apache.spark.sql.DataFrame
--- End diff --

This whole file fix is already going into master separately as a hotfix


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request #19307: [SPARK-22087][SPARK-14650][WIP][BUILD][REPL][CORE...

[srowen]

Github user srowen commented on a diff in the pull request:

https://github.com/apache/spark/pull/19307#discussion_r140210894
  
--- Diff: core/src/main/scala/org/apache/spark/scheduler/DAGScheduler.scala 
---
@@ -396,12 +396,12 @@ class DAGScheduler(
 
   /** Find ancestor shuffle dependencies that are not registered in 
shuffleToMapStage yet */
   private def getMissingAncestorShuffleDependencies(
-  rdd: RDD[_]): Stack[ShuffleDependency[_, _, _]] = {
-val ancestors = new Stack[ShuffleDependency[_, _, _]]
+  rdd: RDD[_]): ArrayStack[ShuffleDependency[_, _, _]] = {
--- End diff --

Stack is deprecated in 2.12 for poor performance; ArrayStack should work 
the same and be faster, in 2.11 too


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request #19307: [SPARK-22087][SPARK-14650][WIP][BUILD][REPL][CORE...

[srowen]

Github user srowen commented on a diff in the pull request:

https://github.com/apache/spark/pull/19307#discussion_r140210563
  
--- Diff: core/src/main/scala/org/apache/spark/SparkContext.scala ---
@@ -2826,33 +2826,33 @@ object WritableConverter {
   // them automatically. However, we still keep the old functions in 
SparkContext for backward
   // compatibility and forward to the following functions directly.
 
-  implicit def intWritableConverter(): WritableConverter[Int] =
-simpleWritableConverter[Int, IntWritable](_.get)
+  implicit val intWritableConverter: () => WritableConverter[Int] =
+() => simpleWritableConverter[Int, IntWritable](_.get)
 
-  implicit def longWritableConverter(): WritableConverter[Long] =
-simpleWritableConverter[Long, LongWritable](_.get)
+  implicit val longWritableConverter: () => WritableConverter[Long] =
+() => simpleWritableConverter[Long, LongWritable](_.get)
 
-  implicit def doubleWritableConverter(): WritableConverter[Double] =
-simpleWritableConverter[Double, DoubleWritable](_.get)
+  implicit val doubleWritableConverter: () => WritableConverter[Double] =
+() => simpleWritableConverter[Double, DoubleWritable](_.get)
 
-  implicit def floatWritableConverter(): WritableConverter[Float] =
-simpleWritableConverter[Float, FloatWritable](_.get)
+  implicit val floatWritableConverter: () => WritableConverter[Float] =
+() => simpleWritableConverter[Float, FloatWritable](_.get)
 
-  implicit def booleanWritableConverter(): WritableConverter[Boolean] =
-simpleWritableConverter[Boolean, BooleanWritable](_.get)
+  implicit val booleanWritableConverter: () => WritableConverter[Boolean] =
+() => simpleWritableConverter[Boolean, BooleanWritable](_.get)
 
-  implicit def bytesWritableConverter(): WritableConverter[Array[Byte]] = {
-simpleWritableConverter[Array[Byte], BytesWritable] { bw =>
+  implicit val bytesWritableConverter: () => 
WritableConverter[Array[Byte]] = {
+() => simpleWritableConverter[Array[Byte], BytesWritable] { bw =>
   // getBytes method returns array which is longer then data to be 
returned
   Arrays.copyOfRange(bw.getBytes, 0, bw.getLength)
 }
   }
 
-  implicit def stringWritableConverter(): WritableConverter[String] =
-simpleWritableConverter[String, Text](_.toString)
+  implicit val stringWritableConverter: () => WritableConverter[String] =
+() => simpleWritableConverter[String, Text](_.toString)
 
-  implicit def writableWritableConverter[T <: Writable](): 
WritableConverter[T] =
-new WritableConverter[T](_.runtimeClass.asInstanceOf[Class[T]], 
_.asInstanceOf[T])
+  implicit def writableWritableConverter[T <: Writable : ClassTag]: () => 
WritableConverter[T] =
--- End diff --

ClassTag was required here, for reasons I don't fully get


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request #19307: [SPARK-22087][SPARK-14650][WIP][BUILD][REPL][CORE...

[srowen]

Github user srowen commented on a diff in the pull request:

https://github.com/apache/spark/pull/19307#discussion_r140210654
  
--- Diff: 
core/src/main/scala/org/apache/spark/deploy/history/FsHistoryProvider.scala ---
@@ -218,11 +218,13 @@ private[history] class FsHistoryProvider(conf: 
SparkConf, clock: Clock)
 if (!conf.contains("spark.testing")) {
   // A task that periodically checks for event log updates on disk.
   logDebug(s"Scheduling update thread every $UPDATE_INTERVAL_S 
seconds")
-  pool.scheduleWithFixedDelay(getRunner(checkForLogs), 0, 
UPDATE_INTERVAL_S, TimeUnit.SECONDS)
+  pool.scheduleWithFixedDelay(
+getRunner(() => checkForLogs()), 0, UPDATE_INTERVAL_S, 
TimeUnit.SECONDS)
--- End diff --

These changes avoid warnings about eta-expansion of zero-arg methods. It 
works fine in 2.11 as well; just not relying on syntactic sugar for the same.


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request #19307: [SPARK-22087][SPARK-14650][WIP][BUILD][REPL][CORE...

[srowen]

Github user srowen commented on a diff in the pull request:

https://github.com/apache/spark/pull/19307#discussion_r140211444
  
--- Diff: external/kafka-0-10-sql/pom.xml ---
@@ -102,8 +102,19 @@
 
 
   
+
   
 
target/scala-${scala.binary.version}/classes
 
target/scala-${scala.binary.version}/test-classes
   
+
+  
+
+  scala-2.12
+  
+0.10.1.1
--- End diff --

Only 0.10.1+ supports Scala 2.12. By the time a 2.12 build is actually 
supported we may be on to 0.10.2. Not sure. This at least makes it work


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request #19307: [SPARK-22087][SPARK-14650][WIP][BUILD][REPL][CORE...

[srowen]

Github user srowen commented on a diff in the pull request:

https://github.com/apache/spark/pull/19307#discussion_r140211712
  
--- Diff: 
sql/core/src/test/scala/org/apache/spark/sql/DataFrameStatSuite.scala ---
@@ -230,19 +230,16 @@ class DataFrameStatSuite extends QueryTest with 
SharedSQLContext {
 
 val resNaN1 = dfNaN.stat.approxQuantile("input1", Array(q1, q2), 
epsilon)
 assert(resNaN1.count(_.isNaN) === 0)
-assert(resNaN1.count(_ == null) === 0)
--- End diff --

resNaN1 is an Array[Double] so can never contain null. This is always true 
and generated a warning


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request #19307: [SPARK-22087][SPARK-14650][WIP][BUILD][REPL][CORE...

[srowen]

Github user srowen commented on a diff in the pull request:

https://github.com/apache/spark/pull/19307#discussion_r140210528
  
--- Diff: core/src/main/scala/org/apache/spark/SparkContext.scala ---
@@ -2826,33 +2826,33 @@ object WritableConverter {
   // them automatically. However, we still keep the old functions in 
SparkContext for backward
   // compatibility and forward to the following functions directly.
 
-  implicit def intWritableConverter(): WritableConverter[Int] =
-simpleWritableConverter[Int, IntWritable](_.get)
+  implicit val intWritableConverter: () => WritableConverter[Int] =
--- End diff --

These changes were necessary to make the implicits work in 2.12 now that 
eta-expansion of zero-arg methods is deprecated and apparently doesn't work for 
implicit resolution. It passes MiMa, but we could be conservative and retain 
the existing methods, and add function vals instead.


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request #19307: [SPARK-22087][SPARK-14650][WIP][BUILD][REPL][CORE...

[srowen]

Github user srowen commented on a diff in the pull request:

https://github.com/apache/spark/pull/19307#discussion_r140211059
  
--- Diff: 
core/src/main/scala/org/apache/spark/serializer/KryoSerializer.scala ---
@@ -501,8 +501,8 @@ private class JavaIterableWrapperSerializer
 private object JavaIterableWrapperSerializer extends Logging {
   // The class returned by JavaConverters.asJava
   // (scala.collection.convert.Wrappers$IterableWrapper).
-  val wrapperClass =
-scala.collection.convert.WrapAsJava.asJavaIterable(Seq(1)).getClass
+  import scala.collection.JavaConverters._
+  val wrapperClass = Seq(1).asJava.getClass
--- End diff --

WrapAsJava is deprecated and this ought to be the equivalent to obtain a 
Java Iterable wrapper


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request #19307: [SPARK-22087][SPARK-14650][WIP][BUILD][REPL][CORE...

[srowen]

Github user srowen commented on a diff in the pull request:

https://github.com/apache/spark/pull/19307#discussion_r140211607
  
--- Diff: 
repl/scala-2.12/src/main/scala/org/apache/spark/repl/SparkILoop.scala ---
@@ -0,0 +1,134 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark.repl
+
+import java.io.BufferedReader
+
+// scalastyle:off println
+import scala.Predef.{println => _, _}
+// scalastyle:on println
+import scala.tools.nsc.Settings
+import scala.tools.nsc.interpreter.{ILoop, JPrintWriter}
+import scala.tools.nsc.util.stringFromStream
+import scala.util.Properties.{javaVersion, javaVmName, versionString}
+
+/**
+ *  A Spark-specific interactive shell.
+ */
+class SparkILoop(in0: Option[BufferedReader], out: JPrintWriter)
+extends ILoop(in0, out) {
+  def this(in0: BufferedReader, out: JPrintWriter) = this(Some(in0), out)
+  def this() = this(None, new JPrintWriter(Console.out, true))
+
+  def initializeSpark() {
+intp.beQuietDuring {
+  processLine("""
+@transient val spark = if (org.apache.spark.repl.Main.sparkSession 
!= null) {
+org.apache.spark.repl.Main.sparkSession
+  } else {
+org.apache.spark.repl.Main.createSparkSession()
+  }
+@transient val sc = {
+  val _sc = spark.sparkContext
+  if (_sc.getConf.getBoolean("spark.ui.reverseProxy", false)) {
+val proxyUrl = _sc.getConf.get("spark.ui.reverseProxyUrl", 
null)
+if (proxyUrl != null) {
+  println(
+s"Spark Context Web UI is available at 
${proxyUrl}/proxy/${_sc.applicationId}")
+} else {
+  println(s"Spark Context Web UI is available at Spark Master 
Public URL")
+}
+  } else {
+_sc.uiWebUrl.foreach {
+  webUrl => println(s"Spark context Web UI available at 
${webUrl}")
+}
+  }
+  println("Spark context available as 'sc' " +
+s"(master = ${_sc.master}, app id = ${_sc.applicationId}).")
+  println("Spark session available as 'spark'.")
+  _sc
+}
+""")
+  processLine("import org.apache.spark.SparkContext._")
+  processLine("import spark.implicits._")
+  processLine("import spark.sql")
+  processLine("import org.apache.spark.sql.functions._")
+}
+  }
+
+  /** Print a welcome message */
+  override def printWelcome() {
+import org.apache.spark.SPARK_VERSION
+echo("""Welcome to
+    __
+ / __/__  ___ _/ /__
+_\ \/ _ \/ _ `/ __/  '_/
+   /___/ .__/\_,_/_/ /_/\_\   version %s
+  /_/
+ """.format(SPARK_VERSION))
+val welcomeMsg = "Using Scala %s (%s, Java %s)".format(
+  versionString, javaVmName, javaVersion)
+echo(welcomeMsg)
+echo("Type in expressions to have them evaluated.")
+echo("Type :help for more information.")
+  }
+
+  /** Available commands */
+  override def commands: List[LoopCommand] = standardCommands
+
+  /**
+   * We override `createInterpreter` because we need to initialize Spark 
*before* the REPL
+   * sees any files, so that the Spark context is visible in those files. 
This is a bit of a
+   * hack, but there isn't another hook available to us at this point.
+   */
+  override def createInterpreter(): Unit = {
--- End diff --

This is the only meaningful difference from the 2.11 REPL, as it has to 
hook into a different place. All other REPL-related code isn't specific to 
2.11/2.12 and was moved out into the common src directory in the repl module


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request #19307: [SPARK-22087][SPARK-14650][WIP][BUILD][REPL][CORE...

[srowen]

Github user srowen commented on a diff in the pull request:

https://github.com/apache/spark/pull/19307#discussion_r140212140
  
--- Diff: sql/hive-thriftserver/pom.xml ---
@@ -63,6 +63,16 @@
   ${hive.group}
   hive-beeline
 
+
--- End diff --

For some reason, hive-thriftserver wouldn't compile, unable to find 
jetty.server classes. It has a point, these should explicit, even if it somehow 
worked before.


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request #19307: [SPARK-22087][SPARK-14650][WIP][BUILD][REPL][CORE...

[srowen]

GitHub user srowen opened a pull request:

https://github.com/apache/spark/pull/19307

[SPARK-22087][SPARK-14650][WIP][BUILD][REPL][CORE] Compile Spark REPL for 
Scala 2.12 + other 2.12 fixes

## What changes were proposed in this pull request?

Enable Scala 2.12 REPL. Fix most remaining issues with 2.12 compilation and 
warnings, including:

- Selecting Kafka 0.10.1+ for Scala 2.12 and patching over a minor API 
difference
- Fixing lots of "eta expansion of zero arg method deprecated" warnings
- Resolving the SparkContext.sequenceFile implicits compile problem
- Fixing an odd but valid jetty-server missing dependency in 
hive-thriftserver

## How was this patch tested?

Existing tests

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/srowen/spark Scala212

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/spark/pull/19307.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #19307


commit 99f082a71643dac129a94b162ca0d04ad1d68c8f
Author: Sean Owen 
Date:   2017-09-19T15:15:02Z

Initial Scala 2.12 deprecation fixes and compilation fixes




---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org