You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by gerashegalov <gi...@git.apache.org> on 2018/08/24 03:14:38 UTC
[GitHub] spark pull request #22213: [SPARK-25221][DEPLOY] Consistent trailing whitesp...
GitHub user gerashegalov opened a pull request:
https://github.com/apache/spark/pull/22213
[SPARK-25221][DEPLOY] Consistent trailing whitespace treatment of conf values
## What changes were proposed in this pull request?
Stop trimming values of properties loaded from a file
## How was this patch tested?
Added unit test demonstrating the issue hit in production.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/gerashegalov/spark gera/SPARK-25221
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/22213.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 #22213
----
commit 874b2c82a83d94c338d8b1ef9c3b37074cc2e4cc
Author: Gera Shegalov <ge...@...>
Date: 2018-08-23T19:09:05Z
[SPARK-25221][DEPLOY] Consistent trailing whitespace treatment of conf values
----
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22213: [SPARK-25221][DEPLOY] Consistent trailing whitespace tre...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22213
**[Test build #95786 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/95786/testReport)** for PR 22213 at commit [`01d0904`](https://github.com/apache/spark/commit/01d0904542de3487fb65abfb513c15410a6a2b7b).
* This patch passes all 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 #22213: [SPARK-25221][DEPLOY] Consistent trailing whitespace tre...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on the issue:
https://github.com/apache/spark/pull/22213
ok to test
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22213: [SPARK-25221][DEPLOY] Consistent trailing whitesp...
Posted by jerryshao <gi...@git.apache.org>.
Github user jerryshao commented on a diff in the pull request:
https://github.com/apache/spark/pull/22213#discussion_r214244665
--- Diff: core/src/test/scala/org/apache/spark/deploy/SparkSubmitSuite.scala ---
@@ -1144,6 +1144,46 @@ class SparkSubmitSuite
conf1.get(PY_FILES.key) should be (s"s3a://${pyFile.getAbsolutePath}")
conf1.get("spark.submit.pyFiles") should (startWith("/"))
}
+
+ test("handles natural line delimiters in --properties-file and --conf uniformly") {
+ val delimKey = "spark.my.delimiter."
+ val LF = "\n"
+ val CR = "\r"
+
+ val leadingDelimKeyFromFile = s"${delimKey}leadingDelimKeyFromFile" -> s"${LF}blah"
+ val trailingDelimKeyFromFile = s"${delimKey}trailingDelimKeyFromFile" -> s"blah${CR}"
+ val infixDelimFromFile = s"${delimKey}infixDelimFromFile" -> s"${CR}blah${LF}"
+ val nonDelimSpaceFromFile = s"${delimKey}nonDelimSpaceFromFile" -> " blah\f"
--- End diff --
Sorry for the stupid question. I guess I was thinking of something different.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22213: [SPARK-25221][DEPLOY] Consistent trailing whitespace tre...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22213
**[Test build #95920 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/95920/testReport)** for PR 22213 at commit [`76635ce`](https://github.com/apache/spark/commit/76635cef7eab1fceef20fe0f4397460879cb0a43).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22213: [SPARK-25221][DEPLOY] Consistent trailing whitesp...
Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on a diff in the pull request:
https://github.com/apache/spark/pull/22213#discussion_r216093949
--- Diff: core/src/test/scala/org/apache/spark/deploy/SparkSubmitSuite.scala ---
@@ -1144,6 +1144,48 @@ class SparkSubmitSuite
conf1.get(PY_FILES.key) should be (s"s3a://${pyFile.getAbsolutePath}")
conf1.get("spark.submit.pyFiles") should (startWith("/"))
}
+
+ test("handles natural line delimiters in --properties-file and --conf uniformly") {
+ val delimKey = "spark.my.delimiter."
+ val LF = "\n"
+ val CR = "\r"
+
+ val leadingDelimKeyFromFile = s"${delimKey}leadingDelimKeyFromFile" -> s"${LF}blah"
+ val trailingDelimKeyFromFile = s"${delimKey}trailingDelimKeyFromFile" -> s"blah${CR}"
+ val infixDelimFromFile = s"${delimKey}infixDelimFromFile" -> s"${CR}blah${LF}"
+ val nonDelimSpaceFromFile = s"${delimKey}nonDelimSpaceFromFile" -> " blah\f"
+
+ val testProps = Seq(leadingDelimKeyFromFile, trailingDelimKeyFromFile, infixDelimFromFile,
+ nonDelimSpaceFromFile)
+
+ val props = new java.util.Properties()
+ val propsFile = File.createTempFile("test-spark-conf", ".properties",
+ Utils.createTempDir())
+ val propsOutputStream = new FileOutputStream(propsFile)
+ try {
+ testProps.foreach { case (k, v) => props.put(k, v) }
+ props.store(propsOutputStream, "test whitespace")
+ } finally {
+ propsOutputStream.close()
+ }
+
+ val clArgs = Seq(
+ "--class", "org.SomeClass",
+ "--conf", s"${delimKey}=$LF",
+ "--conf", "spark.master=yarn",
+ "--properties-file", propsFile.getPath,
+ "thejar.jar")
+
+ val appArgs = new SparkSubmitArguments(clArgs)
+ val (_, _, conf, _) = submit.prepareSubmitEnvironment(appArgs)
+
+ Seq((delimKey -> LF), leadingDelimKeyFromFile, trailingDelimKeyFromFile, infixDelimFromFile)
+ .foreach {
--- End diff --
nit: `case ... =>` stays with the opening brace.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22213: [SPARK-25221][DEPLOY] Consistent trailing whitespace tre...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22213
**[Test build #95782 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/95782/testReport)** for PR 22213 at commit [`9e4ac10`](https://github.com/apache/spark/commit/9e4ac104dd524e19f1675a383d84a89751da0af7).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22213: [SPARK-25221][DEPLOY] Consistent trailing whitesp...
Posted by jerryshao <gi...@git.apache.org>.
Github user jerryshao commented on a diff in the pull request:
https://github.com/apache/spark/pull/22213#discussion_r212530383
--- Diff: core/src/main/scala/org/apache/spark/util/Utils.scala ---
@@ -2062,8 +2062,10 @@ private[spark] object Utils extends Logging {
try {
val properties = new Properties()
properties.load(inReader)
- properties.stringPropertyNames().asScala.map(
- k => (k, properties.getProperty(k).trim)).toMap
+ properties.stringPropertyNames().asScala
+ .map(k => (k, properties.getProperty(k)))
--- End diff --
@gerashegalov would you please elaborate the use case here? I saw that you're treating `\n` as a property value, what is the specific usage scenario here?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22213: [SPARK-25221][DEPLOY] Consistent trailing whitespace tre...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22213
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/95826/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22213: [SPARK-25221][DEPLOY] Consistent trailing whitespace tre...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22213
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 #22213: [SPARK-25221][DEPLOY] Consistent trailing whitespace tre...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22213
**[Test build #95819 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/95819/testReport)** for PR 22213 at commit [`d341dae`](https://github.com/apache/spark/commit/d341dae0511fd9456ab3e0bd686e11749d43aeb7).
* This patch **fails due to an unknown error code, -9**.
* 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 #22213: [SPARK-25221][DEPLOY] Consistent trailing whitespace tre...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22213
**[Test build #95786 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/95786/testReport)** for PR 22213 at commit [`01d0904`](https://github.com/apache/spark/commit/01d0904542de3487fb65abfb513c15410a6a2b7b).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22213: [SPARK-25221][DEPLOY] Consistent trailing whitespace tre...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22213
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 pull request #22213: [SPARK-25221][DEPLOY] Consistent trailing whitesp...
Posted by jerryshao <gi...@git.apache.org>.
Github user jerryshao commented on a diff in the pull request:
https://github.com/apache/spark/pull/22213#discussion_r213160007
--- Diff: core/src/main/scala/org/apache/spark/util/Utils.scala ---
@@ -2062,8 +2062,10 @@ private[spark] object Utils extends Logging {
try {
val properties = new Properties()
properties.load(inReader)
- properties.stringPropertyNames().asScala.map(
- k => (k, properties.getProperty(k).trim)).toMap
+ properties.stringPropertyNames().asScala
+ .map(k => (k, properties.getProperty(k)))
--- End diff --
>trim removes leading spaces as well that are totally legit.
It is hard to say which solution is legit, the way you proposed may be valid in your case, but it will be unexpected in other user's case. I'm not talking about legit or not, what I'm trying to say is that your proposal will break the convention, that's what I concerned about.
By ASCII I'm you can pass in ASCII number, and translate to actual char in the code, that will mitigate the problem here.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22213: [SPARK-25221][DEPLOY] Consistent trailing whitespace tre...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22213
Can one of the admins verify this patch?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22213: [SPARK-25221][DEPLOY] Consistent trailing whitespace tre...
Posted by steveloughran <gi...@git.apache.org>.
Github user steveloughran commented on the issue:
https://github.com/apache/spark/pull/22213
This actually makes sense. We always forget this, but java properties file format is [more complex than any of us remember](https://docs.oracle.com/javase/10/docs/api/java/util/Properties.html#load(java.io.Reader))
At the time of this trim taking place, all CR/LF chars in the source file will have been stripped through one of
* being the end of an entry: property contains all chars up to that line break (or line skipped if empty/comment)
* being proceeded by a backslash, in which case the following line will have its initial whitespace stripped then joined to the subsequent line.
Whoever did the wikipedia article [did some good examples](https://en.wikipedia.org/wiki/.properties)
What this means is: by the time the spark trim() code is reached, the only CR and LF entries in a property are those from expanding \r and \n character pairs in the actual property itself. All of these within a property, e.g `p1=a\nb` already get through, this extends it to propertlies like `p2=\r`.
* should be able to easy to write some tests for `trimExceptCRLF()` directly, e.g. how it handles odd strings (one char, value == 0), empty string, ...
* There's an XML format for properties too, which should also be tested to see WTF goes on there.
PS, looking up for the properties spec highlights that Java 9 [uses UTF-8 for the properties encoding](https://docs.oracle.com/javase/9/intl/internationalization-enhancements-jdk-9.htm#JSINT-GUID-974CF488-23E8-4963-A322-82006A7A14C7). Don't know of any implications here.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22213: [SPARK-25221][DEPLOY] Consistent trailing whitespace tre...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22213
**[Test build #95820 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/95820/testReport)** for PR 22213 at commit [`603fdc9`](https://github.com/apache/spark/commit/603fdc9c148e2a36393140054c559f5a0a380e5d).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22213: [SPARK-25221][DEPLOY] Consistent trailing whitespace tre...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22213
Can one of the admins verify this patch?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22213: [SPARK-25221][DEPLOY] Consistent trailing whitespace tre...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22213
**[Test build #95826 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/95826/testReport)** for PR 22213 at commit [`603fdc9`](https://github.com/apache/spark/commit/603fdc9c148e2a36393140054c559f5a0a380e5d).
* This patch passes all 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 #22213: [SPARK-25221][DEPLOY] Consistent trailing whitesp...
Posted by gerashegalov <gi...@git.apache.org>.
Github user gerashegalov commented on a diff in the pull request:
https://github.com/apache/spark/pull/22213#discussion_r214237801
--- Diff: core/src/test/scala/org/apache/spark/deploy/SparkSubmitSuite.scala ---
@@ -1144,6 +1144,46 @@ class SparkSubmitSuite
conf1.get(PY_FILES.key) should be (s"s3a://${pyFile.getAbsolutePath}")
conf1.get("spark.submit.pyFiles") should (startWith("/"))
}
+
+ test("handles natural line delimiters in --properties-file and --conf uniformly") {
+ val delimKey = "spark.my.delimiter."
+ val LF = "\n"
+ val CR = "\r"
+
+ val leadingDelimKeyFromFile = s"${delimKey}leadingDelimKeyFromFile" -> s"${LF}blah"
+ val trailingDelimKeyFromFile = s"${delimKey}trailingDelimKeyFromFile" -> s"blah${CR}"
+ val infixDelimFromFile = s"${delimKey}infixDelimFromFile" -> s"${CR}blah${LF}"
+ val nonDelimSpaceFromFile = s"${delimKey}nonDelimSpaceFromFile" -> " blah\f"
--- End diff --
@jerryshao I try not to spend time on issues unrelated to our production deployments. @steveloughran and this PR already pointed at the `Properties#load` method documenting the format.
Line terminator characters can be included using `\r` and `\n` escape sequences. Or you can encode any character using `\uxxxx`
In addition you can take a look at the file generated by this code:
```
#test whitespace
#Thu Aug 30 20:20:33 PDT 2018
spark.my.delimiter.nonDelimSpaceFromFile=\ blah\f
spark.my.delimiter.infixDelimFromFile=\rblah\n
spark.my.delimiter.trailingDelimKeyFromFile=blah\r
spark.my.delimiter.leadingDelimKeyFromFile=\nblah
```
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22213: [SPARK-25221][DEPLOY] Consistent trailing whitespace tre...
Posted by gerashegalov <gi...@git.apache.org>.
Github user gerashegalov commented on the issue:
https://github.com/apache/spark/pull/22213
Thank you for reviews @vanzin @steveloughran @jerryshao @HyukjinKwon
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22213: [SPARK-25221][DEPLOY] Consistent trailing whitesp...
Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on a diff in the pull request:
https://github.com/apache/spark/pull/22213#discussion_r216092017
--- Diff: core/src/main/scala/org/apache/spark/util/Utils.scala ---
@@ -2062,8 +2086,10 @@ private[spark] object Utils extends Logging {
try {
val properties = new Properties()
properties.load(inReader)
- properties.stringPropertyNames().asScala.map(
- k => (k, properties.getProperty(k).trim)).toMap
+ properties.stringPropertyNames().asScala
+ .map(k => (k, trimExceptCRLF(properties.getProperty(k))))
--- End diff --
nit: `.map { k => ... }`
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22213: [SPARK-25221][DEPLOY] Consistent trailing whitesp...
Posted by gerashegalov <gi...@git.apache.org>.
Github user gerashegalov commented on a diff in the pull request:
https://github.com/apache/spark/pull/22213#discussion_r212696259
--- Diff: core/src/main/scala/org/apache/spark/util/Utils.scala ---
@@ -2062,8 +2062,10 @@ private[spark] object Utils extends Logging {
try {
val properties = new Properties()
properties.load(inReader)
- properties.stringPropertyNames().asScala.map(
- k => (k, properties.getProperty(k).trim)).toMap
+ properties.stringPropertyNames().asScala
+ .map(k => (k, properties.getProperty(k)))
--- End diff --
We could deprecate trimming, and have some other config to disable it but I think because it seems like a real bug to have disparity with --conf maybe we can just fix and document it?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22213: [SPARK-25221][DEPLOY] Consistent trailing whitesp...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on a diff in the pull request:
https://github.com/apache/spark/pull/22213#discussion_r216180051
--- Diff: core/src/main/scala/org/apache/spark/util/Utils.scala ---
@@ -2052,6 +2051,30 @@ private[spark] object Utils extends Logging {
}
}
+ /**
+ * Implements the same logic as JDK java.lang.String#trim by removing leading and trailing
--- End diff --
nit: `java.lang.String#trim` -> ``` `java.lang.String#trim` ```
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22213: [SPARK-25221][DEPLOY] Consistent trailing whitespace tre...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22213
**[Test build #95920 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/95920/testReport)** for PR 22213 at commit [`76635ce`](https://github.com/apache/spark/commit/76635cef7eab1fceef20fe0f4397460879cb0a43).
* This patch **fails due to an unknown error code, -9**.
* 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 #22213: [SPARK-25221][DEPLOY] Consistent trailing whitespace tre...
Posted by gerashegalov <gi...@git.apache.org>.
Github user gerashegalov commented on the issue:
https://github.com/apache/spark/pull/22213
rebased
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22213: [SPARK-25221][DEPLOY] Consistent trailing whitesp...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on a diff in the pull request:
https://github.com/apache/spark/pull/22213#discussion_r212528202
--- Diff: core/src/main/scala/org/apache/spark/util/Utils.scala ---
@@ -2062,8 +2062,10 @@ private[spark] object Utils extends Logging {
try {
val properties = new Properties()
properties.load(inReader)
- properties.stringPropertyNames().asScala.map(
- k => (k, properties.getProperty(k).trim)).toMap
+ properties.stringPropertyNames().asScala
+ .map(k => (k, properties.getProperty(k)))
--- End diff --
Isn't this going to break existing apps dependent on trimmed values?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22213: [SPARK-25221][DEPLOY] Consistent trailing whitespace tre...
Posted by steveloughran <gi...@git.apache.org>.
Github user steveloughran commented on the issue:
https://github.com/apache/spark/pull/22213
code LGTM. Clearly its a tangible problem, especially for some one-char option like "myapp.line.separator"
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22213: [SPARK-25221][DEPLOY] Consistent trailing whitesp...
Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on a diff in the pull request:
https://github.com/apache/spark/pull/22213#discussion_r216091436
--- Diff: core/src/main/scala/org/apache/spark/util/Utils.scala ---
@@ -2052,6 +2051,31 @@ private[spark] object Utils extends Logging {
}
}
+
+ private[this] val nonSpaceOrNaturalLineDelimiter: Char => Boolean = {
--- End diff --
This can be declared inside `trimExceptCRLF`.
Also, nit: `ch =>` stays with the opening brace.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22213: [SPARK-25221][DEPLOY] Consistent trailing whitespace tre...
Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on the issue:
https://github.com/apache/spark/pull/22213
Merging to master / 2.4.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22213: [SPARK-25221][DEPLOY] Consistent trailing whitespace tre...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on the issue:
https://github.com/apache/spark/pull/22213
adding @vanzin as well.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22213: [SPARK-25221][DEPLOY] Consistent trailing whitespace tre...
Posted by gerashegalov <gi...@git.apache.org>.
Github user gerashegalov commented on the issue:
https://github.com/apache/spark/pull/22213
thanks for the comment @steveloughran. I'll add more tests for now and see how the discussion goes from there.
as for transition to UTF it means to be fully correct Spark needs to switch to using [strip](http://hg.openjdk.java.net/jdk/jdk/rev/92560438d306) starting JDK11 with or without this PR.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22213: [SPARK-25221][DEPLOY] Consistent trailing whitesp...
Posted by jerryshao <gi...@git.apache.org>.
Github user jerryshao commented on a diff in the pull request:
https://github.com/apache/spark/pull/22213#discussion_r214231103
--- Diff: core/src/test/scala/org/apache/spark/deploy/SparkSubmitSuite.scala ---
@@ -1144,6 +1144,46 @@ class SparkSubmitSuite
conf1.get(PY_FILES.key) should be (s"s3a://${pyFile.getAbsolutePath}")
conf1.get("spark.submit.pyFiles") should (startWith("/"))
}
+
+ test("handles natural line delimiters in --properties-file and --conf uniformly") {
+ val delimKey = "spark.my.delimiter."
+ val LF = "\n"
+ val CR = "\r"
+
+ val leadingDelimKeyFromFile = s"${delimKey}leadingDelimKeyFromFile" -> s"${LF}blah"
+ val trailingDelimKeyFromFile = s"${delimKey}trailingDelimKeyFromFile" -> s"blah${CR}"
+ val infixDelimFromFile = s"${delimKey}infixDelimFromFile" -> s"${CR}blah${LF}"
+ val nonDelimSpaceFromFile = s"${delimKey}nonDelimSpaceFromFile" -> " blah\f"
--- End diff --
@gerashegalov , I'm not sure how do we manually add LF to the end of line using editor to edit property file? Here in your test, it is the program code to explicitly mimic the case, but I don't think in a real scenario, how do we manually update the property file with additional LF or CR?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22213: [SPARK-25221][DEPLOY] Consistent trailing whitespace tre...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22213
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 issue #22213: [SPARK-25221][DEPLOY] Consistent trailing whitespace tre...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22213
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/95782/
Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22213: [SPARK-25221][DEPLOY] Consistent trailing whitespace tre...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22213
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/95820/
Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22213: [SPARK-25221][DEPLOY] Consistent trailing whitespace tre...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22213
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/95819/
Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22213: [SPARK-25221][DEPLOY] Consistent trailing whitespace tre...
Posted by gerashegalov <gi...@git.apache.org>.
Github user gerashegalov commented on the issue:
https://github.com/apache/spark/pull/22213
@steveloughran Regarding XML format, java.util.Properties has its dedicated storeTo/loadFromXML methods which Spark does not use, so we don't need to check this
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22213: [SPARK-25221][DEPLOY] Consistent trailing whitesp...
Posted by gerashegalov <gi...@git.apache.org>.
Github user gerashegalov commented on a diff in the pull request:
https://github.com/apache/spark/pull/22213#discussion_r213049701
--- Diff: core/src/main/scala/org/apache/spark/util/Utils.scala ---
@@ -2062,8 +2062,10 @@ private[spark] object Utils extends Logging {
try {
val properties = new Properties()
properties.load(inReader)
- properties.stringPropertyNames().asScala.map(
- k => (k, properties.getProperty(k).trim)).toMap
+ properties.stringPropertyNames().asScala
+ .map(k => (k, properties.getProperty(k)))
--- End diff --
@jerryshao `trim` removes leading spaces as well that are totally legit.
I also need more info regarding what you mean by ASCII in this context.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22213: [SPARK-25221][DEPLOY] Consistent trailing whitesp...
Posted by gerashegalov <gi...@git.apache.org>.
Github user gerashegalov commented on a diff in the pull request:
https://github.com/apache/spark/pull/22213#discussion_r212695364
--- Diff: core/src/main/scala/org/apache/spark/util/Utils.scala ---
@@ -2062,8 +2062,10 @@ private[spark] object Utils extends Logging {
try {
val properties = new Properties()
properties.load(inReader)
- properties.stringPropertyNames().asScala.map(
- k => (k, properties.getProperty(k).trim)).toMap
+ properties.stringPropertyNames().asScala
+ .map(k => (k, properties.getProperty(k)))
--- End diff --
@HyukjinKwon your concern is valid although it has a simple solution of fixing up the file to your liking. Moreover the user editing a file for --properties-file probably legitimately expects the format prescribed by JDK https://docs.oracle.com/javase/6/docs/api/java/util/Properties.html#load(java.io.Reader)
@jerryshao the use case described in the JIRA is that our customers sometimes have some unusual line delimiters that we need to pass to Hadoop's `TextInputFormat`. In this case it's actually a conventional unix line separator and suppose the customer really insists that only `'\n'` should be the line separator and not the default set `["\n", "\r\n", "\r"]` . We had no issues with configuring it via Spark until we switched from `--conf` to the `--properties-file` to reduce the command line length.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22213: [SPARK-25221][DEPLOY] Consistent trailing whitespace tre...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22213
**[Test build #95819 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/95819/testReport)** for PR 22213 at commit [`d341dae`](https://github.com/apache/spark/commit/d341dae0511fd9456ab3e0bd686e11749d43aeb7).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22213: [SPARK-25221][DEPLOY] Consistent trailing whitespace tre...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22213
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 pull request #22213: [SPARK-25221][DEPLOY] Consistent trailing whitesp...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/spark/pull/22213
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22213: [SPARK-25221][DEPLOY] Consistent trailing whitespace tre...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22213
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 issue #22213: [SPARK-25221][DEPLOY] Consistent trailing whitespace tre...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on the issue:
https://github.com/apache/spark/pull/22213
retest this please
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22213: [SPARK-25221][DEPLOY] Consistent trailing whitespace tre...
Posted by gerashegalov <gi...@git.apache.org>.
Github user gerashegalov commented on the issue:
https://github.com/apache/spark/pull/22213
@witgo please take a look since you worked on #2379
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22213: [SPARK-25221][DEPLOY] Consistent trailing whitespace tre...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22213
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 #22213: [SPARK-25221][DEPLOY] Consistent trailing whitesp...
Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on a diff in the pull request:
https://github.com/apache/spark/pull/22213#discussion_r216093880
--- Diff: core/src/test/scala/org/apache/spark/deploy/SparkSubmitSuite.scala ---
@@ -1144,6 +1144,48 @@ class SparkSubmitSuite
conf1.get(PY_FILES.key) should be (s"s3a://${pyFile.getAbsolutePath}")
conf1.get("spark.submit.pyFiles") should (startWith("/"))
}
+
+ test("handles natural line delimiters in --properties-file and --conf uniformly") {
+ val delimKey = "spark.my.delimiter."
+ val LF = "\n"
+ val CR = "\r"
+
+ val leadingDelimKeyFromFile = s"${delimKey}leadingDelimKeyFromFile" -> s"${LF}blah"
+ val trailingDelimKeyFromFile = s"${delimKey}trailingDelimKeyFromFile" -> s"blah${CR}"
+ val infixDelimFromFile = s"${delimKey}infixDelimFromFile" -> s"${CR}blah${LF}"
+ val nonDelimSpaceFromFile = s"${delimKey}nonDelimSpaceFromFile" -> " blah\f"
+
+ val testProps = Seq(leadingDelimKeyFromFile, trailingDelimKeyFromFile, infixDelimFromFile,
+ nonDelimSpaceFromFile)
+
+ val props = new java.util.Properties()
+ val propsFile = File.createTempFile("test-spark-conf", ".properties",
+ Utils.createTempDir())
+ val propsOutputStream = new FileOutputStream(propsFile)
+ try {
+ testProps.foreach { case (k, v) => props.put(k, v) }
+ props.store(propsOutputStream, "test whitespace")
+ } finally {
+ propsOutputStream.close()
+ }
+
+ val clArgs = Seq(
+ "--class", "org.SomeClass",
+ "--conf", s"${delimKey}=$LF",
--- End diff --
Took a couple of minutes for me to notice this; could you make this similar to the other properties (add a variable to hold its key/value) so that someone reading L1182 knows easily where it comes from?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22213: [SPARK-25221][DEPLOY] Consistent trailing whitespace tre...
Posted by gerashegalov <gi...@git.apache.org>.
Github user gerashegalov commented on the issue:
https://github.com/apache/spark/pull/22213
@jerryshao here is my new take on the problem that should be more acceptable. The premise is that since JDK has already parsed out natural line delimiters '\r' and '\n', the remaining ones are user-provided escaped line delimiters.
@steveloughran would you mind taking a look as well?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22213: [SPARK-25221][DEPLOY] Consistent trailing whitesp...
Posted by gerashegalov <gi...@git.apache.org>.
Github user gerashegalov commented on a diff in the pull request:
https://github.com/apache/spark/pull/22213#discussion_r213488600
--- Diff: core/src/main/scala/org/apache/spark/util/Utils.scala ---
@@ -2062,8 +2062,10 @@ private[spark] object Utils extends Logging {
try {
val properties = new Properties()
properties.load(inReader)
- properties.stringPropertyNames().asScala.map(
- k => (k, properties.getProperty(k).trim)).toMap
+ properties.stringPropertyNames().asScala
+ .map(k => (k, properties.getProperty(k)))
--- End diff --
I have also mentioned that we can make this a conditional logic: https://github.com/apache/spark/pull/22213#discussion_r212696259
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22213: [SPARK-25221][DEPLOY] Consistent trailing whitesp...
Posted by jerryshao <gi...@git.apache.org>.
Github user jerryshao commented on a diff in the pull request:
https://github.com/apache/spark/pull/22213#discussion_r212889779
--- Diff: core/src/main/scala/org/apache/spark/util/Utils.scala ---
@@ -2062,8 +2062,10 @@ private[spark] object Utils extends Logging {
try {
val properties = new Properties()
properties.load(inReader)
- properties.stringPropertyNames().asScala.map(
- k => (k, properties.getProperty(k).trim)).toMap
+ properties.stringPropertyNames().asScala
+ .map(k => (k, properties.getProperty(k)))
--- End diff --
The changes here will break the current assumptions. Some editors will leave the trailing WS without removing it, but in fact user doesn't need that trailing WS, the changes here will break the assumptions, user have to check and remove all the trailing WS to avoid unexpected things.
AFAIK in Hive usually it uses ASCII or others to specify the separator, not "\n" or "\r\n", which will be removed or converted during the parse (which is quite brittle). So this is more like things you could fix in your side, not necessary in Spark side.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22213: [SPARK-25221][DEPLOY] Consistent trailing whitespace tre...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on the issue:
https://github.com/apache/spark/pull/22213
Seems fine to me too.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22213: [SPARK-25221][DEPLOY] Consistent trailing whitespace tre...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22213
**[Test build #95934 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/95934/testReport)** for PR 22213 at commit [`76635ce`](https://github.com/apache/spark/commit/76635cef7eab1fceef20fe0f4397460879cb0a43).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22213: [SPARK-25221][DEPLOY] Consistent trailing whitesp...
Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on a diff in the pull request:
https://github.com/apache/spark/pull/22213#discussion_r216094096
--- Diff: core/src/test/scala/org/apache/spark/util/UtilsSuite.scala ---
@@ -1205,6 +1205,39 @@ class UtilsSuite extends SparkFunSuite with ResetSystemProperties with Logging {
assert(Utils.stringHalfWidth("\u0967\u0968\u0969") == 3)
// scalastyle:on nonascii
}
+
+ test("trimExceptCRLF standalone") {
+ val crlfSet = Set("\r", "\n")
+ val nonPrintableButCRLF =
+ (0 to 32).map(_.toChar.toString).toSet -- crlfSet
+
+ // identity for CRLF
+ crlfSet
+ .foreach(s => Utils.trimExceptCRLF(s) === s)
+
+ // empty for other non-printables
+ nonPrintableButCRLF
+ .foreach(s => assert(Utils.trimExceptCRLF(s) === ""))
+
+ // identity for a printable string
+ assert(Utils.trimExceptCRLF("a") === "a")
+
+ // identity for strings with CRLF
+ crlfSet
+ .foreach { s =>
--- End diff --
nit: keep in previous line (also below).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22213: [SPARK-25221][DEPLOY] Consistent trailing whitespace tre...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22213
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/95920/
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 #22213: [SPARK-25221][DEPLOY] Consistent trailing whitesp...
Posted by gerashegalov <gi...@git.apache.org>.
Github user gerashegalov commented on a diff in the pull request:
https://github.com/apache/spark/pull/22213#discussion_r213489337
--- Diff: core/src/main/scala/org/apache/spark/util/Utils.scala ---
@@ -2062,8 +2062,10 @@ private[spark] object Utils extends Logging {
try {
val properties = new Properties()
properties.load(inReader)
- properties.stringPropertyNames().asScala.map(
- k => (k, properties.getProperty(k).trim)).toMap
+ properties.stringPropertyNames().asScala
+ .map(k => (k, properties.getProperty(k)))
--- End diff --
> By ASCII I mean you can pass in ASCII number, and translate to actual char in the code, that will mitigate the problem here.
I think I'll just keep passing the delimiter via `--conf` to Hadoop and everything else in a single properties to avoid dealing with manual conversion of ints to char.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22213: [SPARK-25221][DEPLOY] Consistent trailing whitespace tre...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22213
**[Test build #95820 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/95820/testReport)** for PR 22213 at commit [`603fdc9`](https://github.com/apache/spark/commit/603fdc9c148e2a36393140054c559f5a0a380e5d).
* 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 #22213: [SPARK-25221][DEPLOY] Consistent trailing whitespace tre...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22213
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/95786/
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 #22213: [SPARK-25221][DEPLOY] Consistent trailing whitesp...
Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on a diff in the pull request:
https://github.com/apache/spark/pull/22213#discussion_r216094051
--- Diff: core/src/test/scala/org/apache/spark/util/UtilsSuite.scala ---
@@ -1205,6 +1205,39 @@ class UtilsSuite extends SparkFunSuite with ResetSystemProperties with Logging {
assert(Utils.stringHalfWidth("\u0967\u0968\u0969") == 3)
// scalastyle:on nonascii
}
+
+ test("trimExceptCRLF standalone") {
+ val crlfSet = Set("\r", "\n")
+ val nonPrintableButCRLF =
+ (0 to 32).map(_.toChar.toString).toSet -- crlfSet
+
+ // identity for CRLF
+ crlfSet
+ .foreach(s => Utils.trimExceptCRLF(s) === s)
--- End diff --
nit. `.foreach { s => ... }` (also in others)
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22213: [SPARK-25221][DEPLOY] Consistent trailing whitespace tre...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22213
**[Test build #95934 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/95934/testReport)** for PR 22213 at commit [`76635ce`](https://github.com/apache/spark/commit/76635cef7eab1fceef20fe0f4397460879cb0a43).
* This patch passes all 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 #22213: [SPARK-25221][DEPLOY] Consistent trailing whitespace tre...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22213
**[Test build #95782 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/95782/testReport)** for PR 22213 at commit [`9e4ac10`](https://github.com/apache/spark/commit/9e4ac104dd524e19f1675a383d84a89751da0af7).
* 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 #22213: [SPARK-25221][DEPLOY] Consistent trailing whitespace tre...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22213
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 issue #22213: [SPARK-25221][DEPLOY] Consistent trailing whitespace tre...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on the issue:
https://github.com/apache/spark/pull/22213
retest this please
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22213: [SPARK-25221][DEPLOY] Consistent trailing whitespace tre...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22213
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/95934/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22213: [SPARK-25221][DEPLOY] Consistent trailing whitespace tre...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22213
Can one of the admins verify this patch?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22213: [SPARK-25221][DEPLOY] Consistent trailing whitespace tre...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22213
**[Test build #95826 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/95826/testReport)** for PR 22213 at commit [`603fdc9`](https://github.com/apache/spark/commit/603fdc9c148e2a36393140054c559f5a0a380e5d).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org