[BE][GHF] Do not call GraphQL twice (#100434)

malfet · pytorchmergebot · commit 7b684310c8fa · 2023-05-02T00:26:49.000Z
During regular merge process, `GitHubPR` and `GitHubRepo` objects are first created in main() and than re-created in `merge()` instead of being passed by reference, which results in making the same GraphQL requests to the repo twice  ### <samp>🤖 Generated by Copilot at ee4e23e</samp> > _Sing, O Muse, of the skillful coder who refactored_ > _The `merge` function, to accept a `GitHubPR` object,_ > _And thus reduced the calls to the divine API_ > _And the duplication of code, that source of errors._ Pull Request resolved: #100434 Approved by: https://github.com/kit1980, https://github.com/PaliC, https://github.com/huydhn, https://github.com/ZainRizvi
diff --git a/.github/scripts/test_trymerge.py b/.github/scripts/test_trymerge.py
@@ -136,7 +136,7 @@ def mock_revert(
 
 
 def mock_merge(
-    pr_num: int,
+    pr: GitHubPR,
     repo: GitRepo,
     dry_run: bool = False,
     skip_mandatory_checks: bool = False,
diff --git a/.github/scripts/trymerge.py b/.github/scripts/trymerge.py
@@ -1756,7 +1756,7 @@ def categorize_checks(
 
 
 def merge(
-    pr_num: int,
+    pr: GitHubPR,
     repo: GitRepo,
     dry_run: bool = False,
     skip_mandatory_checks: bool = False,
@@ -1765,17 +1765,19 @@ def merge(
     stale_pr_days: int = 3,
     ignore_current: bool = False,
 ) -> None:
-    repo = GitRepo(get_git_repo_dir(), get_git_remote_name())
-    org, project = repo.gh_owner_and_name()
-    pr = GitHubPR(org, project, pr_num)
     initial_commit_sha = pr.last_commit()["oid"]
     print(f"Attempting merge of {initial_commit_sha}")
 
     if MERGE_IN_PROGRESS_LABEL not in pr.get_labels():
-        gh_add_labels(org, project, pr_num, [MERGE_IN_PROGRESS_LABEL])
+        gh_add_labels(pr.org, pr.project, pr.pr_num, [MERGE_IN_PROGRESS_LABEL])
 
     explainer = TryMergeExplainer(
-        skip_mandatory_checks, pr.get_labels(), pr.pr_num, org, project, ignore_current
+        skip_mandatory_checks,
+        pr.get_labels(),
+        pr.pr_num,
+        pr.org,
+        pr.project,
+        ignore_current,
     )
 
     # probably a bad name, but this is a list of current checks that should be
@@ -1785,12 +1787,16 @@ def merge(
     if pr.is_ghstack_pr():
         get_ghstack_prs(repo, pr)  # raises error if out of sync
 
-    check_for_sev(org, project, skip_mandatory_checks)
+    check_for_sev(pr.org, pr.project, skip_mandatory_checks)
 
     if skip_mandatory_checks or can_skip_internal_checks(pr, comment_id):
         # do not wait for any pending signals if PR is closed as part of co-development process
         gh_post_pr_comment(
-            org, project, pr.pr_num, explainer.get_merge_message(), dry_run=dry_run
+            pr.org,
+            pr.project,
+            pr.pr_num,
+            explainer.get_merge_message(),
+            dry_run=dry_run,
         )
         return pr.merge_into(
             repo,
@@ -1811,8 +1817,8 @@ def merge(
         ignore_current_checks_info = failing
 
     gh_post_pr_comment(
-        org,
-        project,
+        pr.org,
+        pr.project,
         pr.pr_num,
         explainer.get_merge_message(ignore_current_checks_info),
         dry_run=dry_run,
@@ -1838,13 +1844,13 @@ def merge(
         x[0] for x in ignore_current_checks_info
     ]  # convert to List[str] for convenience
     while elapsed_time < timeout_minutes * 60:
-        check_for_sev(org, project, skip_mandatory_checks)
+        check_for_sev(pr.org, pr.project, skip_mandatory_checks)
         current_time = time.time()
         elapsed_time = current_time - start_time
         print(
-            f"Attempting merge of https://github.com/{org}/{project}/pull/{pr_num} ({elapsed_time / 60} minutes elapsed)"
+            f"Attempting merge of https://github.com/{pr.org}/{pr.project}/pull/{pr.pr_num} ({elapsed_time / 60} minutes elapsed)"
         )
-        pr = GitHubPR(org, project, pr_num)
+        pr = GitHubPR(pr.org, pr.project, pr.pr_num)
         if initial_commit_sha != pr.last_commit()["oid"]:
             raise RuntimeError(
                 "New commits were pushed while merging. Please rerun the merge command."
@@ -1913,14 +1919,14 @@ def merge(
         except MandatoryChecksMissingError as ex:
             last_exception = str(ex)
             print(
-                f"Merge of https://github.com/{org}/{project}/pull/{pr_num} failed due to: {ex}. Retrying in 5 min"
+                f"Merge of https://github.com/{pr.org}/{pr.project}/pull/{pr.pr_num} failed due to: {ex}. Retrying in 5 min"
             )
             time.sleep(5 * 60)
     # Finally report timeout back
     msg = f"Merged timed out after {timeout_minutes} minutes. Please contact the pytorch_dev_infra team."
     msg += f"The last exception was: {last_exception}"
     if not dry_run:
-        gh_add_labels(org, project, pr_num, ["land-failed"])
+        gh_add_labels(pr.org, pr.project, pr.pr_num, ["land-failed"])
     raise RuntimeError(msg)
 
 
@@ -2008,7 +2014,7 @@ def handle_exception(e: Exception, title: str = "Merge failed") -> None:
         return
     try:
         merge(
-            args.pr_num,
+            pr,
             repo,
             dry_run=args.dry_run,
             skip_mandatory_checks=args.force,
