Skip to content

[SPARK-33155][K8S] spark.kubernetes.pyspark.pythonVersion allows only '3' #30049

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
dongjoon-hyun wants to merge 5 commits into from
Closed

[SPARK-33155][K8S] spark.kubernetes.pyspark.pythonVersion allows only '3' #30049

dongjoon-hyun wants to merge 5 commits into from

Conversation

@dongjoon-hyun
Copy link
Member

@dongjoon-hyun dongjoon-hyun commented Oct 15, 2020 •
edited
Loading

What changes were proposed in this pull request?

This PR makes spark.kubernetes.pyspark.pythonVersion allow only 3. In other words, it will reject 2 for Python 2.

  • Configuration description and check is updated.
  • Documentation is updated
  • Unit test cases are updated.
  • Docker image script is updated.

Why are the changes needed?

After SPARK-32138, Apache Spark 3.1 dropped Python 2 support.

Does this PR introduce any user-facing change?

Yes, but Python 2 support is already dropped officially.

How was this patch tested?

Pass the CI.

@dongjoon-hyun dongjoon-hyun changed the title [SPARK-XXX][K8S][DOC] spark.kubernetes.pyspark.pythonVersion allows only '3' [SPARK-XXX][K8S] spark.kubernetes.pyspark.pythonVersion allows only '3' Oct 15, 2020
@dongjoon-hyun dongjoon-hyun changed the title [SPARK-XXX][K8S] spark.kubernetes.pyspark.pythonVersion allows only '3' [SPARK-33155][K8S] spark.kubernetes.pyspark.pythonVersion allows only '3' Oct 15, 2020
@dongjoon-hyun
Copy link
Member Author

dongjoon-hyun commented Oct 15, 2020

Hi, @holdenk and @HyukjinKwon .
Could you review this PR?

@SparkQA

This comment has been minimized.

Sign in to view
@SparkQA

This comment has been minimized.

Sign in to view
@SparkQA
Copy link

SparkQA commented Oct 15, 2020

Test build #129803 has finished for PR 30049 at commit cfa9a2c.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented Oct 15, 2020

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/34405/

@SparkQA
Copy link

SparkQA commented Oct 15, 2020

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/34407/

@SparkQA
Copy link

SparkQA commented Oct 15, 2020

Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/34405/

@SparkQA
Copy link

SparkQA commented Oct 15, 2020

Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/34407/

@dongjoon-hyun
Copy link
Member Author

dongjoon-hyun commented Oct 15, 2020

Retest this please.

@SparkQA
Copy link

SparkQA commented Oct 15, 2020

Test build #129814 has finished for PR 30049 at commit cfa9a2c.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

HyukjinKwon
HyukjinKwon approved these changes Oct 15, 2020
View reviewed changes
Copy link
Member

@HyukjinKwon HyukjinKwon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks we should change;

spark/resource-managers/kubernetes/integration-tests/tests/pyfiles.py

Line 34 in 4ad9bfd

version_check(sys.argv[1], 2 if sys.argv[1] == "python" else 3)

to

version_check(sys.argv[1], 3)

It was added from 1a644af#diff-ea97a33dc022bc99ad9f0b0f0031a3f9929aa2dec8a7e2e41b5bc8795fcab325R36

cc @viirya FYI from 37eedf6

LGTM otherwise

@SparkQA
Copy link

SparkQA commented Oct 15, 2020

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/34419/

@SparkQA
Copy link

SparkQA commented Oct 15, 2020

Test build #129820 has finished for PR 30049 at commit a7ccc1c.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@dongjoon-hyun
Copy link
Member Author

dongjoon-hyun commented Oct 15, 2020

Thank you so much, @HyukjinKwon !

@SparkQA
Copy link

SparkQA commented Oct 15, 2020

Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/34419/

@dongjoon-hyun
Copy link
Member Author

dongjoon-hyun commented Oct 15, 2020

The test passed already in the previous commit. The last commit doesn't affect K8s IT result.
Merged to master.

@dongjoon-hyun dongjoon-hyun deleted the SPARK-DROP-PYTHON2 branch October 15, 2020 08:51
@SparkQA
Copy link

SparkQA commented Oct 15, 2020

Test build #129822 has finished for PR 30049 at commit d500290.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@AmplabJenkins
Copy link

AmplabJenkins commented Oct 15, 2020

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/34427/
Test FAILed.

viirya
viirya reviewed Oct 15, 2020
View reviewed changes
docs/running-on-kubernetes.md
<td><code>"3"</code></td>
<td>
This sets the major Python version of the docker image used to run the driver and executor containers. Can either be 2 or 3.
This sets the major Python version of the docker image used to run the driver and executor containers. Can be 3.
Copy link
Member

@viirya viirya Oct 15, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: I think Only 3 is available for Python3 like below looks better.

viirya
viirya reviewed Oct 15, 2020
View reviewed changes
Copy link
Member

@viirya viirya left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm too, just one minor comment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@viirya viirya viirya left review comments

@HyukjinKwon HyukjinKwon HyukjinKwon approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@dongjoon-hyun @SparkQA @AmplabJenkins @viirya @HyukjinKwon