You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by GitBox <gi...@apache.org> on 2022/11/28 11:51:37 UTC

[GitHub] [arrow] amol- opened a new pull request, #14750: GH-14720: Update merge_arrow_pr script to accept GitHub issues

amol- opened a new pull request, #14750:
URL: https://github.com/apache/arrow/pull/14750

   As discussed in https://github.com/apache/arrow/issues/14720


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow] github-actions[bot] commented on pull request #14750: GH-14720: Update merge_arrow_pr script to accept GitHub issues

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #14750:
URL: https://github.com/apache/arrow/pull/14750#issuecomment-1332468274

   * Closes: #14720


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow] jorisvandenbossche commented on pull request #14750: GH-14720: [Dev] Update merge_arrow_pr script to accept GitHub issues

Posted by GitBox <gi...@apache.org>.
jorisvandenbossche commented on PR #14750:
URL: https://github.com/apache/arrow/pull/14750#issuecomment-1333725351

   @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


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow] kou commented on a diff in pull request #14750: GH-14720: Update merge_arrow_pr script to accept GitHub issues

Posted by GitBox <gi...@apache.org>.
kou commented on code in PR #14750:
URL: https://github.com/apache/arrow/pull/14750#discussion_r1036421443


##########
dev/merge_arrow_pr.py:
##########
@@ -187,71 +156,177 @@ def _filter_mainline_versions(self, versions):
 
         return [x for x in versions if mainline_regex.match(x.name)]
 
-    def resolve(self, fix_versions, comment):
+    def resolve(self, fix_version, comment):
         fields = self.issue.fields
         cur_status = fields.status.name
 
         if cur_status == "Resolved" or cur_status == "Closed":
             self.cmd.fail("JIRA issue %s already has status '%s'"
                           % (self.jira_id, cur_status))
 
-        if DEBUG:
-            print("JIRA issue %s untouched" % (self.jira_id))
-            return
-
         resolve = [x for x in self.jira_con.transitions(self.jira_id)
                    if x['name'] == "Resolve Issue"][0]
 
         # ARROW-6915: do not overwrite existing fix versions corresponding to
         # point releases
-        fix_versions = list(fix_versions)
+        fix_versions = [v.raw for v in self.jira_con.project_versions(
+            self.project) if v.name == fix_version]
         fix_version_names = set(x['name'] for x in fix_versions)
         for version in self.current_fix_versions:
             major, minor, patch = version.name.split('.')
             if patch != '0' and version.name not in fix_version_names:
                 fix_versions.append(version.raw)
 
-        self.jira_con.transition_issue(self.jira_id, resolve["id"],
-                                       comment=comment,
-                                       fixVersions=fix_versions)
-
-        print("Successfully resolved %s!" % (self.jira_id))
+        if DEBUG:
+            print("JIRA issue %s untouched -> %s" %
+                  (self.jira_id, [v["name"] for v in fix_versions]))
+        else:
+            self.jira_con.transition_issue(self.jira_id, resolve["id"],
+                                           comment=comment,
+                                           fixVersions=fix_versions)
+            print("Successfully resolved %s!" % (self.jira_id))
 
         self.issue = self.jira_con.issue(self.jira_id)
         self.show()
 
     def show(self):
         fields = self.issue.fields
-        print(format_jira_output(self.jira_id, fields.status.name,
-                                 fields.summary, fields.assignee,
-                                 fields.components))
+        print(format_issue_output("jira", self.jira_id, fields.status.name,
+                                  fields.summary, fields.assignee,
+                                  fields.components))
 
 
-def format_jira_output(jira_id, status, summary, assignee, components):
-    if assignee is None:
+class GitHubIssue(object):
+
+    def __init__(self, github_api, github_id, cmd):
+        self.github_api = github_api
+        self.github_id = github_id
+        self.cmd = cmd
+
+        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))

Review Comment:
   ```suggestion
               self.cmd.fail("GitHub could not find %s\n%s" % (github_id, e))
   ```



##########
dev/merge_arrow_pr.py:
##########
@@ -366,24 +491,25 @@ def maintenance_branches(self):
         return [x["name"] for x in self._github_api.get_branches()
                 if x["name"].startswith("maint-")]
 
-    def _get_jira(self):
+    def _get_issue(self):
         if self.title.startswith("MINOR:"):
             return None
 
-        jira_id = None
-        for project, regex in PR_TITLE_REGEXEN:
+        m = self.GITHUB_PR_TITLE_REGEXEN.search(self.title)
+        if m:
+            github_id = m.group(1)
+            return GitHubIssue(self._github_api, github_id, self.cmd)
+
+        for project, regex in self.JIRA_PR_TITLE_REGEXEN:
             m = regex.search(self.title)
             if m:
                 jira_id = m.group(1)
-                break
-
-        if jira_id is None:
-            options = ' or '.join('{0}-XXX'.format(project)
-                                  for project in SUPPORTED_PROJECTS)
-            self.cmd.fail("PR title should be prefixed by a jira id "
-                          "{0}, but found {1}".format(options, self.title))
+                return JiraIssue(self.con, jira_id, project, self.cmd)
 
-        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 "

Review Comment:
   ```suggestion
           self.cmd.fail("PR title should be prefixed by a GitHub ID or a Jira ID "
   ```



##########
dev/merge_arrow_pr.py:
##########
@@ -187,71 +156,177 @@ def _filter_mainline_versions(self, versions):
 
         return [x for x in versions if mainline_regex.match(x.name)]
 
-    def resolve(self, fix_versions, comment):
+    def resolve(self, fix_version, comment):
         fields = self.issue.fields
         cur_status = fields.status.name
 
         if cur_status == "Resolved" or cur_status == "Closed":
             self.cmd.fail("JIRA issue %s already has status '%s'"
                           % (self.jira_id, cur_status))
 
-        if DEBUG:
-            print("JIRA issue %s untouched" % (self.jira_id))
-            return
-
         resolve = [x for x in self.jira_con.transitions(self.jira_id)
                    if x['name'] == "Resolve Issue"][0]
 
         # ARROW-6915: do not overwrite existing fix versions corresponding to
         # point releases
-        fix_versions = list(fix_versions)
+        fix_versions = [v.raw for v in self.jira_con.project_versions(
+            self.project) if v.name == fix_version]
         fix_version_names = set(x['name'] for x in fix_versions)
         for version in self.current_fix_versions:
             major, minor, patch = version.name.split('.')
             if patch != '0' and version.name not in fix_version_names:
                 fix_versions.append(version.raw)
 
-        self.jira_con.transition_issue(self.jira_id, resolve["id"],
-                                       comment=comment,
-                                       fixVersions=fix_versions)
-
-        print("Successfully resolved %s!" % (self.jira_id))
+        if DEBUG:
+            print("JIRA issue %s untouched -> %s" %
+                  (self.jira_id, [v["name"] for v in fix_versions]))
+        else:
+            self.jira_con.transition_issue(self.jira_id, resolve["id"],
+                                           comment=comment,
+                                           fixVersions=fix_versions)
+            print("Successfully resolved %s!" % (self.jira_id))
 
         self.issue = self.jira_con.issue(self.jira_id)
         self.show()
 
     def show(self):
         fields = self.issue.fields
-        print(format_jira_output(self.jira_id, fields.status.name,
-                                 fields.summary, fields.assignee,
-                                 fields.components))
+        print(format_issue_output("jira", self.jira_id, fields.status.name,
+                                  fields.summary, fields.assignee,
+                                  fields.components))
 
 
-def format_jira_output(jira_id, status, summary, assignee, components):
-    if assignee is None:
+class GitHubIssue(object):
+
+    def __init__(self, github_api, github_id, cmd):
+        self.github_api = github_api
+        self.github_id = github_id
+        self.cmd = cmd
+
+        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))
+
+    def get_label(self, prefix):
+        prefix = f"{prefix}:"
+        return [
+            lbl["name"][len(prefix):].strip()
+            for lbl in self.issue["labels"] if lbl["name"].startswith(prefix)
+        ]
+
+    @property
+    def components(self):
+        return self.get_label("Component")
+
+    @property
+    def assignees(self):
+        return [a["login"] for a in self.issue["assignees"]]
+
+    @property
+    def current_fix_versions(self):
+        return self.issue.get("milestone", {}).get("title")
+
+    @property
+    def current_versions(self):
+        all_versions = self.github_api.get_milestones()
+
+        unreleased_versions = [x for x in all_versions if x["state"] == "open"]
+        unreleased_versions = [x["title"] for x in unreleased_versions]
+
+        return unreleased_versions
+
+    def resolve(self, fix_version, comment):
+        cur_status = self.issue["state"]
+
+        if cur_status == "closed":
+            self.cmd.fail("GitHub issue %s already has status '%s'"
+                          % (self.github_id, cur_status))
+
+        if DEBUG:
+            print("GitHub issue %s untouched -> %s" %
+                  (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)

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



##########
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

Review Comment:
   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?



##########
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.*$')

Review Comment:
   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`? 



##########
dev/test_merge_arrow_pr.py:
##########
@@ -332,9 +346,10 @@ def test_jira_output_no_components():
 
 def test_sorting_versions():
     versions_json = [
+        {'name': '11.0.0', 'released': False},
         {'name': '9.0.0', 'released': False},
         {'name': '10.0.0', 'released': False},
     ]
     versions = [FakeVersion(raw['name'], raw) for raw in versions_json]
-    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"

Review Comment:
   Do we want to add tests for `GitHubIssue` like existing `JiraIssue` tests?



##########
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.*$')
+    JIRA_SUPPORTED_PROJECTS = ['ARROW', 'PARQUET']

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



##########
dev/merge_arrow_pr.py:
##########
@@ -487,25 +613,19 @@ def get_primary_author(cmd, distinct_authors):
     return primary_author, distinct_other_authors
 
 
-def prompt_for_fix_version(cmd, jira_issue, maintenance_branches=()):
-    (all_versions,
-     default_fix_versions) = jira_issue.get_candidate_fix_versions(
+def prompt_for_fix_version(cmd, issue, maintenance_branches=()):
+    default_fix_version = get_candidate_fix_version(
+        issue.current_versions,
         maintenance_branches=maintenance_branches
     )
 
-    default_fix_versions = ",".join(default_fix_versions)
-
-    issue_fix_versions = cmd.prompt("Enter comma-separated "
-                                    "fix version(s) [%s]: "
-                                    % default_fix_versions)
-    if issue_fix_versions == "":
-        issue_fix_versions = default_fix_versions
-    issue_fix_versions = issue_fix_versions.replace(" ", "").split(",")
-
-    def get_version_json(version_str):
-        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 "

Review Comment:
   Do we still support comma-separated fix version?



##########
dev/merge_arrow_pr.py:
##########
@@ -366,24 +491,25 @@ def maintenance_branches(self):
         return [x["name"] for x in self._github_api.get_branches()
                 if x["name"].startswith("maint-")]
 
-    def _get_jira(self):
+    def _get_issue(self):
         if self.title.startswith("MINOR:"):
             return None
 
-        jira_id = None
-        for project, regex in PR_TITLE_REGEXEN:
+        m = self.GITHUB_PR_TITLE_REGEXEN.search(self.title)
+        if m:
+            github_id = m.group(1)
+            return GitHubIssue(self._github_api, github_id, self.cmd)
+
+        for project, regex in self.JIRA_PR_TITLE_REGEXEN:
             m = regex.search(self.title)
             if m:
                 jira_id = m.group(1)
-                break
-
-        if jira_id is None:
-            options = ' or '.join('{0}-XXX'.format(project)
-                                  for project in SUPPORTED_PROJECTS)
-            self.cmd.fail("PR title should be prefixed by a jira id "
-                          "{0}, but found {1}".format(options, self.title))
+                return JiraIssue(self.con, jira_id, project, self.cmd)
 
-        return JiraIssue(self.con, jira_id, project, self.cmd)
+        options = ' or '.join('{0}-XXX'.format(project)
+                              for project in self.JIRA_SUPPORTED_PROJECTS)

Review Comment:
   Can we add `GH-XXX` too?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow] jorisvandenbossche commented on a diff in pull request #14750: GH-14720: Update merge_arrow_pr script to accept GitHub issues

Posted by GitBox <gi...@apache.org>.
jorisvandenbossche commented on code in PR #14750:
URL: https://github.com/apache/arrow/pull/14750#discussion_r1034743953


##########
dev/merge_arrow_pr.py:
##########
@@ -281,6 +357,36 @@ def get_branches(self):
         return get_json("%s/branches" % (self.github_api),
                         headers=self.headers)
 
+    def close_issue(self, number, comment):
+        issue_url = f'{self.github_api}/issues/{number}'
+        comment_url = f'{self.github_api}/issues/{number}/comments'
+
+        r = requests.post(comment_url, json={
+                          "body": comment}, headers=self.headers)

Review Comment:
   ```suggestion
           r = requests.post(
               comment_url, json={"body": comment}, headers=self.headers)
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow] jorisvandenbossche commented on a diff in pull request #14750: GH-14720: Update merge_arrow_pr script to accept GitHub issues

Posted by GitBox <gi...@apache.org>.
jorisvandenbossche commented on code in PR #14750:
URL: https://github.com/apache/arrow/pull/14750#discussion_r1034692746


##########
dev/merge_arrow_pr.py:
##########
@@ -581,20 +677,20 @@ def cli():
 
     pr.merge()
 
-    if pr.jira_issue is None:
-        print("Minor PR.  No JIRA issue to update.\n")
+    if pr.issue is None:
+        print("Minor PR.  No issue to update.\n")
         return
 
-    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]"

Review Comment:
   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)



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow] jorisvandenbossche commented on pull request #14750: GH-14720: Update merge_arrow_pr script to accept GitHub issues

Posted by GitBox <gi...@apache.org>.
jorisvandenbossche commented on PR #14750:
URL: https://github.com/apache/arrow/pull/14750#issuecomment-1330546953

   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: https://github.com/apache/arrow/issues/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 (https://github.com/apache/arrow/issues/14745#issuecomment-1330536087), so I don't know exactly how to interpret that failure.
   
   The comment body should also be updated for GitHub (will comment inline)


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow] amol- commented on pull request #14750: GH-14720: Update merge_arrow_pr script to accept GitHub issues

Posted by GitBox <gi...@apache.org>.
amol- commented on PR #14750:
URL: https://github.com/apache/arrow/pull/14750#issuecomment-1330519686

   > 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.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow] kou commented on pull request #14750: GH-14720: Update merge_arrow_pr script to accept GitHub issues

Posted by GitBox <gi...@apache.org>.
kou commented on PR #14750:
URL: https://github.com/apache/arrow/pull/14750#issuecomment-1332736355

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


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow] raulcd commented on a diff in pull request #14750: GH-14720: [Dev] Update merge_arrow_pr script to accept GitHub issues

Posted by GitBox <gi...@apache.org>.
raulcd commented on code in PR #14750:
URL: https://github.com/apache/arrow/pull/14750#discussion_r1037050892


##########
dev/merge_arrow_pr.py:
##########
@@ -487,25 +613,19 @@ def get_primary_author(cmd, distinct_authors):
     return primary_author, distinct_other_authors
 
 
-def prompt_for_fix_version(cmd, jira_issue, maintenance_branches=()):
-    (all_versions,
-     default_fix_versions) = jira_issue.get_candidate_fix_versions(
+def prompt_for_fix_version(cmd, issue, maintenance_branches=()):
+    default_fix_version = get_candidate_fix_version(
+        issue.current_versions,
         maintenance_branches=maintenance_branches
     )
 
-    default_fix_versions = ",".join(default_fix_versions)
-
-    issue_fix_versions = cmd.prompt("Enter comma-separated "
-                                    "fix version(s) [%s]: "
-                                    % default_fix_versions)
-    if issue_fix_versions == "":
-        issue_fix_versions = default_fix_versions
-    issue_fix_versions = issue_fix_versions.replace(" ", "").split(",")
-
-    def get_version_json(version_str):
-        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 "

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



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow] jorisvandenbossche commented on pull request #14750: GH-14720: Update merge_arrow_pr script to accept GitHub issues

Posted by GitBox <gi...@apache.org>.
jorisvandenbossche commented on PR #14750:
URL: https://github.com/apache/arrow/pull/14750#issuecomment-1330639491

   You still have some linting issues (line length)


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow] raulcd commented on a diff in pull request #14750: GH-14720: [Dev] Update merge_arrow_pr script to accept GitHub issues

Posted by GitBox <gi...@apache.org>.
raulcd commented on code in PR #14750:
URL: https://github.com/apache/arrow/pull/14750#discussion_r1037042971


##########
dev/merge_arrow_pr.py:
##########
@@ -187,71 +156,177 @@ def _filter_mainline_versions(self, versions):
 
         return [x for x in versions if mainline_regex.match(x.name)]
 
-    def resolve(self, fix_versions, comment):
+    def resolve(self, fix_version, comment):
         fields = self.issue.fields
         cur_status = fields.status.name
 
         if cur_status == "Resolved" or cur_status == "Closed":
             self.cmd.fail("JIRA issue %s already has status '%s'"
                           % (self.jira_id, cur_status))
 
-        if DEBUG:
-            print("JIRA issue %s untouched" % (self.jira_id))
-            return
-
         resolve = [x for x in self.jira_con.transitions(self.jira_id)
                    if x['name'] == "Resolve Issue"][0]
 
         # ARROW-6915: do not overwrite existing fix versions corresponding to
         # point releases
-        fix_versions = list(fix_versions)
+        fix_versions = [v.raw for v in self.jira_con.project_versions(
+            self.project) if v.name == fix_version]
         fix_version_names = set(x['name'] for x in fix_versions)
         for version in self.current_fix_versions:
             major, minor, patch = version.name.split('.')
             if patch != '0' and version.name not in fix_version_names:
                 fix_versions.append(version.raw)
 
-        self.jira_con.transition_issue(self.jira_id, resolve["id"],
-                                       comment=comment,
-                                       fixVersions=fix_versions)
-
-        print("Successfully resolved %s!" % (self.jira_id))
+        if DEBUG:
+            print("JIRA issue %s untouched -> %s" %
+                  (self.jira_id, [v["name"] for v in fix_versions]))
+        else:
+            self.jira_con.transition_issue(self.jira_id, resolve["id"],
+                                           comment=comment,
+                                           fixVersions=fix_versions)
+            print("Successfully resolved %s!" % (self.jira_id))
 
         self.issue = self.jira_con.issue(self.jira_id)
         self.show()
 
     def show(self):
         fields = self.issue.fields
-        print(format_jira_output(self.jira_id, fields.status.name,
-                                 fields.summary, fields.assignee,
-                                 fields.components))
+        print(format_issue_output("jira", self.jira_id, fields.status.name,
+                                  fields.summary, fields.assignee,
+                                  fields.components))
 
 
-def format_jira_output(jira_id, status, summary, assignee, components):
-    if assignee is None:
+class GitHubIssue(object):
+
+    def __init__(self, github_api, github_id, cmd):
+        self.github_api = github_api
+        self.github_id = github_id
+        self.cmd = cmd
+
+        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))
+
+    def get_label(self, prefix):
+        prefix = f"{prefix}:"
+        return [
+            lbl["name"][len(prefix):].strip()
+            for lbl in self.issue["labels"] if lbl["name"].startswith(prefix)
+        ]
+
+    @property
+    def components(self):
+        return self.get_label("Component")
+
+    @property
+    def assignees(self):
+        return [a["login"] for a in self.issue["assignees"]]
+
+    @property
+    def current_fix_versions(self):
+        return self.issue.get("milestone", {}).get("title")
+
+    @property
+    def current_versions(self):
+        all_versions = self.github_api.get_milestones()
+
+        unreleased_versions = [x for x in all_versions if x["state"] == "open"]
+        unreleased_versions = [x["title"] for x in unreleased_versions]
+
+        return unreleased_versions
+
+    def resolve(self, fix_version, comment):
+        cur_status = self.issue["state"]
+
+        if cur_status == "closed":
+            self.cmd.fail("GitHub issue %s already has status '%s'"
+                          % (self.github_id, cur_status))
+
+        if DEBUG:
+            print("GitHub issue %s untouched -> %s" %
+                  (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)

Review Comment:
   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.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow] kou commented on pull request #14750: GH-14720: Update merge_arrow_pr script to accept GitHub issues

Posted by GitBox <gi...@apache.org>.
kou commented on PR #14750:
URL: https://github.com/apache/arrow/pull/14750#issuecomment-1332738188

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


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow] jorisvandenbossche commented on pull request #14750: GH-14720: Update merge_arrow_pr script to accept GitHub issues

Posted by GitBox <gi...@apache.org>.
jorisvandenbossche commented on PR #14750:
URL: https://github.com/apache/arrow/pull/14750#issuecomment-1333310503

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


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow] jorisvandenbossche commented on pull request #14750: GH-14720: [Dev] Update merge_arrow_pr script to accept GitHub issues

Posted by GitBox <gi...@apache.org>.
jorisvandenbossche commented on PR #14750:
URL: https://github.com/apache/arrow/pull/14750#issuecomment-1333727510

   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)


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow] github-actions[bot] commented on pull request #14750: GH-14720: Update merge_arrow_pr script to accept GitHub issues

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #14750:
URL: https://github.com/apache/arrow/pull/14750#issuecomment-1328946363

   <!--
     Licensed to the Apache Software Foundation (ASF) under one
     or more contributor license agreements.  See the NOTICE file
     distributed with this work for additional information
     regarding copyright ownership.  The ASF licenses this file
     to you under the Apache License, Version 2.0 (the
     "License"); you may not use this file except in compliance
     with the License.  You may obtain a copy of the License at
   
       http://www.apache.org/licenses/LICENSE-2.0
   
     Unless required by applicable law or agreed to in writing,
     software distributed under the License is distributed on an
     "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
     KIND, either express or implied.  See the License for the
     specific language governing permissions and limitations
     under the License.
   -->
   
   Thanks for opening a pull request!
   
   If this is not a [minor PR](https://github.com/apache/arrow/blob/master/CONTRIBUTING.md#Minor-Fixes). 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](http://theapacheway.com/open/#:~:text=Openness%20allows%20new%20users%20the,must%20happen%20in%20the%20open.) 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:
   
     * [Other pull requests](https://github.com/apache/arrow/pulls/)
     * [Contribution Guidelines - How to contribute patches](https://arrow.apache.org/docs/developers/contributing.html#how-to-contribute-patches)
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow] ursabot commented on pull request #14750: GH-14720: [Dev] Update merge_arrow_pr script to accept GitHub issues

Posted by GitBox <gi...@apache.org>.
ursabot commented on PR #14750:
URL: https://github.com/apache/arrow/pull/14750#issuecomment-1333991975

   Benchmark runs are scheduled for baseline = 958fbfa5fe567a908f5cbcc09dfc54a00e480be9 and contender = 1ba97a62944accfd756491438dd6325ff26c9660. 1ba97a62944accfd756491438dd6325ff26c9660 is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
   Conbench compare runs links:
   [Finished :arrow_down:0.0% :arrow_up:0.0%] [ec2-t3-xlarge-us-east-2](https://conbench.ursa.dev/compare/runs/297f7bdc7f944cd08bdcf27ed475a965...a29b32e690e74419b6a859695f77cd0f/)
   [Failed :arrow_down:0.2% :arrow_up:0.0%] [test-mac-arm](https://conbench.ursa.dev/compare/runs/b69a7568a710452996484b1f1e083829...4a0b81c142bf4ad7a721c7cfe86666d4/)
   [Finished :arrow_down:0.54% :arrow_up:0.0%] [ursa-i9-9960x](https://conbench.ursa.dev/compare/runs/496abd4c1acc427da8e3fa043457dc51...567f6951280b483ba7b486fa0ffb291f/)
   [Finished :arrow_down:0.24% :arrow_up:0.35%] [ursa-thinkcentre-m75q](https://conbench.ursa.dev/compare/runs/7d41c6746b8840fea3028f4ecc002b65...87d1fb49444c471cb22014396935de3d/)
   Buildkite builds:
   [Finished] [`1ba97a62` ec2-t3-xlarge-us-east-2](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ec2-t3-xlarge-us-east-2/builds/1955)
   [Finished] [`1ba97a62` test-mac-arm](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-test-mac-arm/builds/1977)
   [Finished] [`1ba97a62` ursa-i9-9960x](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-i9-9960x/builds/1947)
   [Finished] [`1ba97a62` ursa-thinkcentre-m75q](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-thinkcentre-m75q/builds/1969)
   [Finished] [`958fbfa5` ec2-t3-xlarge-us-east-2](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ec2-t3-xlarge-us-east-2/builds/1954)
   [Failed] [`958fbfa5` test-mac-arm](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-test-mac-arm/builds/1976)
   [Finished] [`958fbfa5` ursa-i9-9960x](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-i9-9960x/builds/1946)
   [Finished] [`958fbfa5` ursa-thinkcentre-m75q](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-thinkcentre-m75q/builds/1968)
   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
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow] raulcd commented on pull request #14750: GH-14720: [Dev] Update merge_arrow_pr script to accept GitHub issues

Posted by GitBox <gi...@apache.org>.
raulcd commented on PR #14750:
URL: https://github.com/apache/arrow/pull/14750#issuecomment-1333706685

   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 <ra...@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```


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow] raulcd commented on a diff in pull request #14750: GH-14720: [Dev] Update merge_arrow_pr script to accept GitHub issues

Posted by GitBox <gi...@apache.org>.
raulcd commented on code in PR #14750:
URL: https://github.com/apache/arrow/pull/14750#discussion_r1037053927


##########
dev/test_merge_arrow_pr.py:
##########
@@ -332,9 +346,10 @@ def test_jira_output_no_components():
 
 def test_sorting_versions():
     versions_json = [
+        {'name': '11.0.0', 'released': False},
         {'name': '9.0.0', 'released': False},
         {'name': '10.0.0', 'released': False},
     ]
     versions = [FakeVersion(raw['name'], raw) for raw in versions_json]
-    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"

Review Comment:
   Sounds like a good idea. I've added a ticket to track this separately if that's ok: https://github.com/apache/arrow/issues/14802



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow] raulcd commented on a diff in pull request #14750: GH-14720: [Dev] Update merge_arrow_pr script to accept GitHub issues

Posted by GitBox <gi...@apache.org>.
raulcd commented on code in PR #14750:
URL: https://github.com/apache/arrow/pull/14750#discussion_r1037050892


##########
dev/merge_arrow_pr.py:
##########
@@ -487,25 +613,19 @@ def get_primary_author(cmd, distinct_authors):
     return primary_author, distinct_other_authors
 
 
-def prompt_for_fix_version(cmd, jira_issue, maintenance_branches=()):
-    (all_versions,
-     default_fix_versions) = jira_issue.get_candidate_fix_versions(
+def prompt_for_fix_version(cmd, issue, maintenance_branches=()):
+    default_fix_version = get_candidate_fix_version(
+        issue.current_versions,
         maintenance_branches=maintenance_branches
     )
 
-    default_fix_versions = ",".join(default_fix_versions)
-
-    issue_fix_versions = cmd.prompt("Enter comma-separated "
-                                    "fix version(s) [%s]: "
-                                    % default_fix_versions)
-    if issue_fix_versions == "":
-        issue_fix_versions = default_fix_versions
-    issue_fix_versions = issue_fix_versions.replace(" ", "").split(",")
-
-    def get_version_json(version_str):
-        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 "

Review Comment:
   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.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow] jorisvandenbossche merged pull request #14750: GH-14720: [Dev] Update merge_arrow_pr script to accept GitHub issues

Posted by GitBox <gi...@apache.org>.
jorisvandenbossche merged PR #14750:
URL: https://github.com/apache/arrow/pull/14750


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow] jorisvandenbossche commented on pull request #14750: GH-14720: Update merge_arrow_pr script to accept GitHub issues

Posted by GitBox <gi...@apache.org>.
jorisvandenbossche commented on PR #14750:
URL: https://github.com/apache/arrow/pull/14750#issuecomment-1330496864

   FYI, I tested the changes to the merge script by merging https://github.com/apache/arrow/pull/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.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow] amol- commented on a diff in pull request #14750: GH-14720: Update merge_arrow_pr script to accept GitHub issues

Posted by GitBox <gi...@apache.org>.
amol- commented on code in PR #14750:
URL: https://github.com/apache/arrow/pull/14750#discussion_r1034712678


##########
dev/merge_arrow_pr.py:
##########
@@ -581,20 +677,20 @@ def cli():
 
     pr.merge()
 
-    if pr.jira_issue is None:
-        print("Minor PR.  No JIRA issue to update.\n")
+    if pr.issue is None:
+        print("Minor PR.  No issue to update.\n")
         return
 
-    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]"

Review Comment:
   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. 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org