GH-14720: [Dev] Update merge_arrow_pr script to accept GitHub issues #14750

amol- · 2022-11-28T11:51:35Z

Closes: MIGRATION: [Developer] The merge_arrow_pr script should be able to handle issues on GitHub and JIRA #14720

jorisvandenbossche · 2022-11-29T11:40:35Z

FYI, I tested the changes to the merge script by merging #14762. Merging itself went fine. When it asked "Would you like to update the associated JIRA? (y/n)", I said no (because there is no JIRA), but maybe I should have said yes to see to how the github issue got updated? (closed, assigned, ..)? In that case, we should probably update the wording of the question.

amol- · 2022-11-29T12:01:32Z

FYI, I tested the changes to the merge script by merging #14762. Merging itself went fine. When it asked "Would you like to update the associated JIRA? (y/n)", I said no (because there is no JIRA), but maybe I should have said yes to see to how the github issue got updated? (closed, assigned, ..)? In that case, we should probably update the wording of the question.

Good catch, you were supposed to answer "yes" but as you noticed the wording is wrong. I'll update it to mention GitHub too.

jorisvandenbossche · 2022-11-29T12:22:13Z

Ah, I see you also updated it to be able to test on another repo .. In any case, I already just tested on another PR, but updating the issue failed: #14745

Traceback (most recent call last):
  File "/home/joris/scipy/repos/arrow/dev/merge_arrow_pr.py", line 698, in <module>
    cli()
  File "/home/joris/scipy/repos/arrow/dev/merge_arrow_pr.py", line 693, in cli
    pr.issue.resolve(fix_version, issue_comment)
  File "/home/joris/scipy/repos/arrow/dev/merge_arrow_pr.py", line 239, in resolve
    self.github_api.close_issue(self.github_id, comment)
  File "/home/joris/scipy/repos/arrow/dev/merge_arrow_pr.py", line 362, in close_issue
    raise ValueError(f"Failed request: {comment_url} -> {r.json()}")
ValueError: Failed request: https://api.github.com/repos/apache/arrow/issues/14745/comments -> {'url': 'https://api.github.com/repos/apache/arrow/issues/comments/1330536087', ...

Now, the comment seems to actually be posted (#14745 (comment)), so I don't know exactly how to interpret that failure.

The comment body should also be updated for GitHub (will comment inline)

jorisvandenbossche · 2022-11-29T12:29:28Z

dev/merge_arrow_pr.py

-    cmd.continue_maybe("Would you like to update the associated JIRA?")
-    jira_comment = (
+    cmd.continue_maybe("Would you like to update the associated issue?")
+    issue_comment = (
        "Issue resolved by pull request %s\n[%s/%s]"


For posting this on GitHub issues, the formatting doesn't need to include the [ ] (I would personally also remove the newline and double PR number/url for GitHub (since the url gets rendered as a number anyway)

So something like

"Issue resolved by pull request GH-%s" with just the pr_num (or using the full url, and then the message can maybe be the same for both JIRA and GitHub)

Yes, I'm aware of that. I tweaked it a little bit so that it works reasonably on both GitHub and Jira.
See https://github.com/amol-/arrow/issues/3

We can refine it further in the future, at the moment my goal was to make it good enough for both Jira and Github.

PS: I still need to test it against a Jira issue as I did some changes on the Jira side too.

jorisvandenbossche · 2022-11-29T13:16:38Z

You still have some linting issues (line length)

dev/merge_arrow_pr.py

github-actions · 2022-11-30T16:59:33Z

Closes: MIGRATION: [Developer] The merge_arrow_pr script should be able to handle issues on GitHub and JIRA #14720

kou

+1

kou · 2022-11-30T20:38:57Z

dev/merge_arrow_pr.py

+        try:
+            self.issue = self.github_api.get_issue_data(github_id)
+        except Exception as e:
+            self.cmd.fail("Github could not find %s\n%s" % (github_id, e))


Suggested change

self.cmd.fail("Github could not find %s\n%s" % (github_id, e))

self.cmd.fail("GitHub could not find %s\n%s" % (github_id, e))

kou · 2022-11-30T20:45:01Z

dev/merge_arrow_pr.py

+                  (self.github_id, fix_version))
+        else:
+            self.github_api.assign_milestone(self.github_id, fix_version)
+            self.github_api.close_issue(self.github_id, comment)


We may not need this because we can use GitHub's auto close feature by close GH-XXX commit message.

I've decided to validate whether the message Closes: #XXX is in the body as expected from the dev_pr workflow:
https://github.com/apache/arrow/blob/master/.github/workflows/dev_pr/link.js#L85
otherwise we close the issue. That will close issues that might have been created after an initial MINOR PR if we forgot to add the Closes or if the description was manually modified, etcetera.

kou · 2022-11-30T20:53:48Z

dev/merge_arrow_pr.py

+    # We can merge both ARROW and PARQUET patches
+    GITHUB_PR_TITLE_REGEXEN = re.compile(r'^GH-([0-9]+)\b.*$')
+    JIRA_SUPPORTED_PROJECTS = ['ARROW', 'PARQUET']


Suggested change

# We can merge both ARROW and PARQUET patches

GITHUB_PR_TITLE_REGEXEN = re.compile(r'^GH-([0-9]+)\b.*$')

JIRA_SUPPORTED_PROJECTS = ['ARROW', 'PARQUET']

GITHUB_PR_TITLE_REGEXEN = re.compile(r'^GH-([0-9]+)\b.*$')

# We can merge both ARROW and PARQUET patches

JIRA_SUPPORTED_PROJECTS = ['ARROW', 'PARQUET']

kou · 2022-11-30T20:55:52Z

dev/merge_arrow_pr.py

@@ -322,6 +440,13 @@ def continue_maybe(self, prompt):


 class PullRequest(object):
+    # We can merge both ARROW and PARQUET patches
+    GITHUB_PR_TITLE_REGEXEN = re.compile(r'^GH-([0-9]+)\b.*$')


I think that REGEXEN meant "N regular expressions". (I think that "EXE" is a typo of "EXP" or EX.)

This is just one regular expression. How about GITHUB_PR_TITLE_REGEX or GITHUB_PR_TITLE_PATTERN?

kou · 2022-11-30T20:58:04Z

dev/merge_arrow_pr.py

+        options = ' or '.join('{0}-XXX'.format(project)
+                              for project in self.JIRA_SUPPORTED_PROJECTS)


Can we add GH-XXX too?

kou · 2022-11-30T20:58:23Z

dev/merge_arrow_pr.py

-        return JiraIssue(self.con, jira_id, project, self.cmd)
+        options = ' or '.join('{0}-XXX'.format(project)
+                              for project in self.JIRA_SUPPORTED_PROJECTS)
+        self.cmd.fail("PR title should be prefixed by a jira id "


Suggested change

self.cmd.fail("PR title should be prefixed by a jira id "

self.cmd.fail("PR title should be prefixed by a GitHub ID or a Jira ID "

kou · 2022-11-30T20:59:34Z

dev/merge_arrow_pr.py

-        return [x for x in all_versions if x.name == version_str][0].raw
-
-    return [get_version_json(v) for v in issue_fix_versions]
+    issue_fix_version = cmd.prompt("Enter comma-separated "


Do we still support comma-separated fix version?

we don't, I have changed the comment to: Enter fix version [%s]: , I am not sure how useful is that but I think we can add this functionality as an improvement later on if required.

kou · 2022-11-30T21:03:23Z

dev/test_merge_arrow_pr.py

-    ordered_versions = merge_arrow_pr.JiraIssue.sort_versions(versions)
-    assert ordered_versions[0].name == "10.0.0"
+    fix_version = merge_arrow_pr.get_candidate_fix_version(versions)
+    assert fix_version == "9.0.0"


Do we want to add tests for GitHubIssue like existing JiraIssue tests?

Sounds like a good idea. I've added a ticket to track this separately if that's ok: #14802

kou · 2022-11-30T21:06:33Z

dev/merge_arrow_pr.py

@@ -32,8 +32,8 @@
 # Configuration environment variables:
 #   - APACHE_JIRA_TOKEN: your Apache JIRA Personal Access Token
 #   - ARROW_GITHUB_API_TOKEN: a GitHub API token to use for API requests
-#   - PR_REMOTE_NAME: the name of the remote to the Apache git repo (set to
-#                     'apache' by default)
+#   - ORG_NAME: the name of the remote to the Apache git repo


How about ARROW_GITHUB_ORG or ARROW_GITHUB_OWNER because ARROW_GITHUB_API_TOKEN has ARROW_GITHUB_ prefix?

the remote to the Apache git repo

Could you also update this description?

kou · 2022-11-30T21:07:56Z

Is component name missing in this title?
Should we use GH-14720: [Dev] Update ...?

kou · 2022-11-30T21:09:49Z

NOTE: I didn't try this. I just reviewed source code. Please let me know if I should try this.

jorisvandenbossche · 2022-12-01T07:13:07Z

I already tried it with both PRs that close JIRA or GitHub issues, and that seems to be working fine.

…sions anymore

raulcd · 2022-12-01T12:38:43Z

I've tested on my fork and all looks good. PR was merged successfully and issue was closed as expected:

ARROW_HOME = /home/raulcd/code/arrow/dev
ORG_NAME = raulcd
PROJECT_NAME = arrow
Which pull request would you like to merge? (e.g. 34): 67

=== Pull Request #67 ===
title	GH-48: [Dev] Initial exploration on Python-dev build
source	raulcd/explore-python-dev-build
target	master
url	https://api.github.com/repos/raulcd/arrow/pulls/67
=== GITHUB 48 ===
Summary		[Dev] Testing automatically assign issue
Assignee	raulcd
Components	Python
Status		open
URL		https://github.com/raulcd/arrow/issues/48

Proceed with merging pull request #67? (y/n): y
Author 1: Raúl Cumplido <raulcumplido@gmail.com>
Pull request #67 merged!
Merge hash: 1820dddfe30bc26e0a58a2c227d46c4d03913533

Would you like to update the associated issue? (y/n): y
Enter fix version [11.0.0]: 
Successfully resolved 48!
=== GITHUB 48 ===
Summary		[Dev] Testing automatically assign issue
Assignee	raulcd
Components	Python
Status		closed
URL		https://github.com/raulcd/arrow/issues/48```

jorisvandenbossche

Latest changes look good to me!

jorisvandenbossche · 2022-12-01T12:57:06Z

@kou I am going to merge this, since we have several PRs open that need this to be able to be merged. I think the remaining items can be done as follow-ups, as Raúl indicated

jorisvandenbossche · 2022-12-01T12:59:08Z

And I merged this PR with the PR itself, and that seems to have gone fine! (the issue was closed because of the link in the body)

ursabot · 2022-12-01T16:02:16Z

Benchmark runs are scheduled for baseline = 958fbfa and contender = 1ba97a6. 1ba97a6 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.2% ⬆️0.0%] test-mac-arm
[Finished ⬇️0.54% ⬆️0.0%] ursa-i9-9960x
[Finished ⬇️0.24% ⬆️0.35%] ursa-thinkcentre-m75q
Buildkite builds:
[Finished] 1ba97a62 ec2-t3-xlarge-us-east-2
[Finished] 1ba97a62 test-mac-arm
[Finished] 1ba97a62 ursa-i9-9960x
[Finished] 1ba97a62 ursa-thinkcentre-m75q
[Finished] 958fbfa5 ec2-t3-xlarge-us-east-2
[Failed] 958fbfa5 test-mac-arm
[Finished] 958fbfa5 ursa-i9-9960x
[Finished] 958fbfa5 ursa-thinkcentre-m75q
Supported benchmarks:
ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python, R. Runs only benchmarks with cloud = True
test-mac-arm: Supported benchmark langs: C++, Python, R
ursa-i9-9960x: Supported benchmark langs: Python, R, JavaScript
ursa-thinkcentre-m75q: Supported benchmark langs: C++, Java

amol- added 2 commits November 28, 2022 12:50

Initial effort to update merge_pr script for Github

d0590b0

Nearly working merge_script

80e1393

amol- changed the title ~~GH-14720: Update merge_arrow_pr script to accept GitHub issues~~ GH-14720 Update merge_arrow_pr script to accept GitHub issues Nov 28, 2022

amol- changed the title ~~GH-14720 Update merge_arrow_pr script to accept GitHub issues~~ GH-14720: Update merge_arrow_pr script to accept GitHub issues Nov 28, 2022

amol- added 2 commits November 28, 2022 15:52

Nearly complete

9ebb8f1

Remove hardcoded debug

9e4769c

amol- mentioned this pull request Nov 28, 2022

ARROW-18380: [Dev] Update dev_pr GitHub workflows to accept both GitHub issues and JIRA #14731

Merged

amol- added 3 commits November 29, 2022 13:06

Update wording

b9b3021

Make target organization configurable, so that we can test on forks

3ad943f

Tweaks

3484b77

jorisvandenbossche reviewed Nov 29, 2022

View reviewed changes

Remaining tweaks

53727e8

amol- marked this pull request as ready for review November 29, 2022 12:56

lint

99b0d2e

amol- added the Component: Developer Tools label Nov 29, 2022

amol- assigned amol- and unassigned amol- Nov 29, 2022

amol- removed the Component: Developer Tools label Nov 29, 2022

jorisvandenbossche reviewed Nov 29, 2022

View reviewed changes

dev/merge_arrow_pr.py Show resolved Hide resolved

amol- added 3 commits November 29, 2022 14:20

lint

5b0521a

fix tests

cbb75df

lint

ee561a3

zeroshade mentioned this pull request Nov 29, 2022

GH-14775: [Go] Fix UnionBuilder.Len implementations #14776

Merged

jorisvandenbossche requested a review from kou November 30, 2022 12:17

amol- added the Component: Developer Tools label Nov 30, 2022

apache deleted a comment from github-actions bot Nov 30, 2022

lidavidm mentioned this pull request Nov 30, 2022

GH-14786: [Java][Doc] Replace in-folder documentation #14789

Merged

kou approved these changes Nov 30, 2022

View reviewed changes

jorisvandenbossche changed the title ~~GH-14720: Update merge_arrow_pr script to accept GitHub issues~~ GH-14720: [Dev] Update merge_arrow_pr script to accept GitHub issues Dec 1, 2022

raulcd added 3 commits December 1, 2022 13:07

Review comments for merge_arrow_pr

1da35b9

Fix lint and change comment as we don't allow comma separated fix ver…

93575e0

…sions anymore

Minor removal of (s) for version

6db7f3f

jorisvandenbossche approved these changes Dec 1, 2022

View reviewed changes

jorisvandenbossche merged commit 1ba97a6 into apache:master Dec 1, 2022

amol- deleted the GH-14720 branch December 1, 2022 13:06

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

GH-14720: [Dev] Update merge_arrow_pr script to accept GitHub issues #14750

GH-14720: [Dev] Update merge_arrow_pr script to accept GitHub issues #14750

amol- commented Nov 28, 2022 •

edited

Loading

jorisvandenbossche commented Nov 29, 2022

amol- commented Nov 29, 2022

jorisvandenbossche commented Nov 29, 2022

jorisvandenbossche Nov 29, 2022

amol- Nov 29, 2022

jorisvandenbossche commented Nov 29, 2022

github-actions bot commented Nov 30, 2022

kou left a comment

kou Nov 30, 2022

kou Nov 30, 2022

raulcd Dec 1, 2022

kou Nov 30, 2022

kou Nov 30, 2022

kou Nov 30, 2022

kou Nov 30, 2022

kou Nov 30, 2022

raulcd Dec 1, 2022 •

edited

Loading

kou Nov 30, 2022

raulcd Dec 1, 2022

kou Nov 30, 2022

kou commented Nov 30, 2022

kou commented Nov 30, 2022

jorisvandenbossche commented Dec 1, 2022

raulcd commented Dec 1, 2022

jorisvandenbossche left a comment

jorisvandenbossche commented Dec 1, 2022

jorisvandenbossche commented Dec 1, 2022

ursabot commented Dec 1, 2022

	self.cmd.fail("Github could not find %s\n%s" % (github_id, e))
	self.cmd.fail("GitHub could not find %s\n%s" % (github_id, e))

		options = ' or '.join('{0}-XXX'.format(project)
		for project in self.JIRA_SUPPORTED_PROJECTS)

	self.cmd.fail("PR title should be prefixed by a jira id "
	self.cmd.fail("PR title should be prefixed by a GitHub ID or a Jira ID "

GH-14720: [Dev] Update merge_arrow_pr script to accept GitHub issues #14750

GH-14720: [Dev] Update merge_arrow_pr script to accept GitHub issues #14750

Conversation

amol- commented Nov 28, 2022 • edited Loading

jorisvandenbossche commented Nov 29, 2022

amol- commented Nov 29, 2022

jorisvandenbossche commented Nov 29, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jorisvandenbossche commented Nov 29, 2022

github-actions bot commented Nov 30, 2022

kou left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

raulcd Dec 1, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kou commented Nov 30, 2022

kou commented Nov 30, 2022

jorisvandenbossche commented Dec 1, 2022

raulcd commented Dec 1, 2022

jorisvandenbossche left a comment

Choose a reason for hiding this comment

jorisvandenbossche commented Dec 1, 2022

jorisvandenbossche commented Dec 1, 2022

ursabot commented Dec 1, 2022

amol- commented Nov 28, 2022 •

edited

Loading

raulcd Dec 1, 2022 •

edited

Loading