You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by srowen <gi...@git.apache.org> on 2018/11/20 22:48:23 UTC
[GitHub] spark pull request #23098: [WIP][SPARK-26132][BUILD][CORE] Remove support fo...
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 <se...@...>
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-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #23098: [WIP][SPARK-26132][BUILD][CORE] Remove support fo...
Posted by srowen <gi...@git.apache.org>.
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-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23098: [WIP][SPARK-26132][BUILD][CORE] Remove support for Scala...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/23098
**[Test build #99087 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/99087/testReport)** for PR 23098 at commit [`1c1f000`](https://github.com/apache/spark/commit/1c1f0006c4a9439459d07132180b8f7fbd26b879).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #23098: [WIP][SPARK-26132][BUILD][CORE] Remove support fo...
Posted by HyukjinKwon <gi...@git.apache.org>.
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-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23098: [WIP][SPARK-26132][BUILD][CORE] Remove support for Scala...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/23098
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/99667/
Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #23098: [WIP][SPARK-26132][BUILD][CORE] Remove support fo...
Posted by HyukjinKwon <gi...@git.apache.org>.
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-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #23098: [WIP][SPARK-26132][BUILD][CORE] Remove support fo...
Posted by srowen <gi...@git.apache.org>.
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-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23098: [WIP][SPARK-26132][BUILD][CORE] Remove support for Scala...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/23098
Merged build finished. Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #23098: [WIP][SPARK-26132][BUILD][CORE] Remove support fo...
Posted by HyukjinKwon <gi...@git.apache.org>.
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-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #23098: [WIP][SPARK-26132][BUILD][CORE] Remove support fo...
Posted by srowen <gi...@git.apache.org>.
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-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23098: [WIP][SPARK-26132][BUILD][CORE] Remove support for Scala...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/23098
Merged build finished. Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #23098: [WIP][SPARK-26132][BUILD][CORE] Remove support fo...
Posted by srowen <gi...@git.apache.org>.
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-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23098: [WIP][SPARK-26132][BUILD][CORE] Remove support for Scala...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/23098
**[Test build #99382 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/99382/testReport)** for PR 23098 at commit [`2b8d770`](https://github.com/apache/spark/commit/2b8d7704fa8b1f14101b1ce510da91db348f42b2).
* This patch **fails Spark unit tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23098: [WIP][SPARK-26132][BUILD][CORE] Remove support for Scala...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/23098
**[Test build #99086 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/99086/testReport)** for PR 23098 at commit [`0420c2d`](https://github.com/apache/spark/commit/0420c2d5ac56752f8f171a0abfa9fb47d678e90b).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #23098: [WIP][SPARK-26132][BUILD][CORE] Remove support fo...
Posted by HyukjinKwon <gi...@git.apache.org>.
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-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #23098: [WIP][SPARK-26132][BUILD][CORE] Remove support fo...
Posted by vanzin <gi...@git.apache.org>.
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-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23098: [WIP][SPARK-26132][BUILD][CORE] Remove support for Scala...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/23098
**[Test build #99087 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/99087/testReport)** for PR 23098 at commit [`1c1f000`](https://github.com/apache/spark/commit/1c1f0006c4a9439459d07132180b8f7fbd26b879).
* This patch **fails Spark unit tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23098: [WIP][SPARK-26132][BUILD][CORE] Remove support for Scala...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/23098
**[Test build #99667 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/99667/testReport)** for PR 23098 at commit [`96f9c41`](https://github.com/apache/spark/commit/96f9c419961df2f6be1c137bbcd5a2ff27225fad).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23098: [WIP][SPARK-26132][BUILD][CORE] Remove support for Scala...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/23098
**[Test build #99086 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/99086/testReport)** for PR 23098 at commit [`0420c2d`](https://github.com/apache/spark/commit/0420c2d5ac56752f8f171a0abfa9fb47d678e90b).
* This patch **fails Scala style tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23098: [WIP][SPARK-26132][BUILD][CORE] Remove support for Scala...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/23098
Merged build finished. Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #23098: [WIP][SPARK-26132][BUILD][CORE] Remove support fo...
Posted by srowen <gi...@git.apache.org>.
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-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23098: [WIP][SPARK-26132][BUILD][CORE] Remove support for Scala...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/23098
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23098: [WIP][SPARK-26132][BUILD][CORE] Remove support for Scala...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/23098
**[Test build #99382 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/99382/testReport)** for PR 23098 at commit [`2b8d770`](https://github.com/apache/spark/commit/2b8d7704fa8b1f14101b1ce510da91db348f42b2).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #23098: [WIP][SPARK-26132][BUILD][CORE] Remove support fo...
Posted by srowen <gi...@git.apache.org>.
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-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23098: [WIP][SPARK-26132][BUILD][CORE] Remove support for Scala...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/23098
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/99086/
Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #23098: [WIP][SPARK-26132][BUILD][CORE] Remove support fo...
Posted by HyukjinKwon <gi...@git.apache.org>.
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-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23098: [WIP][SPARK-26132][BUILD][CORE] Remove support for Scala...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/23098
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23098: [WIP][SPARK-26132][BUILD][CORE] Remove support for Scala...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/23098
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution-unified/5457/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #23098: [WIP][SPARK-26132][BUILD][CORE] Remove support fo...
Posted by srowen <gi...@git.apache.org>.
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-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23098: [WIP][SPARK-26132][BUILD][CORE] Remove support for Scala...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the issue:
https://github.com/apache/spark/pull/23098
Note I'm holding on to this PR for a while as I understand it might be disruptive to downstream builds to remove 2.11 support just now. Will look at merging it in weeks. Right now it's an FYI.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23098: [WIP][SPARK-26132][BUILD][CORE] Remove support for Scala...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/23098
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution-unified/5199/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23098: [WIP][SPARK-26132][BUILD][CORE] Remove support for Scala...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/23098
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution-unified/5200/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #23098: [WIP][SPARK-26132][BUILD][CORE] Remove support fo...
Posted by srowen <gi...@git.apache.org>.
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-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23098: [WIP][SPARK-26132][BUILD][CORE] Remove support for Scala...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/23098
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution-unified/5721/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #23098: [WIP][SPARK-26132][BUILD][CORE] Remove support fo...
Posted by felixcheung <gi...@git.apache.org>.
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-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23098: [WIP][SPARK-26132][BUILD][CORE] Remove support for Scala...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/23098
**[Test build #99667 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/99667/testReport)** for PR 23098 at commit [`96f9c41`](https://github.com/apache/spark/commit/96f9c419961df2f6be1c137bbcd5a2ff27225fad).
* This patch **fails Spark unit tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #23098: [WIP][SPARK-26132][BUILD][CORE] Remove support fo...
Posted by srowen <gi...@git.apache.org>.
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-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #23098: [WIP][SPARK-26132][BUILD][CORE] Remove support fo...
Posted by vanzin <gi...@git.apache.org>.
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-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23098: [WIP][SPARK-26132][BUILD][CORE] Remove support for Scala...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/23098
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/99382/
Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #23098: [WIP][SPARK-26132][BUILD][CORE] Remove support fo...
Posted by gengliangwang <gi...@git.apache.org>.
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-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #23098: [WIP][SPARK-26132][BUILD][CORE] Remove support fo...
Posted by vanzin <gi...@git.apache.org>.
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-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23098: [WIP][SPARK-26132][BUILD][CORE] Remove support for Scala...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/23098
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23098: [WIP][SPARK-26132][BUILD][CORE] Remove support for Scala...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/23098
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/99087/
Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23098: [WIP][SPARK-26132][BUILD][CORE] Remove support for Scala...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/23098
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23098: [WIP][SPARK-26132][BUILD][CORE] Remove support for Scala...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/23098
Merged build finished. Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org