SteNicholas commented on code in PR #3823:
URL: https://github.com/apache/celeborn/pull/3823#discussion_r3844658265
##########
dev/merge_pr.py:
##########
@@ -634,11 +691,27 @@ def main():
merge_hash = merge_pr(pr_num, target_ref, title, body, pr_repo_desc)
+ # Ordered (branch, commit hash) pairs for the merge comment: the merge
sink first,
+ # then each cherry-pick target as it is picked.
+ merged_commits = [(target_ref, merge_hash)]
+
pick_prompt = "Would you like to pick %s into another branch?" % merge_hash
- while input("\n%s (y/n): " % pick_prompt).lower() == "y":
- merged_refs = merged_refs + [
- cherry_pick(pr_num, merge_hash, next(branch_iter, branch_names[0]))
- ]
+ # Post the summary in a finally block: the merge into the target branch
has already
+ # been pushed, so aborting a later cherry-pick must not drop that line.
+ try:
+ while input("\n%s (y/N): " % pick_prompt).lower() == "y":
+ picked = cherry_pick(pr_num, merge_hash, next(branch_iter,
branch_names[0]))
+ merged_refs = merged_refs + [picked[0]]
+ merged_commits = merged_commits + [picked]
+ finally:
+ # The "Closes #N" string in the commit message only auto-closes the PR
when the
+ # commit lands on the default branch. For merges into other branches
(e.g.
+ # branch-X.Y backport PRs), GitHub leaves the PR open, so close it
through the API.
+ pr_state = get_json("%s/pulls/%s" % (GITHUB_API_BASE,
pr_num)).get("state")
+ if pr_state != "closed":
+ print("\nPR #%s is still open after push; closing it
explicitly.\n" % pr_num)
+ close_pr(pr_num)
Review Comment:
This PATCH creates a regular close event whose `commit_id` is null, while
lines 658-660 recognize a previous merge only from close events with a non-null
`commit_id`. On a later invocation for another backport, the script therefore
misses the already-merged path and attempts a fresh merge. This is observable
on #3697: commit `0bf9c5a58` landed on `branch-0.6`, but its close event has
`commit_id: null`. Please adapt merge detection to the pushed hash/state, or
record and consume another durable marker, before automatically closing.
##########
dev/merge_pr.py:
##########
@@ -90,6 +91,59 @@ def get_json(url):
sys.exit(-1)
+def close_pr(pr_num):
+ url = "%s/pulls/%s" % (GITHUB_API_BASE, pr_num)
+ data = json.dumps({"state": "closed"}).encode("utf-8")
+ request = Request(url, data=data, method="PATCH")
+ request.add_header("Content-Type", "application/json")
+ request.add_header("Accept", "application/vnd.github+json")
+ if GITHUB_OAUTH_KEY:
Review Comment:
`GITHUB_OAUTH_KEY` is still documented as optional, but when it is unset
this sends an unauthenticated PATCH. GitHub requires write permission for
updating PR state, so the call fails, the `None` result is ignored, and
maintenance-branch PRs remain open. Please either require and validate a
write-capable token before the push, or explicitly skip this feature and update
the configuration documentation.
https://docs.github.com/en/rest/pulls/pulls#update-a-pull-request
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
