[GitHub] spark pull request #22782: [HOTFIX] Fix PySpark pip packaging tests by non-a...
Github user rvesse commented on a diff in the pull request:
https://github.com/apache/spark/pull/22782#discussion_r227373891
--- Diff: bin/docker-image-tool.sh ---
@@ -79,7 +79,7 @@ function build {
fi
# Verify that Spark has actually been built/is a runnable distribution
- #Â i.e. the Spark JARs that the Docker files will place into the image
are present
--- End diff --
It wasn't as if I used a non-breaking space intentionally, I just used my
OSes default editor for Shell scripts!
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #22782: [HOTFIX] Fix PySpark pip packaging tests by non-a...
Github user HyukjinKwon commented on a diff in the pull request:
https://github.com/apache/spark/pull/22782#discussion_r227336895
--- Diff: bin/docker-image-tool.sh ---
@@ -79,7 +79,7 @@ function build {
fi
# Verify that Spark has actually been built/is a runnable distribution
- #Â i.e. the Spark JARs that the Docker files will place into the image
are present
--- End diff --
Therefore, using non-breakable spaces in the codes is obviously not a good
practice. Please avoid next time.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #22782: [HOTFIX] Fix PySpark pip packaging tests by non-a...
Github user HyukjinKwon commented on a diff in the pull request:
https://github.com/apache/spark/pull/22782#discussion_r227336318
--- Diff: bin/docker-image-tool.sh ---
@@ -79,7 +79,7 @@ function build {
fi
# Verify that Spark has actually been built/is a runnable distribution
- #Â i.e. the Spark JARs that the Docker files will place into the image
are present
--- End diff --
For the issue itself, It's related to a historical reason for Python.
Python 2 supported `str` type as bytes like string. It looked a mistake that
confuses users about the concept between bytes and string, and then Python 3
introduced `str` as unicode strings concepts like other programing languages.
`open(...).read()` reads it as `str` (which is bytes) in Python 2 but it's
read in unicode strings in Python 3 - where we need an implicit conversion
between bytes and strings. Looks it had to be to minimise the breaking changes
in users codes.
So, bytes to string conversion happened here and unfortunately our
Jenkins's system default encoding is set to ascii (even though arguably UTF-8
is common).
For non-ascii itself, please see the justification at
http://www.scalastyle.org/rules-dev.html in ScalaStyle.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #22782: [HOTFIX] Fix PySpark pip packaging tests by non-a...
Github user rvesse commented on a diff in the pull request:
https://github.com/apache/spark/pull/22782#discussion_r227274978
--- Diff: bin/docker-image-tool.sh ---
@@ -79,7 +79,7 @@ function build {
fi
# Verify that Spark has actually been built/is a runnable distribution
- #Â i.e. the Spark JARs that the Docker files will place into the image
are present
--- End diff --
Obvious question but why are we still using ASCII encoding for anything?
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #22782: [HOTFIX] Fix PySpark pip packaging tests by non-a...
Github user HyukjinKwon commented on a diff in the pull request:
https://github.com/apache/spark/pull/22782#discussion_r227173103
--- Diff: bin/docker-image-tool.sh ---
@@ -79,7 +79,7 @@ function build {
fi
# Verify that Spark has actually been built/is a runnable distribution
- #Â i.e. the Spark JARs that the Docker files will place into the image
are present
--- End diff --
It was more because non-breakable space was used instead instead of space.
It was in utf-8 but non ascii compatible.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #22782: [HOTFIX] Fix PySpark pip packaging tests by non-a...
Github user rvesse commented on a diff in the pull request:
https://github.com/apache/spark/pull/22782#discussion_r226983785
--- Diff: bin/docker-image-tool.sh ---
@@ -79,7 +79,7 @@ function build {
fi
# Verify that Spark has actually been built/is a runnable distribution
- #Â i.e. the Spark JARs that the Docker files will place into the image
are present
--- End diff --
@dongjoon-hyun This was edited in Xcode on OS X so almost certainly
defaulting to UTF-8 encoding
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #22782: [HOTFIX] Fix PySpark pip packaging tests by non-a...
Github user asfgit closed the pull request at: https://github.com/apache/spark/pull/22782 --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #22782: [HOTFIX] Fix PySpark pip packaging tests by non-a...
Github user dongjoon-hyun commented on a diff in the pull request:
https://github.com/apache/spark/pull/22782#discussion_r226834220
--- Diff: bin/docker-image-tool.sh ---
@@ -79,7 +79,7 @@ function build {
fi
# Verify that Spark has actually been built/is a runnable distribution
- #Â i.e. the Spark JARs that the Docker files will place into the image
are present
--- End diff --
Right. We need some automatic check for scripts too. Anyway, thank you for
fixing this, @HyukjinKwon ! This blocks almost everything PRs.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #22782: [HOTFIX] Fix PySpark pip packaging tests by non-a...
Github user HyukjinKwon commented on a diff in the pull request:
https://github.com/apache/spark/pull/22782#discussion_r226833951
--- Diff: bin/docker-image-tool.sh ---
@@ -79,7 +79,7 @@ function build {
fi
# Verify that Spark has actually been built/is a runnable distribution
- #Â i.e. the Spark JARs that the Docker files will place into the image
are present
--- End diff --
Yea, some text editors insert non-breakable spaces and probably that's why.
I dealt with similar problems at md files before. I think we have scalastyle
for nonascii but looks not for scripts ..
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #22782: [HOTFIX] Fix PySpark pip packaging tests by non-a...
Github user dongjoon-hyun commented on a diff in the pull request:
https://github.com/apache/spark/pull/22782#discussion_r226833751
--- Diff: bin/docker-image-tool.sh ---
@@ -79,7 +79,7 @@ function build {
fi
# Verify that Spark has actually been built/is a runnable distribution
- #Â i.e. the Spark JARs that the Docker files will place into the image
are present
--- End diff --
Hi, @rvesse .
Do you have any clue about the special characters after '#' found by @kiszk
at [here](https://github.com/apache/spark/pull/22782#issuecomment-431599457)?
I'm wondering if we can avoid this situation in the future. Do you have any
idea?
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #22782: [HOTFIX] Fix PySpark pip packaging tests by non-a...
Github user HyukjinKwon commented on a diff in the pull request: https://github.com/apache/spark/pull/22782#discussion_r226833113 --- Diff: python/pyspark/__init__.py --- @@ -16,7 +16,7 @@ # """ -PySpark is the Python API for Spark. +PySpark is the Python API for Spark --- End diff -- yup. I will revert this one too. just intended to test Python tests only since Scala tests takes long. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #22782: [HOTFIX] Fix PySpark pip packaging tests by non-a...
Github user kiszk commented on a diff in the pull request:
https://github.com/apache/spark/pull/22782#discussion_r226833103
--- Diff: dev/run-tests.py ---
@@ -551,7 +551,8 @@ def main():
if not changed_files or any(f.endswith(".scala")
or f.endswith("scalastyle-config.xml")
for f in changed_files):
-run_scala_style_checks()
+# run_scala_style_checks()
--- End diff --
Got it
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #22782: [HOTFIX] Fix PySpark pip packaging tests by non-a...
Github user HyukjinKwon commented on a diff in the pull request:
https://github.com/apache/spark/pull/22782#discussion_r226833051
--- Diff: dev/run-tests.py ---
@@ -551,7 +551,8 @@ def main():
if not changed_files or any(f.endswith(".scala")
or f.endswith("scalastyle-config.xml")
for f in changed_files):
-run_scala_style_checks()
+# run_scala_style_checks()
--- End diff --
tentative workaround. yup yup. I will revert all back once the tests pass
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #22782: [HOTFIX] Fix PySpark pip packaging tests by non-a...
Github user kiszk commented on a diff in the pull request: https://github.com/apache/spark/pull/22782#discussion_r226833042 --- Diff: python/pyspark/__init__.py --- @@ -16,7 +16,7 @@ # """ -PySpark is the Python API for Spark. +PySpark is the Python API for Spark --- End diff -- Is this intentional change? --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #22782: [HOTFIX] Fix PySpark pip packaging tests by non-a...
Github user kiszk commented on a diff in the pull request:
https://github.com/apache/spark/pull/22782#discussion_r226833028
--- Diff: dev/run-tests.py ---
@@ -551,7 +551,8 @@ def main():
if not changed_files or any(f.endswith(".scala")
or f.endswith("scalastyle-config.xml")
for f in changed_files):
-run_scala_style_checks()
+# run_scala_style_checks()
--- End diff --
Is this change necessary? Or just tentative workaround?
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
