Add pull request ID to PushPagureEvent (#2435)

Add pull request ID to `PushPagureEvent`

Fixes #2359.

Reviewed-by: Laura Barcziová

packit_service/worker/events/pagure.py

@@ -60,8 +60,9 @@ def __init__(
         project_url: str,
         commit_sha: str,
         committer: str,
+        pr_id: Optional[int],
     ):
-        super().__init__(project_url=project_url)
+        super().__init__(project_url=project_url, pr_id=pr_id)
         self.repo_namespace = repo_namespace
         self.repo_name = repo_name
         self.git_ref = git_ref

packit_service/worker/mixin.py

@@ -16,7 +16,7 @@
 from packit.local_project import LocalProject
 from packit.utils.repo import RepositoryCache
 
-from ogr.abstract import GitProject, PullRequest, PRStatus
+from ogr.abstract import GitProject, PullRequest
 
 from packit_service.config import ServiceConfig
 from packit_service.worker.reporting import BaseCommitStatus
@@ -250,19 +250,12 @@ class GetPagurePullRequestMixin(GetPagurePullRequest):
     @property
     def pull_request(self):
         if not self._pull_request:
-            logger.debug(
-                f"Getting pull request with head commit {self.data.commit_sha}"
-                f"for repo {self.project.namespace}/{self.project.repo}"
-            )
-            # We are interested just in merge commits.
-            commit_merge_prs = [
-                pr
-                for pr in self.project.get_pr_list(status=PRStatus.all)
-                if pr.head_commit == self.data.commit_sha
-                and pr.target_branch == self.data.git_ref
-            ]
-            if commit_merge_prs:
-                self._pull_request = commit_merge_prs[0]
+            if self.data.pr_id:
+                logger.debug(
+                    f"Getting pull request #{self.data.pr_id}"
+                    f"for repo {self.project.namespace}/{self.project.repo}"
+                )
+                self._pull_request = self.project.get_pr(self.data.pr_id)
         return self._pull_request
 
     def get_pr_author(self):

packit_service/worker/parser.py

@@ -1111,13 +1111,16 @@ def parse_pagure_push_event(event) -> Optional[PushPagureEvent]:
         dg_base_url = getenv("DISTGIT_URL", DISTGIT_INSTANCES["fedpkg"].url)
         dg_project_url = f"{dg_base_url}{dg_repo_namespace}/{dg_repo_name}"
 
+        dg_pr_id = nested_get(event, "pull_request_id")
+
         return PushPagureEvent(
             repo_namespace=dg_repo_namespace,
             repo_name=dg_repo_name,
             git_ref=dg_branch,
             project_url=dg_project_url,
             commit_sha=dg_commit,
             committer=username,
+            pr_id=dg_pr_id,
         )
 
     @staticmethod

tests/data/fedmsg/distgit_push_packit.json

@@ -1,6 +1,7 @@
 {
   "agent": "pagure",
   "branch": "f36",
+  "pull_request_id": 5,
   "end_commit": "ad0c308af91da45cf40b253cd82f07f63ea9cbbf",
   "total_commits": 1,
   "repo": {

tests/integration/test_dg_commit.py

@@ -318,16 +318,7 @@ def test_downstream_koji_build_failure_no_issue():
         get_web_url=lambda: "https://src.fedoraproject.org/rpms/buildah",
         default_branch="main",
     )
-    flexmock(PagureProject).should_receive("get_pr_list").and_return(
-        [
-            flexmock(
-                id=5,
-                author="author",
-                head_commit="not-the-same-commit-hash",
-                status=PRStatus.open,
-            )
-        ]
-    )
+    flexmock(PagureProject).should_receive("get_pr").never()
     pagure_project_mock.should_receive("get_files").with_args(
         ref="main", filter_regex=r".+\.spec$"
     ).and_return(["buildah.spec"])
@@ -418,16 +409,7 @@ def test_downstream_koji_build_failure_issue_created():
         get_web_url=lambda: "https://src.fedoraproject.org/rpms/buildah",
         default_branch="main",
     )
-    flexmock(PagureProject).should_receive("get_pr_list").and_return(
-        [
-            flexmock(
-                id=5,
-                author="author",
-                head_commit="not-the-same-commit-hash",
-                status=PRStatus.open,
-            )
-        ]
-    )
+    flexmock(PagureProject).should_receive("get_pr").never()
     pagure_project_mock.should_receive("get_files").with_args(
         ref="main", filter_regex=r".+\.spec$"
     ).and_return(["buildah.spec"])
@@ -524,16 +506,7 @@ def test_downstream_koji_build_failure_issue_comment():
         get_web_url=lambda: "https://src.fedoraproject.org/rpms/buildah",
         default_branch="main",
     )
-    flexmock(PagureProject).should_receive("get_pr_list").and_return(
-        [
-            flexmock(
-                id=5,
-                author="author",
-                head_commit="not-the-same-commit-hash",
-                status=PRStatus.open,
-            )
-        ]
-    )
+    flexmock(PagureProject).should_receive("get_pr").never()
     pagure_project_mock.should_receive("get_files").with_args(
         ref="main", filter_regex=r".+\.spec$"
     ).and_return(["buildah.spec"])
@@ -633,16 +606,7 @@ def test_downstream_koji_build_no_config():
         get_web_url=lambda: "https://src.fedoraproject.org/rpms/buildah",
         default_branch="main",
     )
-    flexmock(PagureProject).should_receive("get_pr_list").and_return(
-        [
-            flexmock(
-                id=5,
-                author="author",
-                head_commit="not-the-same-commit-hash",
-                status=PRStatus.open,
-            )
-        ]
-    )
+    flexmock(PagureProject).should_receive("get_pr").never()
     pagure_project.should_receive("get_files").with_args(
         ref="main", filter_regex=r".+\.spec$"
     ).and_return(["buildah.spec"])
@@ -715,16 +679,7 @@ def test_downstream_koji_build_where_multiple_branches_defined(jobs_config):
         f"'jobs': {jobs_config},"
         "'downstream_package_name': 'buildah'}"
     )
-    flexmock(PagureProject).should_receive("get_pr_list").and_return(
-        [
-            flexmock(
-                id=5,
-                author="author",
-                head_commit="not-the-same-commit-hash",
-                status=PRStatus.open,
-            )
-        ]
-    )
+    flexmock(PagureProject).should_receive("get_pr").never()
     pagure_project = flexmock(
         PagureProject,
         full_repo_name="rpms/buildah",
@@ -902,16 +857,8 @@ def test_precheck_koji_build_push(
     distgit_push_event, push_username, allowed_committers, should_pass
 ):
     distgit_push_event.committer = push_username
-    flexmock(PagureProject).should_receive("get_pr_list").and_return(
-        [
-            flexmock(
-                id=5,
-                author="author",
-                head_commit="not-the-same-commit-hash",
-                status=PRStatus.open,
-            )
-        ]
-    )
+    distgit_push_event = flexmock(distgit_push_event, _pr_id=None)
+    flexmock(PagureProject).should_receive("get_pr").never()
 
     flexmock(GitProjectModel).should_receive("get_or_create").with_args(
         namespace="rpms",
@@ -1017,16 +964,14 @@ def test_precheck_koji_build_push_pr(
             },
         ),
     ]
-    flexmock(PagureProject).should_receive("get_pr_list").and_return(
-        [
-            flexmock(
-                id=5,
-                author=pr_author,
-                head_commit="ad0c308af91da45cf40b253cd82f07f63ea9cbbf",
-                status=PRStatus.open,
-                target_branch="f36",
-            )
-        ]
+    flexmock(PagureProject).should_receive("get_pr").and_return(
+        flexmock(
+            id=5,
+            author=pr_author,
+            head_commit="ad0c308af91da45cf40b253cd82f07f63ea9cbbf",
+            status=PRStatus.open,
+            target_branch="f36",
+        )
     )
     flexmock(PagureProject).should_receive("get_pr_files_diff").with_args(
         5, retries=int, wait_seconds=int

tests/unit/test_checkers.py

@@ -1025,16 +1025,14 @@ def test_labels_on_distgit_pr(
     )
     job_config = jobs[0]
 
-    flexmock(PagureProject).should_receive("get_pr_list").and_return(
-        [
-            flexmock(
-                id=5,
-                head_commit="ad0c308af91da45cf40b253cd82f07f63ea9cbbf",
-                status=PRStatus.open,
-                labels=pr_labels,
-                target_branch="f36",
-            )
-        ]
+    flexmock(PagureProject).should_receive("get_pr").and_return(
+        flexmock(
+            id=5,
+            head_commit="ad0c308af91da45cf40b253cd82f07f63ea9cbbf",
+            status=PRStatus.open,
+            labels=pr_labels,
+            target_branch="f36",
+        )
     )
 
     checker = LabelsOnDistgitPR(