Skip to content

[SPARK-29646][BUILD] Allow pyspark version name format ${versionNumber}-preview in release script #26306

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
wants to merge 2 commits into from
Closed

[SPARK-29646][BUILD] Allow pyspark version name format ${versionNumber}-preview in release script #26306

wants to merge 2 commits into from

Conversation

@jiangxb1987
Copy link
Contributor

@jiangxb1987 jiangxb1987 commented Oct 30, 2019

What changes were proposed in this pull request?

Update release-build.sh, to allow pyspark version name format ${versionNumber}-preview, otherwise the release script won't generate pyspark release tarballs.

How was this patch tested?

Tested locally.

dongjoon-hyun
dongjoon-hyun approved these changes Oct 30, 2019
View reviewed changes
Copy link
Member

@dongjoon-hyun dongjoon-hyun left a comment

Choose a reason for hiding this comment

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

+1, LGTM. Thank you, @jiangxb1987 .

@SparkQA
Copy link

SparkQA commented Oct 30, 2019

Test build #112884 has finished for PR 26306 at commit 68c5181.

  • This patch fails due to an unknown error code, -9.
  • This patch merges cleanly.
  • This patch adds no public classes.

dongjoon-hyun
dongjoon-hyun reviewed Oct 30, 2019
View reviewed changes
dev/create-release/release-build.sh Outdated
# Write out the VERSION to PySpark version info we rewrite the - into a . and SNAPSHOT
# to dev0 to be closer to PEP440.
PYSPARK_VERSION=`echo "$SPARK_VERSION" | sed -r "s/-/./" | sed -r "s/SNAPSHOT/dev0/"`
PYSPARK_VERSION=`echo "$SPARK_VERSION" | sed -r "s/-/./" | sed -r "s/SNAPSHOT/dev0/" | sed -r "s/preview/dev0/"`
Copy link
Member

@dongjoon-hyun dongjoon-hyun Oct 30, 2019

Choose a reason for hiding this comment

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

Oops. @jiangxb1987 .
This is used here.

PYTHON_DIST_NAME=pyspark-$PYSPARK_VERSION.tar.gz

Copy link
Member

@dongjoon-hyun dongjoon-hyun Oct 30, 2019

Choose a reason for hiding this comment

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

You will generate pyspark-3.0.0.tar.gz instead of pyspark-3.0.0-preview.tar.gz

Copy link
Contributor Author

@jiangxb1987 jiangxb1987 Oct 30, 2019

Choose a reason for hiding this comment

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

IIUC it would be pyspark-3.0.0.dev0.tar.gz which is expected ?

Copy link
Member

@dongjoon-hyun dongjoon-hyun Oct 30, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

pyspark-3.0.0-preview.tar.gz?

Copy link
Member

@dongjoon-hyun dongjoon-hyun Oct 30, 2019

Choose a reason for hiding this comment

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

At least for tarball name?

Copy link
Member

@kiszk kiszk Oct 30, 2019

Choose a reason for hiding this comment

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

super nit: How about using -e instead of using multiple sed commands if we have a change to update further.
https://unix.stackexchange.com/questions/33157/what-is-the-purpose-of-e-in-sed-command

Copy link
Contributor Author

@jiangxb1987 jiangxb1987 Oct 30, 2019

Choose a reason for hiding this comment

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

I tested locally and it generates:

-rw-r--r--  1 ubuntu ubuntu 218128015 Oct 30 05:16 pyspark-3.0.0.dev0.tar.gz
-rw-r--r--  1 ubuntu ubuntu       833 Oct 30 05:16 pyspark-3.0.0.dev0.tar.gz.asc
-rw-r--r--  1 ubuntu ubuntu       225 Oct 30 05:16 pyspark-3.0.0.dev0.tar.gz.sha512

Copy link
Contributor Author

@jiangxb1987 jiangxb1987 Oct 30, 2019

Choose a reason for hiding this comment

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

@kiszk updated, thanks!

Copy link
Member

@dongjoon-hyun dongjoon-hyun Oct 30, 2019

Choose a reason for hiding this comment

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

So, are you going to rename them manually before uploading for Apache Spark VOTE, @jiangxb1987 ? Or, do you want to use that pyspark-3.0.0.dev0.tar.gz directly?

@jiangxb1987
Copy link
Contributor Author

jiangxb1987 commented Oct 30, 2019

Retest this please

@cloud-fan
Copy link
Contributor

cloud-fan commented Oct 30, 2019

retest this please

@dongjoon-hyun
Copy link
Member

dongjoon-hyun commented Oct 30, 2019

Maybe, I was confused the context. Are we going to publish this to PyPi?
If we are not going to publish it, I'm +1 for just renaming after generating with this PR, @jiangxb1987 .

@jiangxb1987
Copy link
Contributor Author

jiangxb1987 commented Oct 30, 2019

@dongjoon-hyun As we previously discussed (#26243 (comment)), we can release pyspark's tarball, but not push to pip. As for the name, I think pyspark-3.0.0.dev0.tar.gz is good enough. Also cc @srowen @HyukjinKwon

@srowen
Copy link
Member

srowen commented Oct 30, 2019

Dumb question: is 3.0.0.dev0 the more 'conventional' version number for a preview-like release in Python? then yeah it's fine. It's just the naming of the tarball, really.

@SparkQA
Copy link

SparkQA commented Oct 30, 2019

Test build #112948 has finished for PR 26306 at commit 68c5181.

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

@jiangxb1987
Copy link
Contributor Author

jiangxb1987 commented Oct 30, 2019

is 3.0.0.dev0 the more 'conventional' version number for a preview-like release in Python?

I'm not able to directly give an answer, but according to the comment in release-build.sh, 3.0.0.dev0 is closer to PEP440:
https://github.com/apache/spark/blob/2baf7a1d8ff0d7018d6b70876c1e65b549ae30b0/dev/create-release/release-build.sh#L220~L221

@SparkQA
Copy link

SparkQA commented Oct 30, 2019

Test build #112956 has finished for PR 26306 at commit 6f2cf6c.

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

dongjoon-hyun
dongjoon-hyun approved these changes Oct 30, 2019
View reviewed changes
Copy link
Member

@dongjoon-hyun dongjoon-hyun left a comment

Choose a reason for hiding this comment

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

Got it~ Thanks, @jiangxb1987 .

@dongjoon-hyun
Copy link
Member

dongjoon-hyun commented Oct 30, 2019

Merged to master!

@jiangxb1987
Copy link
Contributor Author

jiangxb1987 commented Oct 30, 2019

Thanks @dongjoon-hyun @srowen @HyukjinKwon @kiszk @cloud-fan !

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

Reviewers

@kiszk kiszk kiszk left review comments

@HyukjinKwon HyukjinKwon HyukjinKwon approved these changes

@dongjoon-hyun dongjoon-hyun dongjoon-hyun approved these changes

Assignees

No one assigned

Labels

BUILD

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@jiangxb1987 @SparkQA @cloud-fan @dongjoon-hyun @srowen @kiszk @HyukjinKwon