From d0590b0248c536fb0a9a6bf5a4620b86c6bd3f29 Mon Sep 17 00:00:00 2001 From: Alessandro Molina Date: Mon, 28 Nov 2022 12:50:27 +0100 Subject: [PATCH 01/15] Initial effort to update merge_pr script for Github --- dev/merge_arrow_pr.py | 147 ++++++++++++++++++++++++++++++++++++------ 1 file changed, 128 insertions(+), 19 deletions(-) diff --git a/dev/merge_arrow_pr.py b/dev/merge_arrow_pr.py index 490ee787e3374..39084b5727045 100755 --- a/dev/merge_arrow_pr.py +++ b/dev/merge_arrow_pr.py @@ -62,7 +62,7 @@ # For testing to avoid accidentally pushing to apache DEBUG = bool(int(os.environ.get("DEBUG", 0))) - +DEBUG = True if DEBUG: print("**************** DEBUGGING ****************") @@ -115,12 +115,6 @@ def fix_version_from_branch(branch, versions): return [x for x in versions if x.name.startswith(branch_ver)][-1] -# We can merge both ARROW and PARQUET patches -SUPPORTED_PROJECTS = ['ARROW', 'PARQUET'] -PR_TITLE_REGEXEN = [(project, re.compile(r'^(' + project + r'-[0-9]+)\b.*$')) - for project in SUPPORTED_PROJECTS] - - class JiraIssue(object): def __init__(self, jira_con, jira_id, project, cmd): @@ -226,6 +220,111 @@ def show(self): fields.summary, fields.assignee, fields.components)) +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)) + + @property + def current_fix_versions(self): + return self.issue.fields.fixVersions + + @classmethod + def sort_versions(cls, versions): + def version_tuple(x): + # Parquet versions are something like cpp-1.2.0 + numeric_version = x.name.split("-", 1)[-1] + return tuple(int(_) for _ in numeric_version.split(".")) + return sorted(versions, key=version_tuple, reverse=True) + + def get_candidate_fix_versions(self, merge_branches=('master',), + maintenance_branches=()): + # Only suggest versions starting with a number, like 0.x but not JS-0.x + all_versions = self.jira_con.project_versions(self.project) + unreleased_versions = [x for x in all_versions + if not x.raw['released']] + + mainline_versions = self._filter_mainline_versions(unreleased_versions) + mainline_versions = self.sort_versions(mainline_versions) + + mainline_non_patch_versions = [] + for v in mainline_versions: + (major, minor, patch) = v.name.split(".") + if patch == "0": + mainline_non_patch_versions.append(v) + + if len(mainline_versions) > len(mainline_non_patch_versions): + # If there is a non-patch release, suggest that instead + mainline_versions = mainline_non_patch_versions + + mainline_versions = self._filter_maintenance_versions( + mainline_versions, maintenance_branches + ) + default_fix_versions = [ + fix_version_from_branch(x, mainline_versions).name + for x in merge_branches] + + return all_versions, default_fix_versions + + def _filter_maintenance_versions(self, versions, maintenance_branches): + return [v for v in versions + if f"maint-{v.name}" not in maintenance_branches] + + def _filter_mainline_versions(self, versions): + if self.project == 'PARQUET': + mainline_regex = re.compile(r'cpp-\d.*') + else: + mainline_regex = re.compile(r'\d.*') + + return [x for x in versions if mainline_regex.match(x.name)] + + def resolve(self, fix_versions, 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_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)) + + 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)) + + def format_jira_output(jira_id, status, summary, assignee, components): if assignee is None: @@ -269,6 +368,10 @@ def __init__(self, project_name, cmd): } self.headers = headers + def get_issue_data(self, number): + return get_json("%s/issues/%s" % (self.github_api, number), + headers=self.headers) + def get_pr_data(self, number): return get_json("%s/pulls/%s" % (self.github_api, number), headers=self.headers) @@ -322,6 +425,11 @@ def continue_maybe(self, prompt): class PullRequest(object): + # We can merge both ARROW and PARQUET patches + GITHUB_PR_TITLE_REGEXEN = r'^(GH-[0-9]+)\b.*$' + JIRA_SUPPORTED_PROJECTS = ['ARROW', 'PARQUET'] + JIRA_PR_TITLE_REGEXEN = [(project, re.compile(r'^(' + project + r'-[0-9]+)\b.*$')) + for project in JIRA_SUPPORTED_PROJECTS] def __init__(self, cmd, github_api, git_remote, jira_con, number): self.cmd = cmd @@ -342,7 +450,7 @@ def __init__(self, cmd, github_api, git_remote, jira_con, number): raise self.description = "%s/%s" % (self.user_login, self.base_ref) - self.jira_issue = self._get_jira() + self.issue = self._get_issue() def show(self): print("\n=== Pull Request #%s ===" % self.number) @@ -366,24 +474,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 " + "{0}, but found {1}".format(options, self.title)) def merge(self): """ From 80e139364bb725b4f4476656a62759c5e4f2690d Mon Sep 17 00:00:00 2001 From: Alessandro Molina Date: Mon, 28 Nov 2022 15:04:46 +0100 Subject: [PATCH 02/15] Nearly working merge_script --- dev/merge_arrow_pr.py | 239 ++++++++++++++++++------------------------ 1 file changed, 104 insertions(+), 135 deletions(-) diff --git a/dev/merge_arrow_pr.py b/dev/merge_arrow_pr.py index 39084b5727045..a25b115572127 100755 --- a/dev/merge_arrow_pr.py +++ b/dev/merge_arrow_pr.py @@ -73,6 +73,8 @@ def get_json(url, headers=None): response = requests.get(url, headers=headers) + if response.status_code != 200: + raise ValueError(response.json()) return response.json() @@ -112,7 +114,7 @@ def fix_version_from_branch(branch, versions): return versions[-1] else: branch_ver = branch.replace("branch-", "") - return [x for x in versions if x.name.startswith(branch_ver)][-1] + return [v for v in versions if v.startswith(branch_ver)][-1] class JiraIssue(object): @@ -132,46 +134,15 @@ def __init__(self, jira_con, jira_id, project, cmd): def current_fix_versions(self): return self.issue.fields.fixVersions - @classmethod - def sort_versions(cls, versions): - def version_tuple(x): - # Parquet versions are something like cpp-1.2.0 - numeric_version = x.name.split("-", 1)[-1] - return tuple(int(_) for _ in numeric_version.split(".")) - return sorted(versions, key=version_tuple, reverse=True) - - def get_candidate_fix_versions(self, merge_branches=('master',), - maintenance_branches=()): + @property + def current_versions(self): # Only suggest versions starting with a number, like 0.x but not JS-0.x all_versions = self.jira_con.project_versions(self.project) unreleased_versions = [x for x in all_versions if not x.raw['released']] mainline_versions = self._filter_mainline_versions(unreleased_versions) - mainline_versions = self.sort_versions(mainline_versions) - - mainline_non_patch_versions = [] - for v in mainline_versions: - (major, minor, patch) = v.name.split(".") - if patch == "0": - mainline_non_patch_versions.append(v) - - if len(mainline_versions) > len(mainline_non_patch_versions): - # If there is a non-patch release, suggest that instead - mainline_versions = mainline_non_patch_versions - - mainline_versions = self._filter_maintenance_versions( - mainline_versions, maintenance_branches - ) - default_fix_versions = [ - fix_version_from_branch(x, mainline_versions).name - for x in merge_branches] - - return all_versions, default_fix_versions - - def _filter_maintenance_versions(self, versions, maintenance_branches): - return [v for v in versions - if f"maint-{v.name}" not in maintenance_branches] + return mainline_versions def _filter_mainline_versions(self, versions): if self.project == 'PARQUET': @@ -216,9 +187,9 @@ def resolve(self, fix_versions, comment): 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)) class GitHubIssue(object): @@ -232,118 +203,111 @@ def __init__(self, github_api, github_id, cmd): except Exception as e: self.cmd.fail("Github could not find %s\n%s" % (github_id, e)) - @property - def current_fix_versions(self): - return self.issue.fields.fixVersions - - @classmethod - def sort_versions(cls, versions): - def version_tuple(x): - # Parquet versions are something like cpp-1.2.0 - numeric_version = x.name.split("-", 1)[-1] - return tuple(int(_) for _ in numeric_version.split(".")) - return sorted(versions, key=version_tuple, reverse=True) - - def get_candidate_fix_versions(self, merge_branches=('master',), - maintenance_branches=()): - # Only suggest versions starting with a number, like 0.x but not JS-0.x - all_versions = self.jira_con.project_versions(self.project) - unreleased_versions = [x for x in all_versions - if not x.raw['released']] + def get_label(self, prefix): + prefix = f"{prefix}:" + return [l["name"][len(prefix):].strip() for l in self.issue["labels"] if l["name"].startswith(prefix)] - mainline_versions = self._filter_mainline_versions(unreleased_versions) - mainline_versions = self.sort_versions(mainline_versions) - - mainline_non_patch_versions = [] - for v in mainline_versions: - (major, minor, patch) = v.name.split(".") - if patch == "0": - mainline_non_patch_versions.append(v) - - if len(mainline_versions) > len(mainline_non_patch_versions): - # If there is a non-patch release, suggest that instead - mainline_versions = mainline_non_patch_versions + @property + def components(self): + return self.get_label("Component") - mainline_versions = self._filter_maintenance_versions( - mainline_versions, maintenance_branches - ) - default_fix_versions = [ - fix_version_from_branch(x, mainline_versions).name - for x in merge_branches] + @property + def assignees(self): + return [a["login"] for a in self.issue["assignees"]] - return all_versions, default_fix_versions + @property + def current_fix_versions(self): + return self.issue.get("milestone", {}).get("title") - def _filter_maintenance_versions(self, versions, maintenance_branches): - return [v for v in versions - if f"maint-{v.name}" not in maintenance_branches] + @property + def current_versions(self): + all_versions = self.github_api.get_milestones() - def _filter_mainline_versions(self, versions): - if self.project == 'PARQUET': - mainline_regex = re.compile(r'cpp-\d.*') - else: - mainline_regex = re.compile(r'\d.*') + unreleased_versions = [x for x in all_versions if x["state"] == "open"] + unreleased_versions = [x["title"] for x in unreleased_versions] - return [x for x in versions if mainline_regex.match(x.name)] + return unreleased_versions def resolve(self, fix_versions, comment): - fields = self.issue.fields - cur_status = fields.status.name + cur_status = self.issue["state"] - if cur_status == "Resolved" or cur_status == "Closed": - self.cmd.fail("JIRA issue %s already has status '%s'" - % (self.jira_id, cur_status)) + if cur_status == "closed": + self.cmd.fail("GitHub issue %s already has status '%s'" + % (self.github_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_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)) + print("GitHub issue %s untouched -> %s" % (self.github_id, 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.issue = self.github_api.get_issue_data(self.github_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)) - - - -def format_jira_output(jira_id, status, summary, assignee, components): - if assignee is None: + issue = self.issue + print(format_issue_output("github", self.github_id, issue["state"], + issue["title"], ', '.join(self.assignees), + self.components)) + + +def get_candidate_fix_versions(mainline_versions, + merge_branches=('master',), + maintenance_branches=()): + all_versions = [getattr(v, "name", v) for v in mainline_versions] + + def version_tuple(x): + # Parquet versions are something like cpp-1.2.0 + numeric_version = getattr(x, "name", x).split("-", 1)[-1] + return tuple(int(_) for _ in numeric_version.split(".")) + all_versions = sorted(all_versions, key=version_tuple, reverse=True) + + # Only suggest versions starting with a number, like 0.x but not JS-0.x + mainline_versions = all_versions + mainline_non_patch_versions = [] + for v in mainline_versions: + (major, minor, patch) = v.split(".") + if patch == "0": + mainline_non_patch_versions.append(v) + + if len(mainline_versions) > len(mainline_non_patch_versions): + # If there is a non-patch release, suggest that instead + mainline_versions = mainline_non_patch_versions + + mainline_versions = [v for v in mainline_versions + if f"maint-{v}" not in maintenance_branches] + default_fix_versions = [ + fix_version_from_branch(x, mainline_versions) + for x in merge_branches] + + return all_versions, default_fix_versions + + +def format_issue_output(issue_type, issue_id, status, summary, assignee, components): + if not assignee: assignee = "NOT ASSIGNED!!!" else: - assignee = assignee.displayName + assignee = getattr(assignee, "displayName", assignee) if len(components) == 0: components = 'NO COMPONENTS!!!' else: - components = ', '.join((x.name for x in components)) + components = ', '.join((getattr(x, "name", x) for x in components)) - return """=== JIRA {} === + if issue_type == "jira": + url = '/'.join((JIRA_API_BASE, 'browse', issue_id)) + else: + url = f'https://github.com/apache/arrow/issues/{issue_id}' + + return """=== {} {} === Summary\t\t{} Assignee\t{} Components\t{} Status\t\t{} -URL\t\t{}/{}""".format(jira_id, summary, assignee, components, status, - '/'.join((JIRA_API_BASE, 'browse')), jira_id) +URL\t\t{}""".format(issue_type.upper(), issue_id, summary, assignee, + components, status, url) class GitHubAPI(object): @@ -368,6 +332,10 @@ def __init__(self, project_name, cmd): } self.headers = headers + def get_milestones(self): + return get_json("%s/milestones" % (self.github_api, ), + headers=self.headers) + def get_issue_data(self, number): return get_json("%s/issues/%s" % (self.github_api, number), headers=self.headers) @@ -426,7 +394,7 @@ def continue_maybe(self, prompt): class PullRequest(object): # We can merge both ARROW and PARQUET patches - GITHUB_PR_TITLE_REGEXEN = r'^(GH-[0-9]+)\b.*$' + GITHUB_PR_TITLE_REGEXEN = re.compile(r'^GH-([0-9]+)\b.*$') JIRA_SUPPORTED_PROJECTS = ['ARROW', 'PARQUET'] JIRA_PR_TITLE_REGEXEN = [(project, re.compile(r'^(' + project + r'-[0-9]+)\b.*$')) for project in JIRA_SUPPORTED_PROJECTS] @@ -456,8 +424,8 @@ def show(self): print("\n=== Pull Request #%s ===" % self.number) print("title\t%s\nsource\t%s\ntarget\t%s\nurl\t%s" % (self.title, self.description, self.target_ref, self.url)) - if self.jira_issue is not None: - self.jira_issue.show() + if self.issue is not None: + self.issue.show() else: print("Minor PR. Please ensure it meets guidelines for minor.\n") @@ -596,9 +564,10 @@ def get_primary_author(cmd, distinct_authors): return primary_author, distinct_other_authors -def prompt_for_fix_version(cmd, jira_issue, maintenance_branches=()): +def prompt_for_fix_version(cmd, issue, maintenance_branches=()): (all_versions, - default_fix_versions) = jira_issue.get_candidate_fix_versions( + default_fix_versions) = get_candidate_fix_versions( + issue.current_versions, maintenance_branches=maintenance_branches ) @@ -612,7 +581,7 @@ def prompt_for_fix_version(cmd, jira_issue, maintenance_branches=()): 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 [v for v in all_versions if v == version_str][0] return [get_version_json(v) for v in issue_fix_versions] @@ -690,20 +659,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 = ( + issue_comment = ( "Issue resolved by pull request %s\n[%s/%s]" % (pr_num, "https://github.com/apache/" + PROJECT_NAME + "/pull", pr_num)) - fix_versions_json = prompt_for_fix_version(cmd, pr.jira_issue, + fix_versions_json = prompt_for_fix_version(cmd, pr.issue, pr.maintenance_branches) - pr.jira_issue.resolve(fix_versions_json, jira_comment) + pr.issue.resolve(fix_versions_json, issue_comment) if __name__ == '__main__': From 9ebb8f13b33094489d90a4d00459d23e19975d8c Mon Sep 17 00:00:00 2001 From: Alessandro Molina Date: Mon, 28 Nov 2022 15:52:31 +0100 Subject: [PATCH 03/15] Nearly complete --- dev/merge_arrow_pr.py | 95 ++++++++++++++++++++++++++----------------- 1 file changed, 57 insertions(+), 38 deletions(-) diff --git a/dev/merge_arrow_pr.py b/dev/merge_arrow_pr.py index a25b115572127..0db08d89c868e 100755 --- a/dev/merge_arrow_pr.py +++ b/dev/merge_arrow_pr.py @@ -152,7 +152,7 @@ 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 @@ -160,27 +160,25 @@ def resolve(self, fix_versions, comment): 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() @@ -228,7 +226,7 @@ def current_versions(self): return unreleased_versions - def resolve(self, fix_versions, comment): + def resolve(self, fix_version, comment): cur_status = self.issue["state"] if cur_status == "closed": @@ -236,11 +234,10 @@ def resolve(self, fix_versions, comment): % (self.github_id, cur_status)) if DEBUG: - print("GitHub issue %s untouched -> %s" % (self.github_id, fix_versions)) + print("GitHub issue %s untouched -> %s" % (self.github_id, fix_version)) else: - self.jira_con.transition_issue(self.jira_id, resolve["id"], - comment=comment, - fixVersions=fix_versions) + self.github_api.assign_milestone(self.github_id, fix_version) + self.github_api.close_issue(self.github_id, comment) print("Successfully resolved %s!" % (self.jira_id)) self.issue = self.github_api.get_issue_data(self.github_id) @@ -253,9 +250,9 @@ def show(self): self.components)) -def get_candidate_fix_versions(mainline_versions, - merge_branches=('master',), - maintenance_branches=()): +def get_candidate_fix_version(mainline_versions, + merge_branches=('master',), + maintenance_branches=()): all_versions = [getattr(v, "name", v) for v in mainline_versions] def version_tuple(x): @@ -282,7 +279,7 @@ def version_tuple(x): fix_version_from_branch(x, mainline_versions) for x in merge_branches] - return all_versions, default_fix_versions + return default_fix_versions[0] def format_issue_output(issue_type, issue_id, status, summary, assignee, components): @@ -336,6 +333,11 @@ def get_milestones(self): return get_json("%s/milestones" % (self.github_api, ), headers=self.headers) + def get_milestone_number(self, version): + return next(( + m["number"] for m in self.get_milestones() if m["title"] == version + ), None) + def get_issue_data(self, number): return get_json("%s/issues/%s" % (self.github_api, number), headers=self.headers) @@ -352,6 +354,31 @@ 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) + if r.status_code != 200: + raise ValueError(f"Failed request: {comment_url} -> {r.json()}") + + r = requests.patch(issue_url, json={"state": "closed"}, headers=self.headers) + if r.status_code != 200: + raise ValueError(f"Failed request: {issue_url} -> {r.json()}") + + def assign_milestone(self, number, version): + url = f'{self.github_api}/issues/{number}' + milestone_number = self.get_milestone_number(version) + if not milestone_number: + raise ValueError(f"Invalid version {version}, milestone not found") + payload = { + 'milestone': milestone_number + } + r = requests.patch(url, headers=self.headers, json=payload) + if r.status_code != 200: + raise ValueError(f"Failed request: {url} -> {r.json()}") + return r.json() + def merge_pr(self, number, commit_title, commit_message): url = f'{self.github_api}/pulls/{number}/merge' payload = { @@ -565,26 +592,18 @@ def get_primary_author(cmd, distinct_authors): def prompt_for_fix_version(cmd, issue, maintenance_branches=()): - (all_versions, - default_fix_versions) = get_candidate_fix_versions( + 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 " + issue_fix_version = 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 [v for v in all_versions if v == version_str][0] - - return [get_version_json(v) for v in issue_fix_versions] - + % default_fix_version) + if issue_fix_version == "": + issue_fix_version = default_fix_version + issue_fix_version = issue_fix_version.strip() + return issue_fix_version CONFIG_FILE = "~/.config/arrow/merge.conf" @@ -670,9 +689,9 @@ def cli(): "https://github.com/apache/" + PROJECT_NAME + "/pull", pr_num)) - fix_versions_json = prompt_for_fix_version(cmd, pr.issue, - pr.maintenance_branches) - pr.issue.resolve(fix_versions_json, issue_comment) + fix_version = prompt_for_fix_version(cmd, pr.issue, + pr.maintenance_branches) + pr.issue.resolve(fix_version, issue_comment) if __name__ == '__main__': From 9e4769cce17d2346def34da0a2a7c30ece378a75 Mon Sep 17 00:00:00 2001 From: Alessandro Molina Date: Mon, 28 Nov 2022 16:54:31 +0100 Subject: [PATCH 04/15] Remove hardcoded debug --- dev/merge_arrow_pr.py | 1 - 1 file changed, 1 deletion(-) diff --git a/dev/merge_arrow_pr.py b/dev/merge_arrow_pr.py index 0db08d89c868e..e7a37b0fe4bdf 100755 --- a/dev/merge_arrow_pr.py +++ b/dev/merge_arrow_pr.py @@ -62,7 +62,6 @@ # For testing to avoid accidentally pushing to apache DEBUG = bool(int(os.environ.get("DEBUG", 0))) -DEBUG = True if DEBUG: print("**************** DEBUGGING ****************") From b9b30216a7b41472fa6f9030068b69f3c72d3fc6 Mon Sep 17 00:00:00 2001 From: Alessandro Molina Date: Tue, 29 Nov 2022 13:06:21 +0100 Subject: [PATCH 05/15] Update wording --- dev/merge_arrow_pr.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dev/merge_arrow_pr.py b/dev/merge_arrow_pr.py index e7a37b0fe4bdf..64d8a7e46630e 100755 --- a/dev/merge_arrow_pr.py +++ b/dev/merge_arrow_pr.py @@ -681,7 +681,7 @@ def cli(): print("Minor PR. No issue to update.\n") return - cmd.continue_maybe("Would you like to update the associated JIRA?") + cmd.continue_maybe("Would you like to update the associated issue?") issue_comment = ( "Issue resolved by pull request %s\n[%s/%s]" % (pr_num, From 3ad943fbe235616b27d913d24a0348383209f45f Mon Sep 17 00:00:00 2001 From: Alessandro Molina Date: Tue, 29 Nov 2022 13:09:14 +0100 Subject: [PATCH 06/15] Make target organization configurable, so that we can test on forks --- dev/merge_arrow_pr.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/dev/merge_arrow_pr.py b/dev/merge_arrow_pr.py index 64d8a7e46630e..69bab810e9ce3 100755 --- a/dev/merge_arrow_pr.py +++ b/dev/merge_arrow_pr.py @@ -295,7 +295,7 @@ def format_issue_output(issue_type, issue_id, status, summary, assignee, compone if issue_type == "jira": url = '/'.join((JIRA_API_BASE, 'browse', issue_id)) else: - url = f'https://github.com/apache/arrow/issues/{issue_id}' + url = f'https://github.com/{PR_REMOTE_NAME}/arrow/issues/{issue_id}' return """=== {} {} === Summary\t\t{} @@ -309,7 +309,7 @@ def format_issue_output(issue_type, issue_id, status, summary, assignee, compone class GitHubAPI(object): def __init__(self, project_name, cmd): - self.github_api = ("https://api.github.com/repos/apache/{0}" + self.github_api = ("https://api.github.com/repos/{PR_REMOTE_NAME}/{0}" .format(project_name)) token = None @@ -685,7 +685,7 @@ def cli(): issue_comment = ( "Issue resolved by pull request %s\n[%s/%s]" % (pr_num, - "https://github.com/apache/" + PROJECT_NAME + "/pull", + f"https://github.com/{PR_REMOTE_NAME}/{PROJECT_NAME}/pull", pr_num)) fix_version = prompt_for_fix_version(cmd, pr.issue, From 3484b772db9ca8719759c654b46466178c880109 Mon Sep 17 00:00:00 2001 From: Alessandro Molina Date: Tue, 29 Nov 2022 13:16:56 +0100 Subject: [PATCH 07/15] Tweaks --- dev/merge_arrow_pr.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/dev/merge_arrow_pr.py b/dev/merge_arrow_pr.py index 69bab810e9ce3..d3fe76f484928 100755 --- a/dev/merge_arrow_pr.py +++ b/dev/merge_arrow_pr.py @@ -58,7 +58,8 @@ sys.exit(1) # Remote name which points to the GitHub site -PR_REMOTE_NAME = os.environ.get("PR_REMOTE_NAME", "apache") +PR_REMOTE_NAME = os.environ.get("PR_REMOTE_NAME") or "apache" +PROJECT_NAME = os.environ.get('ARROW_PROJECT_NAME') or "arrow" # For testing to avoid accidentally pushing to apache DEBUG = bool(int(os.environ.get("DEBUG", 0))) @@ -295,7 +296,7 @@ def format_issue_output(issue_type, issue_id, status, summary, assignee, compone if issue_type == "jira": url = '/'.join((JIRA_API_BASE, 'browse', issue_id)) else: - url = f'https://github.com/{PR_REMOTE_NAME}/arrow/issues/{issue_id}' + url = f'https://github.com/{PR_REMOTE_NAME}/{PROJECT_NAME}/issues/{issue_id}' return """=== {} {} === Summary\t\t{} @@ -309,8 +310,7 @@ def format_issue_output(issue_type, issue_id, status, summary, assignee, compone class GitHubAPI(object): def __init__(self, project_name, cmd): - self.github_api = ("https://api.github.com/repos/{PR_REMOTE_NAME}/{0}" - .format(project_name)) + self.github_api = f"https://api.github.com/repos/{PR_REMOTE_NAME}/{project_name}" token = None config = load_configuration() @@ -648,9 +648,9 @@ def get_pr_num(): def cli(): # Location of your Arrow git clone ARROW_HOME = os.path.abspath(os.path.dirname(__file__)) - PROJECT_NAME = os.environ.get('ARROW_PROJECT_NAME') or 'arrow' - print("ARROW_HOME = " + ARROW_HOME) - print("PROJECT_NAME = " + PROJECT_NAME) + print(f"ARROW_HOME = {ARROW_HOME}") + print(f"PR_REMOTE_NAME = {PR_REMOTE_NAME}") + print(f"PROJECT_NAME = {PROJECT_NAME}") cmd = CommandInput() From 53727e8122497d728c557d709185a5fce74301be Mon Sep 17 00:00:00 2001 From: Alessandro Molina Date: Tue, 29 Nov 2022 13:47:04 +0100 Subject: [PATCH 08/15] Remaining tweaks --- dev/merge_arrow_pr.py | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/dev/merge_arrow_pr.py b/dev/merge_arrow_pr.py index d3fe76f484928..367a745a5e8ab 100755 --- a/dev/merge_arrow_pr.py +++ b/dev/merge_arrow_pr.py @@ -238,7 +238,7 @@ def resolve(self, fix_version, comment): else: self.github_api.assign_milestone(self.github_id, fix_version) self.github_api.close_issue(self.github_id, comment) - print("Successfully resolved %s!" % (self.jira_id)) + print("Successfully resolved %s!" % (self.github_id)) self.issue = self.github_api.get_issue_data(self.github_id) self.show() @@ -358,12 +358,12 @@ def close_issue(self, number, comment): comment_url = f'{self.github_api}/issues/{number}/comments' r = requests.post(comment_url, json={"body": comment}, headers=self.headers) - if r.status_code != 200: - raise ValueError(f"Failed request: {comment_url} -> {r.json()}") + if not r.ok: + raise ValueError(f"Failed request: {comment_url}:{r.status_code} -> {r.json()}") r = requests.patch(issue_url, json={"state": "closed"}, headers=self.headers) - if r.status_code != 200: - raise ValueError(f"Failed request: {issue_url} -> {r.json()}") + if not r.ok: + raise ValueError(f"Failed request: {issue_url}:{r.status_code} -> {r.json()}") def assign_milestone(self, number, version): url = f'{self.github_api}/issues/{number}' @@ -374,8 +374,8 @@ def assign_milestone(self, number, version): 'milestone': milestone_number } r = requests.patch(url, headers=self.headers, json=payload) - if r.status_code != 200: - raise ValueError(f"Failed request: {url} -> {r.json()}") + if not r.ok: + raise ValueError(f"Failed request: {url}:{r.status_code} -> {r.json()}") return r.json() def merge_pr(self, number, commit_title, commit_message): @@ -683,11 +683,10 @@ def cli(): cmd.continue_maybe("Would you like to update the associated issue?") issue_comment = ( - "Issue resolved by pull request %s\n[%s/%s]" + "Issue resolved by pull request %s\n%s" % (pr_num, - f"https://github.com/{PR_REMOTE_NAME}/{PROJECT_NAME}/pull", - pr_num)) - + f"https://github.com/{PR_REMOTE_NAME}/{PROJECT_NAME}/pull/{pr_num}") + ) fix_version = prompt_for_fix_version(cmd, pr.issue, pr.maintenance_branches) pr.issue.resolve(fix_version, issue_comment) From 99b0d2e700fba8bf5bac276a700936f5d354fbef Mon Sep 17 00:00:00 2001 From: Alessandro Molina Date: Tue, 29 Nov 2022 13:56:55 +0100 Subject: [PATCH 09/15] lint --- dev/merge_arrow_pr.py | 42 ++++++++++++++++++++++++++---------------- 1 file changed, 26 insertions(+), 16 deletions(-) diff --git a/dev/merge_arrow_pr.py b/dev/merge_arrow_pr.py index 367a745a5e8ab..f9bb025c49d6d 100755 --- a/dev/merge_arrow_pr.py +++ b/dev/merge_arrow_pr.py @@ -165,7 +165,8 @@ def resolve(self, fix_version, comment): # ARROW-6915: do not overwrite existing fix versions corresponding to # point releases - fix_versions = [v.raw for v in self.jira_con.project_versions(self.project) if v.name == fix_version] + 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('.') @@ -173,11 +174,12 @@ def resolve(self, fix_version, comment): fix_versions.append(version.raw) if DEBUG: - print("JIRA issue %s untouched -> %s" % (self.jira_id, [v["name"] for v in fix_versions])) + 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) + comment=comment, + fixVersions=fix_versions) print("Successfully resolved %s!" % (self.jira_id)) self.issue = self.jira_con.issue(self.jira_id) @@ -189,6 +191,7 @@ def show(self): fields.summary, fields.assignee, fields.components)) + class GitHubIssue(object): def __init__(self, github_api, github_id, cmd): @@ -234,7 +237,8 @@ def resolve(self, fix_version, comment): % (self.github_id, cur_status)) if DEBUG: - print("GitHub issue %s untouched -> %s" % (self.github_id, fix_version)) + 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) @@ -274,7 +278,7 @@ def version_tuple(x): mainline_versions = mainline_non_patch_versions mainline_versions = [v for v in mainline_versions - if f"maint-{v}" not in maintenance_branches] + if f"maint-{v}" not in maintenance_branches] default_fix_versions = [ fix_version_from_branch(x, mainline_versions) for x in merge_branches] @@ -303,7 +307,7 @@ def format_issue_output(issue_type, issue_id, status, summary, assignee, compone Assignee\t{} Components\t{} Status\t\t{} -URL\t\t{}""".format(issue_type.upper(), issue_id, summary, assignee, +URL\t\t{}""".format(issue_type.upper(), issue_id, summary, assignee, components, status, url) @@ -357,13 +361,17 @@ 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) + r = requests.post(comment_url, json={ + "body": comment}, headers=self.headers) if not r.ok: - raise ValueError(f"Failed request: {comment_url}:{r.status_code} -> {r.json()}") + raise ValueError( + f"Failed request: {comment_url}:{r.status_code} -> {r.json()}") - r = requests.patch(issue_url, json={"state": "closed"}, headers=self.headers) + r = requests.patch( + issue_url, json={"state": "closed"}, headers=self.headers) if not r.ok: - raise ValueError(f"Failed request: {issue_url}:{r.status_code} -> {r.json()}") + raise ValueError( + f"Failed request: {issue_url}:{r.status_code} -> {r.json()}") def assign_milestone(self, number, version): url = f'{self.github_api}/issues/{number}' @@ -375,7 +383,8 @@ def assign_milestone(self, number, version): } r = requests.patch(url, headers=self.headers, json=payload) if not r.ok: - raise ValueError(f"Failed request: {url}:{r.status_code} -> {r.json()}") + raise ValueError( + f"Failed request: {url}:{r.status_code} -> {r.json()}") return r.json() def merge_pr(self, number, commit_title, commit_message): @@ -484,9 +493,9 @@ def _get_issue(self): return JiraIssue(self.con, jira_id, project, self.cmd) options = ' or '.join('{0}-XXX'.format(project) - for project in self.JIRA_SUPPORTED_PROJECTS) + for project in self.JIRA_SUPPORTED_PROJECTS) self.cmd.fail("PR title should be prefixed by a jira id " - "{0}, but found {1}".format(options, self.title)) + "{0}, but found {1}".format(options, self.title)) def merge(self): """ @@ -597,13 +606,14 @@ def prompt_for_fix_version(cmd, issue, maintenance_branches=()): ) issue_fix_version = cmd.prompt("Enter comma-separated " - "fix version(s) [%s]: " - % default_fix_version) + "fix version(s) [%s]: " + % default_fix_version) if issue_fix_version == "": issue_fix_version = default_fix_version issue_fix_version = issue_fix_version.strip() return issue_fix_version + CONFIG_FILE = "~/.config/arrow/merge.conf" From 5b0521a622b72a41975cbac9a1b56fd18c5df203 Mon Sep 17 00:00:00 2001 From: Alessandro Molina Date: Tue, 29 Nov 2022 14:20:47 +0100 Subject: [PATCH 10/15] lint --- dev/merge_arrow_pr.py | 38 ++++++++++++++++++++++++++------------ 1 file changed, 26 insertions(+), 12 deletions(-) diff --git a/dev/merge_arrow_pr.py b/dev/merge_arrow_pr.py index f9bb025c49d6d..485471156a240 100755 --- a/dev/merge_arrow_pr.py +++ b/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 +# (set to 'apache' by default) # - DEBUG: use for testing to avoid pushing to apache (0 by default) import configparser @@ -58,7 +58,11 @@ sys.exit(1) # Remote name which points to the GitHub site -PR_REMOTE_NAME = os.environ.get("PR_REMOTE_NAME") or "apache" +ORG_NAME = ( + os.environ.get("ORG_NAME") or + os.environ.get("PR_REMOTE_NAME") or # backward compatibility + "apache" +) PROJECT_NAME = os.environ.get('ARROW_PROJECT_NAME') or "arrow" # For testing to avoid accidentally pushing to apache @@ -206,7 +210,10 @@ def __init__(self, github_api, github_id, cmd): def get_label(self, prefix): prefix = f"{prefix}:" - return [l["name"][len(prefix):].strip() for l in self.issue["labels"] if l["name"].startswith(prefix)] + return [ + lbl["name"][len(prefix):].strip() + for lbl in self.issue["labels"] if lbl["name"].startswith(prefix) + ] @property def components(self): @@ -286,7 +293,8 @@ def version_tuple(x): return default_fix_versions[0] -def format_issue_output(issue_type, issue_id, status, summary, assignee, components): +def format_issue_output(issue_type, issue_id, status, + summary, assignee, components): if not assignee: assignee = "NOT ASSIGNED!!!" else: @@ -300,7 +308,9 @@ def format_issue_output(issue_type, issue_id, status, summary, assignee, compone if issue_type == "jira": url = '/'.join((JIRA_API_BASE, 'browse', issue_id)) else: - url = f'https://github.com/{PR_REMOTE_NAME}/{PROJECT_NAME}/issues/{issue_id}' + url = ( + f'https://github.com/{ORG_NAME}/{PROJECT_NAME}/issues/{issue_id}' + ) return """=== {} {} === Summary\t\t{} @@ -314,7 +324,9 @@ def format_issue_output(issue_type, issue_id, status, summary, assignee, compone class GitHubAPI(object): def __init__(self, project_name, cmd): - self.github_api = f"https://api.github.com/repos/{PR_REMOTE_NAME}/{project_name}" + self.github_api = ( + f"https://api.github.com/repos/{ORG_NAME}/{project_name}" + ) token = None config = load_configuration() @@ -431,8 +443,10 @@ 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'] - JIRA_PR_TITLE_REGEXEN = [(project, re.compile(r'^(' + project + r'-[0-9]+)\b.*$')) - for project in JIRA_SUPPORTED_PROJECTS] + JIRA_PR_TITLE_REGEXEN = [ + (project, re.compile(r'^(' + project + r'-[0-9]+)\b.*$')) + for project in JIRA_SUPPORTED_PROJECTS + ] def __init__(self, cmd, github_api, git_remote, jira_con, number): self.cmd = cmd @@ -659,7 +673,7 @@ def cli(): # Location of your Arrow git clone ARROW_HOME = os.path.abspath(os.path.dirname(__file__)) print(f"ARROW_HOME = {ARROW_HOME}") - print(f"PR_REMOTE_NAME = {PR_REMOTE_NAME}") + print(f"ORG_NAME = {ORG_NAME}") print(f"PROJECT_NAME = {PROJECT_NAME}") cmd = CommandInput() @@ -671,7 +685,7 @@ def cli(): github_api = GitHubAPI(PROJECT_NAME, cmd) jira_con = connect_jira(cmd) - pr = PullRequest(cmd, github_api, PR_REMOTE_NAME, jira_con, pr_num) + pr = PullRequest(cmd, github_api, ORG_NAME, jira_con, pr_num) if pr.is_merged: print("Pull request %s has already been merged" % pr_num) @@ -695,7 +709,7 @@ def cli(): issue_comment = ( "Issue resolved by pull request %s\n%s" % (pr_num, - f"https://github.com/{PR_REMOTE_NAME}/{PROJECT_NAME}/pull/{pr_num}") + f"https://github.com/{ORG_NAME}/{PROJECT_NAME}/pull/{pr_num}") ) fix_version = prompt_for_fix_version(cmd, pr.issue, pr.maintenance_branches) From cbb75df6dbd33a74c175038089ea90ce1a075aa0 Mon Sep 17 00:00:00 2001 From: Alessandro Molina Date: Tue, 29 Nov 2022 17:52:14 +0100 Subject: [PATCH 11/15] fix tests --- dev/test_merge_arrow_pr.py | 67 ++++++++++++++++++++++---------------- 1 file changed, 39 insertions(+), 28 deletions(-) diff --git a/dev/test_merge_arrow_pr.py b/dev/test_merge_arrow_pr.py index 2367c50b6b697..4fdc910966445 100755 --- a/dev/test_merge_arrow_pr.py +++ b/dev/test_merge_arrow_pr.py @@ -77,8 +77,10 @@ def transition_issue(self, jira_id, transition_id, comment=None, 'fixVersions': fixVersions } - def get_candidate_fix_versions(self, maintenance_branches): - return SOURCE_VERSIONS, ['0.11.0'] + @property + def current_versions(self): + all_versions = self._project_versions or SOURCE_VERSIONS + return [v for v in all_versions if not v.raw.get("released")] + ['0.11.0'] def project_versions(self, project): return self._project_versions @@ -104,9 +106,10 @@ def test_jira_fix_versions(): transitions=TRANSITIONS) issue = merge_arrow_pr.JiraIssue(jira, 'ARROW-1234', 'ARROW', FakeCLI()) - all_versions, default_versions = issue.get_candidate_fix_versions() - assert all_versions == SOURCE_VERSIONS - assert default_versions == ['0.9.0'] + fix_version = merge_arrow_pr.get_candidate_fix_version( + issue.current_versions + ) + assert fix_version == '0.9.0' def test_jira_fix_versions_filters_maintenance(): @@ -115,11 +118,11 @@ def test_jira_fix_versions_filters_maintenance(): transitions=TRANSITIONS) issue = merge_arrow_pr.JiraIssue(jira, 'ARROW-1234', 'ARROW', FakeCLI()) - all_versions, default_versions = issue.get_candidate_fix_versions( + fix_version = merge_arrow_pr.get_candidate_fix_version( + issue.current_versions, maintenance_branches=maintenance_branches ) - assert all_versions == SOURCE_VERSIONS - assert default_versions == ['0.10.0'] + assert fix_version == '0.10.0' def test_jira_no_suggest_patch_release(): @@ -132,9 +135,10 @@ def test_jira_no_suggest_patch_release(): jira = FakeJIRA(project_versions=versions, transitions=TRANSITIONS) issue = merge_arrow_pr.JiraIssue(jira, 'ARROW-1234', 'ARROW', FakeCLI()) - all_versions, default_versions = issue.get_candidate_fix_versions() - assert all_versions == versions - assert default_versions == ['0.10.0'] + fix_version = merge_arrow_pr.get_candidate_fix_version( + issue.current_versions + ) + assert fix_version == '0.10.0' def test_jira_parquet_no_suggest_non_cpp(): @@ -153,9 +157,10 @@ def test_jira_parquet_no_suggest_non_cpp(): jira = FakeJIRA(project_versions=versions, transitions=TRANSITIONS) issue = merge_arrow_pr.JiraIssue(jira, 'PARQUET-1713', 'PARQUET', FakeCLI()) - all_versions, default_versions = issue.get_candidate_fix_versions() - assert all_versions == versions - assert default_versions == ['cpp-1.6.0'] + fix_version = merge_arrow_pr.get_candidate_fix_version( + issue.current_versions + ) + assert fix_version == 'cpp-1.6.0' def test_jira_invalid_issue(): @@ -174,16 +179,16 @@ def test_jira_resolve(): transitions=TRANSITIONS) my_comment = 'my comment' - fix_versions = [SOURCE_VERSIONS[1].raw] + fix_version = "0.10.0" issue = merge_arrow_pr.JiraIssue(jira, 'ARROW-1234', 'ARROW', FakeCLI()) - issue.resolve(fix_versions, my_comment) + issue.resolve(fix_version, my_comment) assert jira.captured_transition == { 'jira_id': 'ARROW-1234', 'transition_id': 1, 'comment': my_comment, - 'fixVersions': fix_versions + 'fixVersions': [{'name': '0.10.0', 'released': False}] } @@ -193,16 +198,16 @@ def test_jira_resolve_non_mainline(): transitions=TRANSITIONS) my_comment = 'my comment' - fix_versions = [SOURCE_VERSIONS[0].raw] + fix_version = "JS-0.4.0" issue = merge_arrow_pr.JiraIssue(jira, 'ARROW-1234', 'ARROW', FakeCLI()) - issue.resolve(fix_versions, my_comment) + issue.resolve(fix_version, my_comment) assert jira.captured_transition == { 'jira_id': 'ARROW-1234', 'transition_id': 1, 'comment': my_comment, - 'fixVersions': fix_versions + 'fixVersions': [{'name': 'JS-0.4.0', 'released': False}] } @@ -214,7 +219,7 @@ def test_jira_resolve_released_fix_version(): cmd = FakeCLI(responses=['0.7.0']) fix_versions_json = merge_arrow_pr.prompt_for_fix_version(cmd, jira) - assert fix_versions_json == [RAW_VERSION_JSON[-1]] + assert fix_versions_json == "0.7.0" def test_multiple_authors_bad_input(): @@ -279,11 +284,16 @@ def test_no_unset_point_release_fix_version(): for v in ['0.17.0', '0.15.1', '0.14.2']]) issue = FakeIssue(fields) - jira = FakeJIRA(issue=issue, project_versions=SOURCE_VERSIONS, - transitions=TRANSITIONS) + jira = FakeJIRA( + issue=issue, + project_versions=[ + FakeVersion(v, vdata) for v, vdata in versions_json.items() + ], + transitions=TRANSITIONS + ) issue = merge_arrow_pr.JiraIssue(jira, 'ARROW-1234', 'ARROW', FakeCLI()) - issue.resolve([versions_json['0.16.0']], "a comment") + issue.resolve('0.16.0', "a comment") assert jira.captured_transition == { 'jira_id': 'ARROW-1234', @@ -307,7 +317,7 @@ def test_jira_output_no_components(): # ARROW-5472 status = 'Interesting work' components = [] - output = merge_arrow_pr.format_jira_output( + output = merge_arrow_pr.format_issue_output("jira", 'ARROW-1234', 'Resolved', status, FakeAssignee('Foo Bar'), components) @@ -318,7 +328,7 @@ def test_jira_output_no_components(): Status\t\tResolved URL\t\thttps://issues.apache.org/jira/browse/ARROW-1234""" - output = merge_arrow_pr.format_jira_output( + output = merge_arrow_pr.format_issue_output("jira", 'ARROW-1234', 'Resolved', status, FakeAssignee('Foo Bar'), [FakeComponent('C++'), FakeComponent('Python')]) @@ -332,9 +342,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" From ee561a3b114a7ab6e67e505d68e1dd278684ebea Mon Sep 17 00:00:00 2001 From: Alessandro Molina Date: Tue, 29 Nov 2022 17:54:28 +0100 Subject: [PATCH 12/15] lint --- dev/test_merge_arrow_pr.py | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/dev/test_merge_arrow_pr.py b/dev/test_merge_arrow_pr.py index 4fdc910966445..39576876d55ea 100755 --- a/dev/test_merge_arrow_pr.py +++ b/dev/test_merge_arrow_pr.py @@ -80,7 +80,9 @@ def transition_issue(self, jira_id, transition_id, comment=None, @property def current_versions(self): all_versions = self._project_versions or SOURCE_VERSIONS - return [v for v in all_versions if not v.raw.get("released")] + ['0.11.0'] + return [ + v for v in all_versions if not v.raw.get("released") + ] + ['0.11.0'] def project_versions(self, project): return self._project_versions @@ -285,7 +287,7 @@ def test_no_unset_point_release_fix_version(): issue = FakeIssue(fields) jira = FakeJIRA( - issue=issue, + issue=issue, project_versions=[ FakeVersion(v, vdata) for v, vdata in versions_json.items() ], @@ -317,9 +319,10 @@ def test_jira_output_no_components(): # ARROW-5472 status = 'Interesting work' components = [] - output = merge_arrow_pr.format_issue_output("jira", - 'ARROW-1234', 'Resolved', status, FakeAssignee('Foo Bar'), - components) + output = merge_arrow_pr.format_issue_output( + "jira", 'ARROW-1234', 'Resolved', status, + FakeAssignee('Foo Bar'), components + ) assert output == """=== JIRA ARROW-1234 === Summary\t\tInteresting work @@ -328,9 +331,10 @@ def test_jira_output_no_components(): Status\t\tResolved URL\t\thttps://issues.apache.org/jira/browse/ARROW-1234""" - output = merge_arrow_pr.format_issue_output("jira", - 'ARROW-1234', 'Resolved', status, FakeAssignee('Foo Bar'), - [FakeComponent('C++'), FakeComponent('Python')]) + output = merge_arrow_pr.format_issue_output( + "jira", 'ARROW-1234', 'Resolved', status, FakeAssignee('Foo Bar'), + [FakeComponent('C++'), FakeComponent('Python')] + ) assert output == """=== JIRA ARROW-1234 === Summary\t\tInteresting work From 1da35b9f108822ec576446ee0be63f1e0b8316d2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ra=C3=BAl=20Cumplido?= Date: Thu, 1 Dec 2022 13:07:25 +0100 Subject: [PATCH 13/15] Review comments for merge_arrow_pr --- dev/merge_arrow_pr.py | 31 +++++++++++++++++-------------- 1 file changed, 17 insertions(+), 14 deletions(-) diff --git a/dev/merge_arrow_pr.py b/dev/merge_arrow_pr.py index 485471156a240..b92ddf85870b0 100755 --- a/dev/merge_arrow_pr.py +++ b/dev/merge_arrow_pr.py @@ -32,8 +32,7 @@ # 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 -# - ORG_NAME: the name of the remote to the Apache git repo -# (set to 'apache' by default) +# - ARROW_GITHUB_ORG: the GitHub organisation ('apache' by default) # - DEBUG: use for testing to avoid pushing to apache (0 by default) import configparser @@ -59,7 +58,7 @@ # Remote name which points to the GitHub site ORG_NAME = ( - os.environ.get("ORG_NAME") or + os.environ.get("ARROW_GITHUB_ORG") or os.environ.get("PR_REMOTE_NAME") or # backward compatibility "apache" ) @@ -156,7 +155,7 @@ def _filter_mainline_versions(self, versions): return [x for x in versions if mainline_regex.match(x.name)] - def resolve(self, fix_version, comment): + def resolve(self, fix_version, comment, *args): fields = self.issue.fields cur_status = fields.status.name @@ -206,7 +205,7 @@ def __init__(self, github_api, github_id, 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)) + self.cmd.fail("GitHub could not find %s\n%s" % (github_id, e)) def get_label(self, prefix): prefix = f"{prefix}:" @@ -236,7 +235,7 @@ def current_versions(self): return unreleased_versions - def resolve(self, fix_version, comment): + def resolve(self, fix_version, comment, pr_body): cur_status = self.issue["state"] if cur_status == "closed": @@ -248,7 +247,8 @@ def resolve(self, fix_version, comment): (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) + if f"Closes: #{self.github_id}" not in pr_body: + self.github_api.close_issue(self.github_id, comment) print("Successfully resolved %s!" % (self.github_id)) self.issue = self.github_api.get_issue_data(self.github_id) @@ -440,8 +440,8 @@ def continue_maybe(self, prompt): class PullRequest(object): + GITHUB_PR_TITLE_PATTERN = re.compile(r'^GH-([0-9]+)\b.*$') # We can merge both ARROW and PARQUET patches - GITHUB_PR_TITLE_REGEXEN = re.compile(r'^GH-([0-9]+)\b.*$') JIRA_SUPPORTED_PROJECTS = ['ARROW', 'PARQUET'] JIRA_PR_TITLE_REGEXEN = [ (project, re.compile(r'^(' + project + r'-[0-9]+)\b.*$')) @@ -495,7 +495,7 @@ def _get_issue(self): if self.title.startswith("MINOR:"): return None - m = self.GITHUB_PR_TITLE_REGEXEN.search(self.title) + m = self.GITHUB_PR_TITLE_PATTERN.search(self.title) if m: github_id = m.group(1) return GitHubIssue(self._github_api, github_id, self.cmd) @@ -506,10 +506,13 @@ def _get_issue(self): jira_id = m.group(1) 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 " - "{0}, but found {1}".format(options, self.title)) + options = ' or '.join( + '{0}-XXX'.format(project) + for project in self.JIRA_SUPPORTED_PROJECTS + ["GH"] + ) + self.cmd.fail("PR title should be prefixed by a GitHub ID or a " + "Jira ID, like: {0}, but found {1}".format( + options, self.title)) def merge(self): """ @@ -713,7 +716,7 @@ def cli(): ) fix_version = prompt_for_fix_version(cmd, pr.issue, pr.maintenance_branches) - pr.issue.resolve(fix_version, issue_comment) + pr.issue.resolve(fix_version, issue_comment, pr.body) if __name__ == '__main__': From 93575e01934084812e87fb1e4df7c0c6ba1e32f2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ra=C3=BAl=20Cumplido?= Date: Thu, 1 Dec 2022 13:26:29 +0100 Subject: [PATCH 14/15] Fix lint and change comment as we don't allow comma separated fix versions anymore --- dev/merge_arrow_pr.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/dev/merge_arrow_pr.py b/dev/merge_arrow_pr.py index b92ddf85870b0..e1713ef02ded1 100755 --- a/dev/merge_arrow_pr.py +++ b/dev/merge_arrow_pr.py @@ -512,7 +512,7 @@ def _get_issue(self): ) self.cmd.fail("PR title should be prefixed by a GitHub ID or a " "Jira ID, like: {0}, but found {1}".format( - options, self.title)) + options, self.title)) def merge(self): """ @@ -622,8 +622,7 @@ def prompt_for_fix_version(cmd, issue, maintenance_branches=()): maintenance_branches=maintenance_branches ) - issue_fix_version = cmd.prompt("Enter comma-separated " - "fix version(s) [%s]: " + issue_fix_version = cmd.prompt("Enter fix version(s) [%s]: " % default_fix_version) if issue_fix_version == "": issue_fix_version = default_fix_version From 6db7f3f0ba07924d65e2265fe387b7e4f3f3ee5b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ra=C3=BAl=20Cumplido?= Date: Thu, 1 Dec 2022 13:38:22 +0100 Subject: [PATCH 15/15] Minor removal of (s) for version --- dev/merge_arrow_pr.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dev/merge_arrow_pr.py b/dev/merge_arrow_pr.py index e1713ef02ded1..6824f6f436e93 100755 --- a/dev/merge_arrow_pr.py +++ b/dev/merge_arrow_pr.py @@ -622,7 +622,7 @@ def prompt_for_fix_version(cmd, issue, maintenance_branches=()): maintenance_branches=maintenance_branches ) - issue_fix_version = cmd.prompt("Enter fix version(s) [%s]: " + issue_fix_version = cmd.prompt("Enter fix version [%s]: " % default_fix_version) if issue_fix_version == "": issue_fix_version = default_fix_version