You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by HyukjinKwon <gi...@git.apache.org> on 2017/10/18 02:22:52 UTC
[GitHub] spark pull request #19524: [SPARK-22302][INFRA] Remove manual backports for ...
GitHub user HyukjinKwon opened a pull request:
https://github.com/apache/spark/pull/19524
[SPARK-22302][INFRA] Remove manual backports for subprocess and print explicit message for < Python 2.7
## What changes were proposed in this pull request?
Seems there is a mistake from missing import for `subprocess.call` which should be used for Python 2.6 backports for some missing functions in `subprocess`. Reproduction is:
```
cd dev && python2.6
```
```
>>> from sparktestsupport import shellutils
>>> shellutils.subprocess_check_call("ls")
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
File "sparktestsupport/shellutils.py", line 46, in subprocess_check_call
retcode = call(*popenargs, **kwargs)
NameError: global name 'call' is not defined
```
For Jenkins logs, please see https://amplab.cs.berkeley.edu/jenkins/job/NewSparkPullRequestBuilder/3950/console
Since we dropped the Python 2.6.x support, looks better we remove those workarounds and print out explicit error messages in order to duplicate the efforts to find out the root causes for such cases.
## How was this patch tested?
Manually tested:
```
cd dev && python2.6
```
```
>>> from sparktestsupport import shellutils
[error] Python versions prior to 2.7 are not supported.
```
```
cd dev && python2.7
```
```
>>> from sparktestsupport import shellutils
>>> shellutils.subprocess_check_output("ls")
'README.md\nappveyor-guide.md\nappveyor-install-dependencies.ps1\nchange-scala-version.sh\ncheck-license\ncheckstyle-suppressions.xml\ncheckstyle.xml\ncreate-release\ndeps\ngithub_jira_sync.py\ngithub_jira_sync.pyc\nlint-java\nlint-python\nlint-r\nlint-r-report.log\nlint-r.R\nlint-scala\nmake-distribution.sh\nmerge_spark_pr.py\nmerge_spark_pr.pyc\nmerge_spark_pr_jira.py\nmerge_spark_pr_jira.pyc\nmima\npep8-1.7.0.py\npep8-1.7.0.pyc\npip-sanity-check.py\npip-sanity-check.pyc\nrequirements.txt\nrun-pip-tests\nrun-tests\nrun-tests-jenkins\nrun-tests-jenkins.py\nrun-tests-jenkins.pyc\nrun-tests.py\nrun-tests.pyc\nscalastyle\nsparktestsupport\ntest-dependencies.sh\ntests\ntox.ini\n'
>>> shellutils.subprocess_check_call("ls")
README.md checkstyle-suppressions.xml github_jira_sync.pyc lint-r.R merge_spark_pr_jira.py pip-sanity-check.py run-tests-jenkins scalastyle
appveyor-guide.md checkstyle.xml lint-java lint-scala merge_spark_pr_jira.pyc pip-sanity-check.pyc run-tests-jenkins.py sparktestsupport
appveyor-install-dependencies.ps1 create-release lint-python make-distribution.sh mima requirements.txt run-tests-jenkins.pyc test-dependencies.sh
change-scala-version.sh deps lint-r merge_spark_pr.py pep8-1.7.0.py run-pip-tests run-tests.py tests
check-license github_jira_sync.py lint-r-report.log merge_spark_pr.pyc pep8-1.7.0.pyc run-tests run-tests.pyc tox.ini
0
```
```
cd dev && python3.6
```
```
>>> from sparktestsupport import shellutils
>>> shellutils.subprocess_check_output("ls")
b'README.md\nappveyor-guide.md\nappveyor-install-dependencies.ps1\nchange-scala-version.sh\ncheck-license\ncheckstyle-suppressions.xml\ncheckstyle.xml\ncreate-release\ndeps\ngithub_jira_sync.py\ngithub_jira_sync.pyc\nlint-java\nlint-python\nlint-r\nlint-r-report.log\nlint-r.R\nlint-scala\nmake-distribution.sh\nmerge_spark_pr.py\nmerge_spark_pr.pyc\nmerge_spark_pr_jira.py\nmerge_spark_pr_jira.pyc\nmima\npep8-1.7.0.py\npep8-1.7.0.pyc\npip-sanity-check.py\npip-sanity-check.pyc\nrequirements.txt\nrun-pip-tests\nrun-tests\nrun-tests-jenkins\nrun-tests-jenkins.py\nrun-tests-jenkins.pyc\nrun-tests.py\nrun-tests.pyc\nscalastyle\nsparktestsupport\ntest-dependencies.sh\ntests\ntox.ini\n'
>>> shellutils.subprocess_check_call("ls")
README.md checkstyle-suppressions.xml github_jira_sync.pyc lint-r.R merge_spark_pr_jira.py pip-sanity-check.py run-tests-jenkins scalastyle
appveyor-guide.md checkstyle.xml lint-java lint-scala merge_spark_pr_jira.pyc pip-sanity-check.pyc run-tests-jenkins.py sparktestsupport
appveyor-install-dependencies.ps1 create-release lint-python make-distribution.sh mima requirements.txt run-tests-jenkins.pyc test-dependencies.sh
change-scala-version.sh deps lint-r merge_spark_pr.py pep8-1.7.0.py run-pip-tests run-tests.py tests
check-license github_jira_sync.py lint-r-report.log merge_spark_pr.pyc pep8-1.7.0.pyc run-tests run-tests.pyc tox.ini
0
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/HyukjinKwon/spark SPARK-22302
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/19524.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 #19524
----
----
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #19524: [SPARK-22302][INFRA] Remove manual backports for ...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on a diff in the pull request:
https://github.com/apache/spark/pull/19524#discussion_r145593044
--- Diff: dev/run-tests ---
@@ -20,4 +20,10 @@
FWDIR="$(cd "`dirname $0`"/..; pwd)"
cd "$FWDIR"
+PYTHON_VERSION_CHECK=$(python -c 'import sys; print(sys.version_info < (2, 7, 0))')
--- End diff --
@holdenk and @shaneknapp, looks I can't just check this within Python scripts.
As we know from the previous issue, `dev/run-tests.py` is not compatible with Python 2.6.x due to dictionary comprehension syntax:
```
python2.6 dev/run-tests.py
```
```
File "dev/run-tests.py", line 128
{m: set(m.dependencies).intersection(modules_to_test) for m in modules_to_test}, sort=True)
^
SyntaxError: invalid syntax
```
I tried to change this as below:
```
git diff dev/run-tests.py
```
```diff
diff --git a/dev/run-tests.py b/dev/run-tests.py
index 72d148d7ea0..dcec912ae23 100755
--- a/dev/run-tests.py
+++ b/dev/run-tests.py
@@ -27,6 +27,10 @@ import sys
import subprocess
from collections import namedtuple
+if sys.version_info < (2, 7):
+ print("[error] Python versions prior to 2.7 are not supported.")
+ sys.exit(-1)
+
from sparktestsupport import SPARK_HOME, USER_HOME, ERROR_CODES
from sparktestsupport.shellutils import exit_from_command_with_retcode, run_cmd, rm_r, which
from sparktestsupport.toposort import toposort_flatten, toposort
```
but it still gives syntax error ahead:
```
python2.6 dev/run-tests.py
```
```
File "dev/run-tests.py", line 128
{m: set(m.dependencies).intersection(modules_to_test) for m in modules_to_test}, sort=True)
^
SyntaxError: invalid syntax
```
I think we should not force to workaround for Python 2.6 syntax error as it was dropped. So, I just tried to fix both `dev/run-tests` and `dev/run-tests-jenkins` as proposed currently.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19524: [SPARK-22302][INFRA] Remove manual backports for subproc...
Posted by shaneknapp <gi...@git.apache.org>.
Github user shaneknapp commented on the issue:
https://github.com/apache/spark/pull/19524
yeah, now that i'm looking at this w/a fresh cup of coffee, i agree w/ @holdenk about shellutil.py being a weird place to exit for a python version check.
run-tests.py and run-tests-jenkins.py would definitely make more sense... we can probably remove the if: statement in shellutil.py altogether (leaving, of course, the subprocess variable names there).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #19524: [SPARK-22302][INFRA] Remove manual backports for ...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/spark/pull/19524
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19524: [SPARK-22302][INFRA] Remove manual backports for subproc...
Posted by holdenk <gi...@git.apache.org>.
Github user holdenk commented on the issue:
https://github.com/apache/spark/pull/19524
Looks reasonable, my only thing is putting in a forced exited inside of shellutil.py is sort of a random place to the version check.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19524: [SPARK-22302][INFRA] Remove manual backports for subproc...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on the issue:
https://github.com/apache/spark/pull/19524
Merged to master.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19524: [SPARK-22302][INFRA] Remove manual backports for subproc...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on the issue:
https://github.com/apache/spark/pull/19524
Sure, thanks. Let me update soon.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19524: [SPARK-22302][INFRA] Remove manual backports for subproc...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/19524
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 #19524: [SPARK-22302][INFRA] Remove manual backports for subproc...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/19524
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/82918/
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 #19524: [SPARK-22302][INFRA] Remove manual backports for ...
Posted by shaneknapp <gi...@git.apache.org>.
Github user shaneknapp commented on a diff in the pull request:
https://github.com/apache/spark/pull/19524#discussion_r145757705
--- Diff: dev/run-tests-jenkins ---
@@ -26,4 +26,11 @@ FWDIR="$( cd "$( dirname "$0" )/.." && pwd )"
cd "$FWDIR"
export PATH=/home/anaconda/bin:$PATH
--- End diff --
actually, we set this in the jenkins worker config, spark PRB build config, and here... we can safely remove this line.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19524: [SPARK-22302][INFRA] Remove manual backports for subproc...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/19524
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/82871/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19524: [SPARK-22302][INFRA] Remove manual backports for subproc...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/19524
**[Test build #82871 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/82871/testReport)** for PR 19524 at commit [`26eeae1`](https://github.com/apache/spark/commit/26eeae140093896155d0b7196cd2de7dd4c6bda6).
* 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 #19524: [SPARK-22302][INFRA] Remove manual backports for subproc...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/19524
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 #19524: [SPARK-22302][INFRA] Remove manual backports for subproc...
Posted by shaneknapp <gi...@git.apache.org>.
Github user shaneknapp commented on the issue:
https://github.com/apache/spark/pull/19524
yeah, this makes sense. i don't think we've officially supported 2.6 in a while, esp for tests and i'd be ok w/removing the backports. this makes for a much clearer exit case.
@rxin @sameeragarwal
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19524: [SPARK-22302][INFRA] Remove manual backports for subproc...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/19524
**[Test build #82918 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/82918/testReport)** for PR 19524 at commit [`de2a70b`](https://github.com/apache/spark/commit/de2a70b2279d214022b1d575270ff7a38764d26f).
* 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 #19524: [SPARK-22302][INFRA] Remove manual backports for subproc...
Posted by rxin <gi...@git.apache.org>.
Github user rxin commented on the issue:
https://github.com/apache/spark/pull/19524
seems fine.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19524: [SPARK-22302][INFRA] Remove manual backports for subproc...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on the issue:
https://github.com/apache/spark/pull/19524
Thanks for your review @shaneknapp.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19524: [SPARK-22302][INFRA] Remove manual backports for subproc...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/19524
**[Test build #82871 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/82871/testReport)** for PR 19524 at commit [`26eeae1`](https://github.com/apache/spark/commit/26eeae140093896155d0b7196cd2de7dd4c6bda6).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19524: [SPARK-22302][INFRA] Remove manual backports for subproc...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/19524
**[Test build #82918 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/82918/testReport)** for PR 19524 at commit [`de2a70b`](https://github.com/apache/spark/commit/de2a70b2279d214022b1d575270ff7a38764d26f).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19524: [SPARK-22302][INFRA] Remove manual backports for subproc...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/19524
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/82900/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19524: [SPARK-22302][INFRA] Remove manual backports for subproc...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/19524
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 #19524: [SPARK-22302][INFRA] Remove manual backports for subproc...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on the issue:
https://github.com/apache/spark/pull/19524
@holdenk, hm, should I maybe make add this to `run-tests.py` and `run-tests-jenkins.py`? I wasn't sure where I should put this.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19524: [SPARK-22302][INFRA] Remove manual backports for subproc...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/19524
**[Test build #82900 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/82900/testReport)** for PR 19524 at commit [`c56793b`](https://github.com/apache/spark/commit/c56793bac07493372f9bb360f6964032641c4867).
* 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 #19524: [SPARK-22302][INFRA] Remove manual backports for subproc...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on the issue:
https://github.com/apache/spark/pull/19524
cc @JoshRosen, @holdenk and @shaneknapp, could you take a look and see if makes sense please?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19524: [SPARK-22302][INFRA] Remove manual backports for subproc...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/19524
**[Test build #82900 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/82900/testReport)** for PR 19524 at commit [`c56793b`](https://github.com/apache/spark/commit/c56793bac07493372f9bb360f6964032641c4867).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org