Ensure the comment id is always passed in to trymerge (#161558)

Pull Request resolved: https://github.com/pytorch/pytorch/pull/161558
Approved by: https://github.com/seemethere, https://github.com/malfet
This commit is contained in:
Zain Rizvi 2025-08-27 13:18:44 -05:00 committed by PyTorch MergeBot
parent 06c7516994
commit 624bc36163
3 changed files with 39 additions and 36 deletions

View File

@ -124,7 +124,7 @@ def mock_parse_args(revert: bool = False, force: bool = False) -> Any:
self.force = force
self.pr_num = 76123
self.dry_run = True
self.comment_id = 0
self.comment_id = 12345 # Set to non-zero value
self.reason = "this is for testing"
self.ignore_current = False
self.check_mergeability = False
@ -152,9 +152,9 @@ def mock_revert(
def mock_merge(
pr: GitHubPR,
repo: GitRepo,
comment_id: int,
dry_run: bool = False,
skip_mandatory_checks: bool = False,
comment_id: Optional[int] = None,
timeout_minutes: int = 400,
stale_pr_days: int = 3,
ignore_current: bool = False,
@ -470,9 +470,9 @@ class TestTryMerge(TestCase):
mock_merge.assert_called_once_with(
mock.ANY,
mock.ANY,
comment_id=mock.ANY,
dry_run=mock.ANY,
skip_mandatory_checks=True,
comment_id=mock.ANY,
ignore_current=False,
)
@ -485,9 +485,9 @@ class TestTryMerge(TestCase):
mock_merge.assert_called_once_with(
mock.ANY,
mock.ANY,
comment_id=mock.ANY,
dry_run=mock.ANY,
skip_mandatory_checks=False,
comment_id=mock.ANY,
ignore_current=False,
)

View File

@ -1151,7 +1151,7 @@ class GitHubPR:
*,
skip_mandatory_checks: bool = False,
dry_run: bool = False,
comment_id: Optional[int] = None,
comment_id: int,
ignore_current_checks: Optional[list[str]] = None,
) -> None:
# Raises exception if matching rule is not found
@ -1231,22 +1231,7 @@ class GitHubPR:
branch_to_merge_into = self.default_branch() if branch is None else branch
if repo.current_branch() != branch_to_merge_into:
repo.checkout(branch_to_merge_into)
if not self.is_ghstack_pr():
msg = self.gen_commit_message()
pr_branch_name = f"__pull-request-{self.pr_num}__init__"
repo.fetch(self.last_commit()["oid"], pr_branch_name)
repo._run_git("merge", "--squash", pr_branch_name)
repo._run_git("commit", f'--author="{self.get_author()}"', "-m", msg)
# Did the PR change since we started the merge?
pulled_sha = repo.show_ref(pr_branch_name)
latest_pr_status = GitHubPR(self.org, self.project, self.pr_num)
if pulled_sha != latest_pr_status.last_commit()["oid"]:
raise RuntimeError(
"PR has been updated since CI checks last passed. Please rerun the merge command."
)
return []
else:
if self.is_ghstack_pr():
return self.merge_ghstack_into(
repo,
skip_mandatory_checks,
@ -1254,6 +1239,21 @@ class GitHubPR:
skip_all_rule_checks=skip_all_rule_checks,
)
msg = self.gen_commit_message()
pr_branch_name = f"__pull-request-{self.pr_num}__init__"
repo.fetch(self.last_commit()["oid"], pr_branch_name)
repo._run_git("merge", "--squash", pr_branch_name)
repo._run_git("commit", f'--author="{self.get_author()}"', "-m", msg)
# Did the PR change since we started the merge?
pulled_sha = repo.show_ref(pr_branch_name)
latest_pr_status = GitHubPR(self.org, self.project, self.pr_num)
if pulled_sha != latest_pr_status.last_commit()["oid"]:
raise RuntimeError(
"PR has been updated since CI checks last passed. Please rerun the merge command."
)
return []
class MergeRuleFailedError(RuntimeError):
def __init__(self, message: str, rule: Optional["MergeRule"] = None) -> None:
@ -2156,9 +2156,9 @@ def categorize_checks(
def merge(
pr: GitHubPR,
repo: GitRepo,
comment_id: int,
dry_run: bool = False,
skip_mandatory_checks: bool = False,
comment_id: Optional[int] = None,
timeout_minutes: int = 400,
stale_pr_days: int = 3,
ignore_current: bool = False,
@ -2416,12 +2416,18 @@ def main() -> None:
gh_post_pr_comment(org, project, args.pr_num, message, dry_run=args.dry_run)
return
try:
# Ensure comment id is set, else fail
if not args.comment_id:
raise ValueError(
"Comment ID is required for merging PRs, please provide it using --comment-id"
)
merge(
pr,
repo,
comment_id=args.comment_id,
dry_run=args.dry_run,
skip_mandatory_checks=args.force,
comment_id=args.comment_id,
ignore_current=args.ignore_current,
)
except Exception as e:

View File

@ -59,22 +59,19 @@ jobs:
# on the PR appear in chronological order (timing issues can shuffle them around)
sleep 60
fi
# Require a comment id for merge operations
if [ -z "${COMMENT_ID}" ]; then
echo "Error: merge requires COMMENT_ID to be specified"
exit 1
fi
if [ -n "${FORCE}" ]; then
if [ -n "${COMMENT_ID}" ]; then
python3 .github/scripts/trymerge.py --force --comment-id "${COMMENT_ID}" "${PR_NUM}"
else
python3 .github/scripts/trymerge.py --force "${PR_NUM}"
fi
python3 .github/scripts/trymerge.py --force --comment-id "${COMMENT_ID}" "${PR_NUM}"
elif [ -n "${IGNORE_CURRENT}" ]; then
if [ -n "${COMMENT_ID}" ]; then
python3 .github/scripts/trymerge.py --ignore-current --comment-id "${COMMENT_ID}" "${PR_NUM}"
else
python3 .github/scripts/trymerge.py --ignore-current "${PR_NUM}"
fi
elif [ -n "${COMMENT_ID}" ]; then
python3 .github/scripts/trymerge.py --comment-id "${COMMENT_ID}" "${PR_NUM}"
python3 .github/scripts/trymerge.py --ignore-current --comment-id "${COMMENT_ID}" "${PR_NUM}"
else
python3 .github/scripts/trymerge.py "${PR_NUM}"
python3 .github/scripts/trymerge.py --comment-id "${COMMENT_ID}" "${PR_NUM}"
fi
- name: Comment on Canceled
if: ${{ cancelled() && steps.checkout.outcome == 'success' }}