Skip to content

[SPARK-28586][INFRA] Make merge-spark-pr script compatible with Python 3 - #25322

Closed
HyukjinKwon wants to merge 1 commit into
apache:masterfrom
HyukjinKwon:merge-script
Closed

[SPARK-28586][INFRA] Make merge-spark-pr script compatible with Python 3#25322
HyukjinKwon wants to merge 1 commit into
apache:masterfrom
HyukjinKwon:merge-script

Conversation

@HyukjinKwon

@HyukjinKwonHyukjinKwon commented Aug 1, 2019

Copy link
Copy Markdown
Member

What changes were proposed in this pull request?

This PR proposes to make merge_spark_pr.py script Python 3 compatible.

How was this patch tested?

Manually tested against my forked remote with the PR and JIRA below:

#25321
#25286
https://issues.apache.org/jira/browse/SPARK-28153

@maropu

maropu commented Aug 1, 2019

Copy link
Copy Markdown
Member

We still need to keep the Python2 support?

@HyukjinKwon

HyukjinKwon commented Aug 1, 2019

Copy link
Copy Markdown
MemberAuthor

Yea, it's deprecated but not removed out yet.

@HyukjinKwon

HyukjinKwon commented Aug 1, 2019•
edited
Loading

Copy link
Copy Markdown
MemberAuthor

Let me cc Spark committers to inform although it might be a bit of noise .. because I know it's annoying to use this script with Python 2 in Mac due to some pre-installed lib issues. After this PR, we can now use Python 3 for this script. cc @apache/spark-committers.

@dongjoon-hyun

dongjoon-hyun commented Aug 1, 2019

Copy link
Copy Markdown
Member

Nice, @HyukjinKwon !

dongjoon-hyun
dongjoon-hyun reviewed Aug 1, 2019
View reviewed changes
Comment threaddev/merge_spark_pr.py
dongjoon-hyun
dongjoon-hyun reviewed Aug 1, 2019
View reviewed changes
Comment threaddev/merge_spark_pr.py
@SparkQA

This comment has been minimized.

Sign in to view
@SparkQA

This comment has been minimized.

Sign in to view
@SparkQA

SparkQA commented Aug 1, 2019

Copy link
Copy Markdown

Test build #108506 has finished for PR 25322 at commit 11c3c7f.

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

dongjoon-hyun
dongjoon-hyun approved these changes Aug 1, 2019
View reviewed changes

@dongjoon-hyundongjoon-hyun left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

+1, LGTM. Merged to master.
Thank you so much, @HyukjinKwon , @maropu , @srowen !

@HyukjinKwon
HyukjinKwon deleted the merge-script branch August 2, 2019 02:21
@BryanCutler

BryanCutler commented Aug 2, 2019

Copy link
Copy Markdown
Member

Great! thanks @HyukjinKwon

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

Reviewers

@srowensrowensrowen approved these changes
@dongjoon-hyundongjoon-hyundongjoon-hyun approved these changes

Assignees

No one assigned

Labels

PROJECT INFRA

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@HyukjinKwon@maropu@dongjoon-hyun@SparkQA@BryanCutler@srowen