Skip to content
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

ARROW-14749: [Python][Release] Set release verification script to use target source instead of current source directory #11735

Closed
wants to merge 2 commits into from
Closed

ARROW-14749: [Python][Release] Set release verification script to use target source instead of current source directory #11735

wants to merge 2 commits into from

Conversation

bkmgit
Copy link
Contributor

@bkmgit bkmgit commented Nov 18, 2021

No description provided.

@github-actions
Copy link

Thanks for opening a pull request!

If this is not a minor PR. Could you open an issue for this pull request on JIRA? https://issues.apache.org/jira/browse/ARROW

Opening JIRAs ahead of time contributes to the Openness of the Apache Arrow project.

Then could you also rename pull request title in the following format?

ARROW-${JIRA_ID}: [${COMPONENT}] ${SUMMARY}

or

MINOR: [${COMPONENT}] ${SUMMARY}

See also:

@bkmgit bkmgit changed the title Arrow-14749: [Python][Release] Set release verification script to use target source instead of current source directory ARROW-14749: [Python][Release] Set release verification script to use target source instead of current source directory Nov 18, 2021
@github-actions
Copy link

https://issues.apache.org/jira/browse/ARROW-14749

@github-actions
Copy link

⚠️ Ticket has no components in JIRA, make sure you assign one.

@bkmgit bkmgit mentioned this pull request Nov 18, 2021
Closed
@bkmgit
Copy link
Contributor Author

bkmgit commented Nov 18, 2021

@kou Ready for review

kou
kou reviewed Nov 28, 2021
View reviewed changes
dev/release/verify-release-candidate.sh Outdated
# Clone testing repositories if not cloned already
if [ ! -d "arrow-testing" ]; then
git clone https://github.com/apache/arrow-testing.git
get_or_decompress_source() {
Copy link
Member

Choose a reason for hiding this comment

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

How about ensure_source?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe get_source_distribution as there is test_source_distribution?

Copy link
Member

Choose a reason for hiding this comment

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

This may not get source distribution. (This may just use a local archive.) So I think that get isn't suitable for this case.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ensure_source suggests verification. Maybe ensure_source_availability or something similar.

Copy link
Member

Choose a reason for hiding this comment

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

ensure_source_distribution? ensure_source_directory?

dev/release/verify-release-candidate.sh Outdated Show resolved Hide resolved
dev/release/verify-release-candidate.sh Outdated Show resolved Hide resolved
dev/release/verify-release-candidate.sh Outdated Show resolved Hide resolved
dev/release/verify-release-candidate.sh Outdated Show resolved Hide resolved
@bkmgit
Copy link
Contributor Author

bkmgit commented Nov 30, 2021

@kou Ready for review

kou
kou reviewed Dec 1, 2021
View reviewed changes
Copy link
Member

@kou kou left a comment •
edited
Loading

Choose a reason for hiding this comment

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

I tried this but it failed:

$ LANG=C dev/release/verify-release-candidate.sh wheels 6.0.1 1
...
+ fetch_archive
+ local dist_name=
+ download_rc_file .tar.gz
+ download_dist_file apache-arrow-6.0.1-rc1/.tar.gz
+ curl --silent --show-error --fail --location --remote-name https://dist.apache.org/repos/dist/dev/arrow/apache-arrow-6.0.1-rc1/.tar.gz
curl: (22) The requested URL returned error: 404 Not Found
...

It seems that you removed dist_name="apache-arrow-${VERSION}".

Did you test this on local before you request a review?

dev/release/verify-release-candidate.sh Outdated Show resolved Hide resolved
@bkmgit
Copy link
Contributor Author

bkmgit commented Dec 1, 2021

@kou Sorry, it works now. seemed to have missed a line setting value.

@kou
Copy link
Member

kou commented Dec 2, 2021

Sorry. I merged a conflicted change.
Could you rebase on master?

@bkmgit
Copy link
Contributor Author

bkmgit commented Dec 3, 2021

@kou Can rebase again if needed.

@kou
Copy link
Member

kou commented Dec 3, 2021

It seems that there are needless changes in this pull request.

Could you use git rebase apache/master instead of git merge apache/master?

@bkmgit
Copy link
Contributor Author

bkmgit commented Dec 3, 2021

Ok. Will do that.

@bkmgit bkmgit force-pushed the ARROW-14749 branch 2 times, most recently from ef8baf7 to 501a6df Compare December 5, 2021 19:50
@kou
Copy link
Member

kou commented Dec 6, 2021

It seems that the changes by #11696 are removed in this pull request.
Do you want me to rebase this branch?

@bkmgit
Copy link
Contributor Author

bkmgit commented Dec 6, 2021

Let me try once more.

@bkmgit
author bkmgit <benson_muite@emailplus.org>
d408e06 
Co-authored-by: Sutou Kouhei <kou@cozmixng.org>
committer bkmgit <benson_muite@emailplus.org>

update to download released sources
@bkmgit
Copy link
Contributor Author

bkmgit commented Dec 6, 2021

Thanks for finding the missing section.

@bkmgit
Copy link
Contributor Author

bkmgit commented Dec 6, 2021

@kou Hopefully ok now. Tested jars, source, binary, wheels, binary + yum

@bkmgit
Copy link
Contributor Author

bkmgit commented Dec 6, 2021

git diff master shows only changes in the verification script

kou
kou approved these changes Dec 8, 2021
View reviewed changes
Copy link
Member

@kou kou left a comment

Choose a reason for hiding this comment

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

+1

Thanks!

@kou kou closed this in 392a25f Dec 8, 2021
@ursabot
Copy link

ursabot commented Dec 8, 2021 •
edited
Loading

Benchmark runs are scheduled for baseline = 3f179ca and contender = 392a25f. 392a25f is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
Conbench compare runs links:
[Finished ⬇️0.0% ⬆️0.0%] ec2-t3-xlarge-us-east-2
[Failed ⬇️0.0% ⬆️0.0%] ursa-i9-9960x
[Finished ⬇️0.04% ⬆️0.0%] ursa-thinkcentre-m75q
Supported benchmarks:
ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python. Runs only benchmarks with cloud = True
ursa-i9-9960x: Supported benchmark langs: Python, R, JavaScript
ursa-thinkcentre-m75q: Supported benchmark langs: C++, Java

@bkmgit bkmgit deleted the ARROW-14749 branch December 8, 2021 14:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kou kou kou approved these changes

Assignees
No one assigned
Labels
Component: C++ - Gandiva Component: C++ Component: GLib Component: Parquet Component: Python Component: R
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@bkmgit @kou @ursabot