ARROW-16602: [Dev] Use GitHub API to merge pull request #13184

kou · 2022-05-18T03:39:26Z

We use local "git merge" to merge a pull request in
dev/merge_arrow_pr.py.

If we use "git merge" to merge a pull request, GitHub's Web UI shows
"Closed" mark not "Merged" mark in a pull request page. This sometimes
confuses new contributors. "Why was my pull request closed without
merging?" See
#12004 (comment) for
example.

If we use GitHub API
https://docs.github.com/en/rest/pulls/pulls#merge-a-pull-request to
merge a pull request, GitHub's Web UI shows "Merged" mark not "Closed"
mark. See #13180 for example. I
used GitHub API to merge the pull request.

And we don't need to create a local branch on local repository to
merge a pull request. But we must specify ARROW_GITHUB_API_TOKEN to
run dev/merge_arrow_pr.py.

We use local "git merge" to merge a pull request in dev/merge_arrow_pr.py. If we use "git merge" to merge a pull request, GitHub's Web UI shows "Closed" mark not "Merged" mark in a pull request page. This sometimes confuses new contributors. "Why was my pull request closed without merging?" See apache#12004 (comment) for example. If we use GitHub API https://docs.github.com/en/rest/pulls/pulls#merge-a-pull-request to merge a pull request, GitHub's Web UI shows "Merged" mark not "Closed" mark. See apache#13180 for example. I used GitHub API to merge the pull request. And we don't need to create a local branch on local repository to merge a pull request. But we must specify ARROW_GITHUB_API_TOKEN to run dev/merge_arrow_pr.py.

github-actions · 2022-05-18T03:40:20Z

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

github-actions · 2022-05-18T03:40:22Z

⚠️ Ticket has not been started in JIRA, please click 'Start Progress'.

kou · 2022-05-18T03:40:24Z

I used this to merge #13180 but I changed a bit after the merge. So we need to merge another pull request with this before we merge this.

kou · 2022-05-19T03:18:50Z

Tested with #13192 that includes Co-authored-by.

I don't know why but this approach can merge some pull requests such as #13180 and #13192 but can't merge some pull requests such as #12904 and #13182 ...

kou · 2022-05-19T03:20:23Z

I could merge #12053 with this...

raulcd · 2022-05-20T12:56:18Z

Hi Kou, should we update this documentation (https://github.com/apache/arrow/blob/master/dev/README.md?plain=1#L56-L58) to specify that now ARROW_GITHUB_API_TOKEN is mandatory then?

kou · 2022-05-21T21:29:54Z

Good catch!
I've updated the documentation and added support for ~/.config/arrow/merge.py and prompt for no ARROW_GITHUB_API_TOKEN case.

cyb70289

+1

When creating personal github access token, there are many choices can be possibily enabled in the "Select scopes" UI, do you know the necessary ones for merging PR?
https://github.com/settings/tokens/new

kou · 2022-05-25T05:33:55Z

We need only the public_repo scope.

jorisvandenbossche

Didn't look at the code in detail, but generally looks good! (and you say it worked on a few test PRs, so that sounds good enough to give this a try)

kou · 2022-05-25T21:15:49Z

Umm... I could merge #13228 but couldn't merge #13113...

$ dev/merge_arrow_pr.py 13113
ARROW_HOME = /home/kou/work/cpp/arrow.kou/dev
PROJECT_NAME = arrow

=== Pull Request #13113 ===
title	ARROW-15893: [CI][Python] Add python minimal builds to nightly builds
source	raulcd/ARROW-15893-2
target	master
url	https://api.github.com/repos/apache/arrow/pulls/13113
=== JIRA ARROW-15893 ===
Summary		[Python][CI] Exercise Python minimal build examples
Assignee	Raúl Cumplido
Components	Continuous Integration, Python
Status		In Progress
URL		https://issues.apache.org/jira/browse/ARROW-15893

Proceed with merging pull request #13113? (y/n): y
Author 1: Raúl Cumplido <raulcumplido@gmail.com>
Traceback (most recent call last):
  File "/home/kou/work/cpp/arrow.kou/dev/merge_arrow_pr.py", line 594, in <module>
    cli()
  File "/home/kou/work/cpp/arrow.kou/dev/merge_arrow_pr.py", line 575, in cli
    pr.merge()
  File "/home/kou/work/cpp/arrow.kou/dev/merge_arrow_pr.py", line 437, in merge
    self.cmd.fail(f'Failed to merge pull request: {message}')
  File "/home/kou/work/cpp/arrow.kou/dev/merge_arrow_pr.py", line 289, in fail
    raise Exception(msg)
Exception: Failed to merge pull request: Not Found

kou · 2022-05-26T07:50:16Z

I could merge #13237.

raulcd · 2022-05-26T12:37:38Z

I also got a 404 NOT FOUND I would have expected to get a 403 FORBIDDEN based on the github documentation (as I don't have permissions) https://docs.github.com/en/rest/pulls/pulls#merge-a-pull-request:

(Pdb) n
> /home/raulcd/open_source/arrow/dev/merge_arrow_pr.py(275)merge_pr()
-> response = requests.put(url, headers=self.headers, json=payload)
(Pdb) payload
{'commit_title': 'ARROW-15893: [CI][Python] Add python minimal builds to nightly builds (#13113)', 'commit_message': 'This PR adds new jobs to the nightly tests in order to exercise the existing Python minimal build examples.\n\nAuthored-by: Raúl Cumplido <raulcumplido@gmail.com>\nSigned-off-by: Raúl Cumplido <raulcumplido@gmail.com>', 'merge_method': 'squash'}
(Pdb) url
'https://api.github.com/repos/apache/arrow/pulls/13113/merge'

kou · 2022-05-26T20:08:02Z

Thanks for confirming it. It may be a GitHub API bug...

I could merge #13239.

cyb70289 · 2022-05-27T03:27:28Z

Looks it's related to access token scopes.
I was not able to merge #13191 with the new script. But it works after enabling all the scopes for the access token.
Maybe we can try the scopes incrementally to find the mininal requirement.

cyb70289 · 2022-05-27T03:29:26Z

FYI, I tried enable all repo scopes, it doesn't work.

cyb70289 · 2022-05-27T03:41:14Z

Looks committer becomes Github if we merge patches with this new script.

$ git shortlog -csn HEAD^..HEAD
     1  GitHub

We use the command to collect committers in release notes, maybe it's an issue need to address.
There are 5 GitHub in 8.0 releae notes https://arrow.apache.org/release/8.0.0.html.

kou · 2022-05-27T05:46:13Z

Looks it's related to access token scopes. I was not able to merge #13191 with the new script. But it works after enabling all the scopes for the access token. Maybe we can try the scopes incrementally to find the mininal requirement.

Wow! Thanks for the information! I could merge #13113 with workflow scope.

kou · 2022-05-27T06:00:33Z

Looks committer becomes Github if we merge patches with this new script.
$ git shortlog -csn HEAD^..HEAD
     1  GitHub

Good catch!
It seems that we should use --group=trailer:signed-off-by instead of -c because we use commiter information for Signed-off-by in the merge script.

I've updated the command line that is used for release note.

$ git shortlog -csn 'HEAD^..HEAD'
     1	GitHub
$ git shortlog -sn --group=trailer:signed-off-by 'HEAD^..HEAD'
     1	Sutou Kouhei

We use the command to collect committers in release notes, maybe it's an issue need to address. There are 5 GitHub in 8.0 releae notes https://arrow.apache.org/release/8.0.0.html.

It seems that the GitHub was caused by pull requests merged with the "merge" button.

cyb70289 · 2022-05-31T02:35:21Z

I tried several PRs, looks it's ready to merge?

jorisvandenbossche · 2022-05-31T09:28:18Z

I could merge #13269 (a minor PR)

kou · 2022-05-31T21:00:18Z

We didn't get objections on dev@arrow.apache.org: https://lists.apache.org/thread/6ok9bj3fs0mgz846963zg3542bkd4m6x

Let's merge this!

wesm · 2022-05-31T22:14:18Z

awesome!

kou added 4 commits May 18, 2022 13:50

Fix lint problems

6b33f4b

Update expected

b1e60ba

Fix variables

5a3a87f

Fix tests

d53f305

kou mentioned this pull request May 20, 2022

ARROW-16614: [C++] Use lz4::lz4 for lz4's CMake target name #13193

Merged

Update document and add support for conf and prompt

4d05238

Pass cmd

d7984d8

cyb70289 approved these changes May 25, 2022

View reviewed changes

jorisvandenbossche approved these changes May 25, 2022

View reviewed changes

Show URL on error

286d7bb

Need workflow scope

397494f

Use --group=trailer:signed-off-by instead of -c

5ca3b61

kou merged commit 01e4ad0 into apache:master May 31, 2022

kou deleted the merge-by-github-api branch May 31, 2022 21:01

asfimport mentioned this pull request Jul 8, 2022

[Dev] Use GitHub API to merge pull request #31956

Closed

kou mentioned this pull request Oct 16, 2023

[Dev][Docs] dev/merge_arrow_pr.py doesn't require "arrow" remote #38300

Closed

ARROW-16602: [Dev] Use GitHub API to merge pull request #13184

ARROW-16602: [Dev] Use GitHub API to merge pull request #13184

Uh oh!

Conversation

kou commented May 18, 2022

Uh oh!

github-actions bot commented May 18, 2022

Uh oh!

github-actions bot commented May 18, 2022

Uh oh!

kou commented May 18, 2022

Uh oh!

kou commented May 19, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kou commented May 19, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

raulcd commented May 20, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kou commented May 21, 2022

Uh oh!

cyb70289 left a comment

Choose a reason for hiding this comment

Uh oh!

kou commented May 25, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jorisvandenbossche left a comment

Choose a reason for hiding this comment

Uh oh!

kou commented May 25, 2022

Uh oh!

kou commented May 26, 2022

Uh oh!

raulcd commented May 26, 2022

Uh oh!

kou commented May 26, 2022

Uh oh!

cyb70289 commented May 27, 2022

Uh oh!

cyb70289 commented May 27, 2022

Uh oh!

cyb70289 commented May 27, 2022

Uh oh!

kou commented May 27, 2022

Uh oh!

kou commented May 27, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cyb70289 commented May 31, 2022

Uh oh!

jorisvandenbossche commented May 31, 2022

Uh oh!

kou commented May 31, 2022

Uh oh!

wesm commented May 31, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

kou commented May 19, 2022 •

edited

Loading

kou commented May 19, 2022 •

edited

Loading

raulcd commented May 20, 2022 •

edited

Loading

kou commented May 25, 2022 •

edited

Loading

kou commented May 27, 2022 •

edited

Loading