[GitHub] spark pull request #23098: [WIP][SPARK-26132][BUILD][CORE] Remove support fo...

2018-11-27 Thread srowen 
Github user srowen commented on a diff in the pull request:

https://github.com/apache/spark/pull/23098#discussion_r236840320
  
--- Diff: dev/create-release/release-build.sh ---
@@ -110,16 +110,18 @@ fi
 # Depending on the version being built, certain extra profiles need to be 
activated, and
 # different versions of Scala are supported.
 BASE_PROFILES="-Pmesos -Pyarn"
-PUBLISH_SCALA_2_10=0
-SCALA_2_10_PROFILES="-Pscala-2.10"
-SCALA_2_11_PROFILES=
+
+# TODO: revisit for Scala 2.13
+
+PUBLISH_SCALA_2_11=1
--- End diff --

OK, the issue is that the release script for 2.3.0 that works might only 
appear in 2.3.1 because of some last-minute hacks during the release process. I 
can see being careful not to drop 2.3.0 logic immediately after 2.3.0. Maybe at 
2.4.0, but that's aggressive.

At least, sure, we can forget Spark < 2.3 in Spark 3.0.0's build script. 
Maybe later we decide to go further. I'll work on making it that way.


---

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

[GitHub] spark pull request #23098: [WIP][SPARK-26132][BUILD][CORE] Remove support fo...

2018-11-27 Thread felixcheung 
Github user felixcheung commented on a diff in the pull request:

https://github.com/apache/spark/pull/23098#discussion_r236764795
  
--- Diff: R/pkg/R/sparkR.R ---
@@ -269,7 +269,7 @@ sparkR.sparkContext <- function(
 #' sparkR.session("yarn-client", "SparkR", "/home/spark",
 #'list(spark.executor.memory="4g"),
 #'c("one.jar", "two.jar", "three.jar"),
-#'c("com.databricks:spark-avro_2.11:2.0.1"))
+#'c("com.databricks:spark-avro_2.12:2.0.1"))
--- End diff --

yes, dummy name is completely fine with me.


---

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

[GitHub] spark pull request #23098: [WIP][SPARK-26132][BUILD][CORE] Remove support fo...

2018-11-27 Thread vanzin 
Github user vanzin commented on a diff in the pull request:

https://github.com/apache/spark/pull/23098#discussion_r236736341
  
--- Diff: dev/create-release/release-build.sh ---
@@ -110,16 +110,18 @@ fi
 # Depending on the version being built, certain extra profiles need to be 
activated, and
 # different versions of Scala are supported.
 BASE_PROFILES="-Pmesos -Pyarn"
-PUBLISH_SCALA_2_10=0
-SCALA_2_10_PROFILES="-Pscala-2.10"
-SCALA_2_11_PROFILES=
+
+# TODO: revisit for Scala 2.13
+
+PUBLISH_SCALA_2_11=1
--- End diff --

>  if you're releasing 2.3 wouldn't you use the release script as of 2.3?

I started like that when I RM'ed 2.3.1, but the scripts were kinda broken 
that I had to fix a bunch of stuff in master. At that point, it didn't make 
sense to use the scripts in the 2.3 branch anymore, and keeping them in sync 
was kinda wasted effort.

> basically trying to support 2.2+, which seems reasonable?

That's my view. The release scripts in master should be usable for all 
non-EOL releases.


---

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

[GitHub] spark pull request #23098: [WIP][SPARK-26132][BUILD][CORE] Remove support fo...

2018-11-27 Thread srowen 
Github user srowen commented on a diff in the pull request:

https://github.com/apache/spark/pull/23098#discussion_r236726267
  
--- Diff: dev/create-release/release-build.sh ---
@@ -110,16 +110,18 @@ fi
 # Depending on the version being built, certain extra profiles need to be 
activated, and
 # different versions of Scala are supported.
 BASE_PROFILES="-Pmesos -Pyarn"
-PUBLISH_SCALA_2_10=0
-SCALA_2_10_PROFILES="-Pscala-2.10"
-SCALA_2_11_PROFILES=
+
+# TODO: revisit for Scala 2.13
+
+PUBLISH_SCALA_2_11=1
--- End diff --

Just a question though, if you're releasing 2.3 wouldn't you use the 
release script as of 2.3? 

I think the script works for Spark 1.6 and 2.x, but not earlier versions, 
already. What about drawing the lines at major releases, so that this can 
simplify further?

Right now I think it's basically trying to support 2.2+, which are the 
non-EOL releases, which seems reasonable?


---

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

[GitHub] spark pull request #23098: [WIP][SPARK-26132][BUILD][CORE] Remove support fo...

2018-11-27 Thread srowen 
Github user srowen commented on a diff in the pull request:

https://github.com/apache/spark/pull/23098#discussion_r236714704
  
--- Diff: R/pkg/R/sparkR.R ---
@@ -269,7 +269,7 @@ sparkR.sparkContext <- function(
 #' sparkR.session("yarn-client", "SparkR", "/home/spark",
 #'list(spark.executor.memory="4g"),
 #'c("one.jar", "two.jar", "three.jar"),
-#'c("com.databricks:spark-avro_2.11:2.0.1"))
+#'c("com.databricks:spark-avro_2.12:2.0.1"))
--- End diff --

@felixcheung was the conclusion that we can make this a dummy package? I 
just want to avoid showing _2.11 usage here.


---

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

[GitHub] spark pull request #23098: [WIP][SPARK-26132][BUILD][CORE] Remove support fo...

2018-11-26 Thread vanzin 
Github user vanzin commented on a diff in the pull request:

https://github.com/apache/spark/pull/23098#discussion_r236452713
  
--- Diff: dev/create-release/release-build.sh ---
@@ -110,16 +110,18 @@ fi
 # Depending on the version being built, certain extra profiles need to be 
activated, and
 # different versions of Scala are supported.
 BASE_PROFILES="-Pmesos -Pyarn"
-PUBLISH_SCALA_2_10=0
-SCALA_2_10_PROFILES="-Pscala-2.10"
-SCALA_2_11_PROFILES=
+
+# TODO: revisit for Scala 2.13
+
+PUBLISH_SCALA_2_11=1
--- End diff --

I prefer keeping the script in master working against all currently 
supported versions. I find it pretty hard to keep things in sync across 
different branches, especially if you need to fix things in the middle of an RC 
cycle. Having master be the source of truth for this makes some of that pain 
goes away, with the cost of some added logic here.

I think the main problem now is that for 3.0 the default Scala version is 
different than for 2.4, which is the only added complication I can think of 
here...


---

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

[GitHub] spark pull request #23098: [WIP][SPARK-26132][BUILD][CORE] Remove support fo...

2018-11-26 Thread vanzin 
Github user vanzin commented on a diff in the pull request:

https://github.com/apache/spark/pull/23098#discussion_r236446713
  
--- Diff: R/pkg/R/sparkR.R ---
@@ -269,7 +269,7 @@ sparkR.sparkContext <- function(
 #' sparkR.session("yarn-client", "SparkR", "/home/spark",
 #'list(spark.executor.memory="4g"),
 #'c("one.jar", "two.jar", "three.jar"),
-#'c("com.databricks:spark-avro_2.11:2.0.1"))
+#'c("com.databricks:spark-avro_2.12:2.0.1"))
--- End diff --

I think there's even a separate discussion about even using this as an 
example, since that package is now in Spark since 2.4.


---

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

[GitHub] spark pull request #23098: [WIP][SPARK-26132][BUILD][CORE] Remove support fo...

2018-11-22 Thread HyukjinKwon 
Github user HyukjinKwon commented on a diff in the pull request:

https://github.com/apache/spark/pull/23098#discussion_r235830558
  
--- Diff: bin/load-spark-env.cmd ---
@@ -21,37 +21,42 @@ rem This script loads spark-env.cmd if it exists, and 
ensures it is only loaded
 rem spark-env.cmd is loaded from SPARK_CONF_DIR if set, or within the 
current directory's
 rem conf\ subdirectory.
 
+set SPARK_ENV_CMD="spark-env.cmd"
 if [%SPARK_ENV_LOADED%] == [] (
   set SPARK_ENV_LOADED=1
 
   if [%SPARK_CONF_DIR%] == [] (
 set SPARK_CONF_DIR=%~dp0..\conf
   )
 
-  call :LoadSparkEnv
+  set SPARK_ENV_CMD="%SPARK_CONF_DIR%/%SPARK_ENV_CMD%"
+  if exist "%SPARK_ENV_CMD%" (
+call "%SPARK_ENV_CMD%"
+  )
 )
 
 rem Setting SPARK_SCALA_VERSION if not already set.
 
-set ASSEMBLY_DIR2="%SPARK_HOME%\assembly\target\scala-2.11"
-set ASSEMBLY_DIR1="%SPARK_HOME%\assembly\target\scala-2.12"
-
-if [%SPARK_SCALA_VERSION%] == [] (
-
-  if exist %ASSEMBLY_DIR2% if exist %ASSEMBLY_DIR1% (
-echo "Presence of build for multiple Scala versions detected."
-echo "Either clean one of them or, set SPARK_SCALA_VERSION in 
spark-env.cmd."
-exit 1
-  )
-  if exist %ASSEMBLY_DIR2% (
-set SPARK_SCALA_VERSION=2.11
-  ) else (
-set SPARK_SCALA_VERSION=2.12
-  )
-)
+rem TODO: revisit for Scala 2.13 support
+set SPARK_SCALA_VERSION=2.12
--- End diff --

Here, I ran some of simple commands:

```cmd
C:\>set A=aa

C:\>ECHO %A%
aa

C:\>set A="aa"

C:\>ECHO %A%
"aa"

C:\>call "python.exe"
Python 3.6.4 (v3.6.4:d48eceb, Dec 19 2017, 06:54:40) [MSC v.1900 64 bit 
(AMD64)] on win32
Type "help", "copyright", "credits" or "license" for more information.
>>> exit(0)

C:\>call python.exe
Python 3.6.4 (v3.6.4:d48eceb, Dec 19 2017, 06:54:40) [MSC v.1900 64 bit 
(AMD64)] on win32
Type "help", "copyright", "credits" or "license" for more information.
>>> exit(0)
```


---

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

[GitHub] spark pull request #23098: [WIP][SPARK-26132][BUILD][CORE] Remove support fo...

2018-11-21 Thread HyukjinKwon 
Github user HyukjinKwon commented on a diff in the pull request:

https://github.com/apache/spark/pull/23098#discussion_r235572895
  
--- Diff: bin/load-spark-env.cmd ---
@@ -21,37 +21,42 @@ rem This script loads spark-env.cmd if it exists, and 
ensures it is only loaded
 rem spark-env.cmd is loaded from SPARK_CONF_DIR if set, or within the 
current directory's
 rem conf\ subdirectory.
 
+set SPARK_ENV_CMD="spark-env.cmd"
 if [%SPARK_ENV_LOADED%] == [] (
   set SPARK_ENV_LOADED=1
 
   if [%SPARK_CONF_DIR%] == [] (
 set SPARK_CONF_DIR=%~dp0..\conf
   )
 
-  call :LoadSparkEnv
+  set SPARK_ENV_CMD="%SPARK_CONF_DIR%/%SPARK_ENV_CMD%"
+  if exist "%SPARK_ENV_CMD%" (
+call "%SPARK_ENV_CMD%"
+  )
 )
 
 rem Setting SPARK_SCALA_VERSION if not already set.
 
-set ASSEMBLY_DIR2="%SPARK_HOME%\assembly\target\scala-2.11"
-set ASSEMBLY_DIR1="%SPARK_HOME%\assembly\target\scala-2.12"
-
-if [%SPARK_SCALA_VERSION%] == [] (
-
-  if exist %ASSEMBLY_DIR2% if exist %ASSEMBLY_DIR1% (
-echo "Presence of build for multiple Scala versions detected."
-echo "Either clean one of them or, set SPARK_SCALA_VERSION in 
spark-env.cmd."
-exit 1
-  )
-  if exist %ASSEMBLY_DIR2% (
-set SPARK_SCALA_VERSION=2.11
-  ) else (
-set SPARK_SCALA_VERSION=2.12
-  )
-)
+rem TODO: revisit for Scala 2.13 support
+set SPARK_SCALA_VERSION=2.12
--- End diff --

For `call`, it's a bit different IIRC (https://ss64.com/nt/cmd.html)


---

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

[GitHub] spark pull request #23098: [WIP][SPARK-26132][BUILD][CORE] Remove support fo...

2018-11-21 Thread HyukjinKwon 
Github user HyukjinKwon commented on a diff in the pull request:

https://github.com/apache/spark/pull/23098#discussion_r235572737
  
--- Diff: bin/load-spark-env.cmd ---
@@ -21,37 +21,42 @@ rem This script loads spark-env.cmd if it exists, and 
ensures it is only loaded
 rem spark-env.cmd is loaded from SPARK_CONF_DIR if set, or within the 
current directory's
 rem conf\ subdirectory.
 
+set SPARK_ENV_CMD="spark-env.cmd"
 if [%SPARK_ENV_LOADED%] == [] (
   set SPARK_ENV_LOADED=1
 
   if [%SPARK_CONF_DIR%] == [] (
 set SPARK_CONF_DIR=%~dp0..\conf
   )
 
-  call :LoadSparkEnv
+  set SPARK_ENV_CMD="%SPARK_CONF_DIR%/%SPARK_ENV_CMD%"
+  if exist "%SPARK_ENV_CMD%" (
+call "%SPARK_ENV_CMD%"
+  )
 )
 
 rem Setting SPARK_SCALA_VERSION if not already set.
 
-set ASSEMBLY_DIR2="%SPARK_HOME%\assembly\target\scala-2.11"
-set ASSEMBLY_DIR1="%SPARK_HOME%\assembly\target\scala-2.12"
-
-if [%SPARK_SCALA_VERSION%] == [] (
-
-  if exist %ASSEMBLY_DIR2% if exist %ASSEMBLY_DIR1% (
-echo "Presence of build for multiple Scala versions detected."
-echo "Either clean one of them or, set SPARK_SCALA_VERSION in 
spark-env.cmd."
-exit 1
-  )
-  if exist %ASSEMBLY_DIR2% (
-set SPARK_SCALA_VERSION=2.11
-  ) else (
-set SPARK_SCALA_VERSION=2.12
-  )
-)
+rem TODO: revisit for Scala 2.13 support
+set SPARK_SCALA_VERSION=2.12
--- End diff --

but at least that's what I remember when I test 
https://github.com/apache/spark/blob/master/bin/spark-sql2.cmd#L23 line.


---

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

[GitHub] spark pull request #23098: [WIP][SPARK-26132][BUILD][CORE] Remove support fo...

2018-11-21 Thread HyukjinKwon 
Github user HyukjinKwon commented on a diff in the pull request:

https://github.com/apache/spark/pull/23098#discussion_r235572442
  
--- Diff: bin/load-spark-env.cmd ---
@@ -21,37 +21,42 @@ rem This script loads spark-env.cmd if it exists, and 
ensures it is only loaded
 rem spark-env.cmd is loaded from SPARK_CONF_DIR if set, or within the 
current directory's
 rem conf\ subdirectory.
 
+set SPARK_ENV_CMD="spark-env.cmd"
 if [%SPARK_ENV_LOADED%] == [] (
   set SPARK_ENV_LOADED=1
 
   if [%SPARK_CONF_DIR%] == [] (
 set SPARK_CONF_DIR=%~dp0..\conf
   )
 
-  call :LoadSparkEnv
+  set SPARK_ENV_CMD="%SPARK_CONF_DIR%/%SPARK_ENV_CMD%"
+  if exist "%SPARK_ENV_CMD%" (
+call "%SPARK_ENV_CMD%"
+  )
 )
 
 rem Setting SPARK_SCALA_VERSION if not already set.
 
-set ASSEMBLY_DIR2="%SPARK_HOME%\assembly\target\scala-2.11"
-set ASSEMBLY_DIR1="%SPARK_HOME%\assembly\target\scala-2.12"
-
-if [%SPARK_SCALA_VERSION%] == [] (
-
-  if exist %ASSEMBLY_DIR2% if exist %ASSEMBLY_DIR1% (
-echo "Presence of build for multiple Scala versions detected."
-echo "Either clean one of them or, set SPARK_SCALA_VERSION in 
spark-env.cmd."
-exit 1
-  )
-  if exist %ASSEMBLY_DIR2% (
-set SPARK_SCALA_VERSION=2.11
-  ) else (
-set SPARK_SCALA_VERSION=2.12
-  )
-)
+rem TODO: revisit for Scala 2.13 support
+set SPARK_SCALA_VERSION=2.12
--- End diff --

Yea, I think we shouldn't quote if I remember this correctly. Let me test 
and get back to you today or tomorrow. I'll have to fly to Korea for my one 
week vacation from tomorrow :D.


---

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

[GitHub] spark pull request #23098: [WIP][SPARK-26132][BUILD][CORE] Remove support fo...

2018-11-21 Thread srowen 
Github user srowen commented on a diff in the pull request:

https://github.com/apache/spark/pull/23098#discussion_r235418193
  
--- Diff: bin/load-spark-env.cmd ---
@@ -21,37 +21,42 @@ rem This script loads spark-env.cmd if it exists, and 
ensures it is only loaded
 rem spark-env.cmd is loaded from SPARK_CONF_DIR if set, or within the 
current directory's
 rem conf\ subdirectory.
 
+set SPARK_ENV_CMD="spark-env.cmd"
 if [%SPARK_ENV_LOADED%] == [] (
   set SPARK_ENV_LOADED=1
 
   if [%SPARK_CONF_DIR%] == [] (
 set SPARK_CONF_DIR=%~dp0..\conf
   )
 
-  call :LoadSparkEnv
+  set SPARK_ENV_CMD="%SPARK_CONF_DIR%/%SPARK_ENV_CMD%"
+  if exist "%SPARK_ENV_CMD%" (
+call "%SPARK_ENV_CMD%"
+  )
 )
 
 rem Setting SPARK_SCALA_VERSION if not already set.
 
-set ASSEMBLY_DIR2="%SPARK_HOME%\assembly\target\scala-2.11"
-set ASSEMBLY_DIR1="%SPARK_HOME%\assembly\target\scala-2.12"
-
-if [%SPARK_SCALA_VERSION%] == [] (
-
-  if exist %ASSEMBLY_DIR2% if exist %ASSEMBLY_DIR1% (
-echo "Presence of build for multiple Scala versions detected."
-echo "Either clean one of them or, set SPARK_SCALA_VERSION in 
spark-env.cmd."
-exit 1
-  )
-  if exist %ASSEMBLY_DIR2% (
-set SPARK_SCALA_VERSION=2.11
-  ) else (
-set SPARK_SCALA_VERSION=2.12
-  )
-)
+rem TODO: revisit for Scala 2.13 support
+set SPARK_SCALA_VERSION=2.12
--- End diff --

Oh I see, so we shouldn't add quotes to values like SPARK_ENV_CMD above, 
but use them in if conditions, call, etc?


---

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

[GitHub] spark pull request #23098: [WIP][SPARK-26132][BUILD][CORE] Remove support fo...

2018-11-21 Thread HyukjinKwon 
Github user HyukjinKwon commented on a diff in the pull request:

https://github.com/apache/spark/pull/23098#discussion_r235322452
  
--- Diff: bin/load-spark-env.cmd ---
@@ -21,37 +21,42 @@ rem This script loads spark-env.cmd if it exists, and 
ensures it is only loaded
 rem spark-env.cmd is loaded from SPARK_CONF_DIR if set, or within the 
current directory's
 rem conf\ subdirectory.
 
+set SPARK_ENV_CMD="spark-env.cmd"
 if [%SPARK_ENV_LOADED%] == [] (
   set SPARK_ENV_LOADED=1
 
   if [%SPARK_CONF_DIR%] == [] (
 set SPARK_CONF_DIR=%~dp0..\conf
   )
 
-  call :LoadSparkEnv
+  set SPARK_ENV_CMD="%SPARK_CONF_DIR%/%SPARK_ENV_CMD%"
+  if exist "%SPARK_ENV_CMD%" (
+call "%SPARK_ENV_CMD%"
+  )
 )
 
 rem Setting SPARK_SCALA_VERSION if not already set.
 
-set ASSEMBLY_DIR2="%SPARK_HOME%\assembly\target\scala-2.11"
-set ASSEMBLY_DIR1="%SPARK_HOME%\assembly\target\scala-2.12"
-
-if [%SPARK_SCALA_VERSION%] == [] (
-
-  if exist %ASSEMBLY_DIR2% if exist %ASSEMBLY_DIR1% (
-echo "Presence of build for multiple Scala versions detected."
-echo "Either clean one of them or, set SPARK_SCALA_VERSION in 
spark-env.cmd."
-exit 1
-  )
-  if exist %ASSEMBLY_DIR2% (
-set SPARK_SCALA_VERSION=2.11
-  ) else (
-set SPARK_SCALA_VERSION=2.12
-  )
-)
+rem TODO: revisit for Scala 2.13 support
+set SPARK_SCALA_VERSION=2.12
--- End diff --

Nope, it takes as is including quotes ... haha odd (to me).


---

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

[GitHub] spark pull request #23098: [WIP][SPARK-26132][BUILD][CORE] Remove support fo...

2018-11-21 Thread gengliangwang 
Github user gengliangwang commented on a diff in the pull request:

https://github.com/apache/spark/pull/23098#discussion_r235317414
  
--- Diff: bin/load-spark-env.cmd ---
@@ -21,37 +21,42 @@ rem This script loads spark-env.cmd if it exists, and 
ensures it is only loaded
 rem spark-env.cmd is loaded from SPARK_CONF_DIR if set, or within the 
current directory's
 rem conf\ subdirectory.
 
+set SPARK_ENV_CMD="spark-env.cmd"
 if [%SPARK_ENV_LOADED%] == [] (
   set SPARK_ENV_LOADED=1
 
   if [%SPARK_CONF_DIR%] == [] (
 set SPARK_CONF_DIR=%~dp0..\conf
   )
 
-  call :LoadSparkEnv
+  set SPARK_ENV_CMD="%SPARK_CONF_DIR%/%SPARK_ENV_CMD%"
+  if exist "%SPARK_ENV_CMD%" (
+call "%SPARK_ENV_CMD%"
+  )
 )
 
 rem Setting SPARK_SCALA_VERSION if not already set.
 
-set ASSEMBLY_DIR2="%SPARK_HOME%\assembly\target\scala-2.11"
-set ASSEMBLY_DIR1="%SPARK_HOME%\assembly\target\scala-2.12"
-
-if [%SPARK_SCALA_VERSION%] == [] (
-
-  if exist %ASSEMBLY_DIR2% if exist %ASSEMBLY_DIR1% (
-echo "Presence of build for multiple Scala versions detected."
-echo "Either clean one of them or, set SPARK_SCALA_VERSION in 
spark-env.cmd."
-exit 1
-  )
-  if exist %ASSEMBLY_DIR2% (
-set SPARK_SCALA_VERSION=2.11
-  ) else (
-set SPARK_SCALA_VERSION=2.12
-  )
-)
+rem TODO: revisit for Scala 2.13 support
+set SPARK_SCALA_VERSION=2.12
--- End diff --

I am not familiar with .cmd script. Should we keep the quote here, `"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 #23098: [WIP][SPARK-26132][BUILD][CORE] Remove support fo...

2018-11-20 Thread srowen 
Github user srowen commented on a diff in the pull request:

https://github.com/apache/spark/pull/23098#discussion_r235198196
  
--- Diff: R/pkg/R/sparkR.R ---
@@ -269,7 +269,7 @@ sparkR.sparkContext <- function(
 #' sparkR.session("yarn-client", "SparkR", "/home/spark",
 #'list(spark.executor.memory="4g"),
 #'c("one.jar", "two.jar", "three.jar"),
-#'c("com.databricks:spark-avro_2.11:2.0.1"))
+#'c("com.databricks:spark-avro_2.12:2.0.1"))
--- End diff --

@felixcheung is it OK to refer to _2.12 artifacts here? I don't think this 
one actually exists, but is it just an example?


---

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

[GitHub] spark pull request #23098: [WIP][SPARK-26132][BUILD][CORE] Remove support fo...

2018-11-20 Thread srowen 
Github user srowen commented on a diff in the pull request:

https://github.com/apache/spark/pull/23098#discussion_r235198775
  
--- Diff: dev/create-release/release-build.sh ---
@@ -110,16 +110,18 @@ fi
 # Depending on the version being built, certain extra profiles need to be 
activated, and
 # different versions of Scala are supported.
 BASE_PROFILES="-Pmesos -Pyarn"
-PUBLISH_SCALA_2_10=0
-SCALA_2_10_PROFILES="-Pscala-2.10"
-SCALA_2_11_PROFILES=
+
+# TODO: revisit for Scala 2.13
+
+PUBLISH_SCALA_2_11=1
--- End diff --

@vanzin @cloud-fan you may want to look at this. It's getting a little 
hairy in this script.

I recall that the goal was to use this script to create older Spark 
releases, so it needs logic for older versions. But looking at it, I don't 
think it actually creates quite the same release as older versions anyway. Is 
it OK to clean house here and assume only Spark 3 will be built from this 
script? I already deleted some really old logic here (Spark < 2.2)


---

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

[GitHub] spark pull request #23098: [WIP][SPARK-26132][BUILD][CORE] Remove support fo...

2018-11-20 Thread srowen 
Github user srowen commented on a diff in the pull request:

https://github.com/apache/spark/pull/23098#discussion_r235198880
  
--- Diff: project/MimaExcludes.scala ---
@@ -375,993 +375,10 @@ object MimaExcludes {
 
ProblemFilters.exclude[FinalMethodProblem]("org.apache.spark.ml.feature.StringIndexerModel.getHandleInvalid")
   )
 
-  // Exclude rules for 2.2.x
--- End diff --

I figure we can delete old Spark 2.2 and earlier excludes, right?


---

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

[GitHub] spark pull request #23098: [WIP][SPARK-26132][BUILD][CORE] Remove support fo...

2018-11-20 Thread srowen 
Github user srowen commented on a diff in the pull request:

https://github.com/apache/spark/pull/23098#discussion_r235199020
  
--- Diff: 
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/ScalaReflection.scala 
---
@@ -939,11 +939,9 @@ trait ScalaReflection extends Logging {
* used with a different Scala version.
*/
   def getParameterTypeNullability(func: AnyRef): Seq[Boolean] = {
-if (!Properties.versionString.contains("2.11")) {
-  logWarning(s"Scala ${Properties.versionString} cannot get type 
nullability correctly via " +
-"reflection, thus Spark cannot add proper input null check for 
UDF. To avoid this " +
-"problem, use the typed UDF interfaces instead.")
-}
+logWarning(s"Scala ${Properties.versionString} cannot get type 
nullability correctly via " +
--- End diff --

@maryannxue I think you helped work on this part ... if we're only on Scala 
2.12 now can we simplify this further?


---

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

[GitHub] spark pull request #23098: [WIP][SPARK-26132][BUILD][CORE] Remove support fo...

2018-11-20 Thread srowen 
Github user srowen commented on a diff in the pull request:

https://github.com/apache/spark/pull/23098#discussion_r235198325
  
--- Diff: bin/load-spark-env.cmd ---
@@ -21,37 +21,42 @@ rem This script loads spark-env.cmd if it exists, and 
ensures it is only loaded
 rem spark-env.cmd is loaded from SPARK_CONF_DIR if set, or within the 
current directory's
 rem conf\ subdirectory.
 
+set SPARK_ENV_CMD="spark-env.cmd"
 if [%SPARK_ENV_LOADED%] == [] (
   set SPARK_ENV_LOADED=1
 
   if [%SPARK_CONF_DIR%] == [] (
 set SPARK_CONF_DIR=%~dp0..\conf
   )
 
-  call :LoadSparkEnv
+  set SPARK_ENV_CMD="%SPARK_CONF_DIR%/%SPARK_ENV_CMD%"
+  if exist "%SPARK_ENV_CMD%" (
+call "%SPARK_ENV_CMD%"
+  )
 )
 
 rem Setting SPARK_SCALA_VERSION if not already set.
 
-set ASSEMBLY_DIR2="%SPARK_HOME%\assembly\target\scala-2.11"
-set ASSEMBLY_DIR1="%SPARK_HOME%\assembly\target\scala-2.12"
-
-if [%SPARK_SCALA_VERSION%] == [] (
-
-  if exist %ASSEMBLY_DIR2% if exist %ASSEMBLY_DIR1% (
-echo "Presence of build for multiple Scala versions detected."
-echo "Either clean one of them or, set SPARK_SCALA_VERSION in 
spark-env.cmd."
-exit 1
-  )
-  if exist %ASSEMBLY_DIR2% (
-set SPARK_SCALA_VERSION=2.11
-  ) else (
-set SPARK_SCALA_VERSION=2.12
-  )
-)
+rem TODO: revisit for Scala 2.13 support
+set SPARK_SCALA_VERSION=2.12
+rem if [%SPARK_SCALA_VERSION%] == [] (
--- End diff --

@gengliangwang this was the update I was talking about to the .cmd script. 
You can follow up with this change, uncommented, if you like, separately from 
this PR.


---

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

[GitHub] spark pull request #23098: [WIP][SPARK-26132][BUILD][CORE] Remove support fo...

2018-11-20 Thread srowen 
GitHub user srowen opened a pull request:

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

[WIP][SPARK-26132][BUILD][CORE] Remove support for Scala 2.11 in Spark 3.0.0

## What changes were proposed in this pull request?

Remove Scala 2.11 support in build files and docs, and in various parts of 
code that accommodated 2.11. See some targeted comments below.

## 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 SPARK-26132

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

https://github.com/apache/spark/pull/23098.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 #23098


commit 0420c2d5ac56752f8f171a0abfa9fb47d678e90b
Author: Sean Owen 
Date:   2018-11-20T22:45:48Z

Remove Scala 2.11 support in build files and docs, and in various parts of 
code that accommodated 2.11.




---

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