mirror of
https://github.com/pytorch/pytorch.git
synced 2025-10-20 21:14:14 +08:00
[GHF] Do not block reverts with internal changes (#115903)
As check is more often than not is unreliable, so better just post a warning and let the revert proceed. Fixes https://github.com/pytorch/test-infra/issues/4797 Pull Request resolved: https://github.com/pytorch/pytorch/pull/115903 Approved by: https://github.com/clee2000, https://github.com/atalman
This commit is contained in:
committed by
PyTorch MergeBot
parent
f54bb1ed56
commit
7ed2bc7c67
15
.github/scripts/test_trymerge.py
vendored
15
.github/scripts/test_trymerge.py
vendored
@ -32,7 +32,6 @@ from trymerge import (
|
|||||||
main as trymerge_main,
|
main as trymerge_main,
|
||||||
MandatoryChecksMissingError,
|
MandatoryChecksMissingError,
|
||||||
MergeRule,
|
MergeRule,
|
||||||
PostCommentError,
|
|
||||||
RE_GHSTACK_DESC,
|
RE_GHSTACK_DESC,
|
||||||
read_merge_rules,
|
read_merge_rules,
|
||||||
remove_job_name_suffix,
|
remove_job_name_suffix,
|
||||||
@ -470,20 +469,6 @@ class TestTryMerge(TestCase):
|
|||||||
|
|
||||||
self.assertEqual(len(changed_files), pr.get_changed_files_count())
|
self.assertEqual(len(changed_files), pr.get_changed_files_count())
|
||||||
|
|
||||||
def test_revert_codev_fails(self, *args: Any) -> None:
|
|
||||||
pr = GitHubPR("pytorch", "pytorch", 91340)
|
|
||||||
|
|
||||||
class GitRepoCoDev(DummyGitRepo):
|
|
||||||
def commit_message(self, ref: str) -> str:
|
|
||||||
return pr.get_body()
|
|
||||||
|
|
||||||
repo = GitRepoCoDev()
|
|
||||||
self.assertRaisesRegex(
|
|
||||||
PostCommentError,
|
|
||||||
"landed via phabricator",
|
|
||||||
lambda: validate_revert(repo, pr, comment_id=1372496233),
|
|
||||||
)
|
|
||||||
|
|
||||||
def test_revert_codev_abandoned_diff_succeeds(self, *args: Any) -> None:
|
def test_revert_codev_abandoned_diff_succeeds(self, *args: Any) -> None:
|
||||||
pr = GitHubPR("pytorch", "pytorch", 100652)
|
pr = GitHubPR("pytorch", "pytorch", 100652)
|
||||||
|
|
||||||
|
27
.github/scripts/trymerge.py
vendored
27
.github/scripts/trymerge.py
vendored
@ -1743,15 +1743,10 @@ def validate_revert(
|
|||||||
f"Will not revert as @{author_login} is not one of "
|
f"Will not revert as @{author_login} is not one of "
|
||||||
f"[{', '.join(allowed_reverters)}], but instead is {author_association}."
|
f"[{', '.join(allowed_reverters)}], but instead is {author_association}."
|
||||||
)
|
)
|
||||||
skip_internal_checks = can_skip_internal_checks(pr, comment_id)
|
|
||||||
|
|
||||||
# Ignore associated diff it PR does not have internal changes
|
|
||||||
if pr.has_no_connected_diff():
|
|
||||||
skip_internal_checks = True
|
|
||||||
|
|
||||||
# Raises exception if matching rule is not found, but ignores all status checks
|
# Raises exception if matching rule is not found, but ignores all status checks
|
||||||
find_matching_merge_rule(
|
find_matching_merge_rule(
|
||||||
pr, repo, skip_mandatory_checks=True, skip_internal_checks=skip_internal_checks
|
pr, repo, skip_mandatory_checks=True, skip_internal_checks=True
|
||||||
)
|
)
|
||||||
commit_sha = pr.get_merge_commit()
|
commit_sha = pr.get_merge_commit()
|
||||||
if commit_sha is None:
|
if commit_sha is None:
|
||||||
@ -1759,13 +1754,6 @@ def validate_revert(
|
|||||||
if len(commits) == 0:
|
if len(commits) == 0:
|
||||||
raise PostCommentError("Can't find any commits resolving PR")
|
raise PostCommentError("Can't find any commits resolving PR")
|
||||||
commit_sha = commits[0]
|
commit_sha = commits[0]
|
||||||
msg = repo.commit_message(commit_sha)
|
|
||||||
rc = RE_DIFF_REV.search(msg)
|
|
||||||
if rc is not None and not skip_internal_checks:
|
|
||||||
raise PostCommentError(
|
|
||||||
f"Can't revert PR that was landed via phabricator as {rc.group(1)}. "
|
|
||||||
+ "Please revert by going to the internal diff and clicking Unland."
|
|
||||||
)
|
|
||||||
return (author_login, commit_sha)
|
return (author_login, commit_sha)
|
||||||
|
|
||||||
|
|
||||||
@ -1784,6 +1772,7 @@ def try_revert(
|
|||||||
author_login, commit_sha = validate_revert(repo, pr, comment_id=comment_id)
|
author_login, commit_sha = validate_revert(repo, pr, comment_id=comment_id)
|
||||||
except PostCommentError as e:
|
except PostCommentError as e:
|
||||||
return post_comment(str(e))
|
return post_comment(str(e))
|
||||||
|
|
||||||
revert_msg = f"\nReverted {pr.get_pr_url()} on behalf of {prefix_with_github_url(author_login)}"
|
revert_msg = f"\nReverted {pr.get_pr_url()} on behalf of {prefix_with_github_url(author_login)}"
|
||||||
revert_msg += f" due to {reason}" if reason is not None else ""
|
revert_msg += f" due to {reason}" if reason is not None else ""
|
||||||
revert_msg += (
|
revert_msg += (
|
||||||
@ -1798,9 +1787,19 @@ def try_revert(
|
|||||||
msg += revert_msg
|
msg += revert_msg
|
||||||
repo.amend_commit_message(msg)
|
repo.amend_commit_message(msg)
|
||||||
repo.push(pr.default_branch(), dry_run)
|
repo.push(pr.default_branch(), dry_run)
|
||||||
post_comment(
|
|
||||||
|
revert_message = (
|
||||||
f"@{pr.get_pr_creator_login()} your PR has been successfully reverted."
|
f"@{pr.get_pr_creator_login()} your PR has been successfully reverted."
|
||||||
)
|
)
|
||||||
|
if (
|
||||||
|
pr.has_internal_changes()
|
||||||
|
and not pr.has_no_connected_diff()
|
||||||
|
and not can_skip_internal_checks(pr, comment_id)
|
||||||
|
):
|
||||||
|
revert_message += "\n:warning: This PR might contain internal changes"
|
||||||
|
revert_message += "\ncc: @pytorch/pytorch-dev-infra"
|
||||||
|
post_comment(revert_message)
|
||||||
|
|
||||||
if not dry_run:
|
if not dry_run:
|
||||||
pr.add_numbered_label("reverted")
|
pr.add_numbered_label("reverted")
|
||||||
gh_post_commit_comment(pr.org, pr.project, commit_sha, revert_msg)
|
gh_post_commit_comment(pr.org, pr.project, commit_sha, revert_msg)
|
||||||
|
Reference in New Issue
Block a user