Resolve notifier pull on demand

A recent change to load the `repo_provider` and `pull` ahead of time regressed the happy path of test results when no test failures are present.
In that case, no notification has to be performed, so no `repo_provider` or `pull` is needed.

Additionally to fixing this regression, this also fixes the case when the `pull` is legitimately `None`, but the on-demand loading was trying to fetch it once again.

helpers/notifier.py

@@ -1,6 +1,7 @@
 import logging
 from dataclasses import dataclass
 from enum import Enum
+from typing import Literal
 
 from asgiref.sync import async_to_sync
 from shared.torngit.base import TorngitBaseAdapter
@@ -27,17 +28,23 @@ class NotifierResult(Enum):
 class BaseNotifier:
     commit: Commit
     commit_yaml: UserYaml | None
-    _pull: EnrichedPull | None = None
+    _pull: EnrichedPull | None | Literal[False] = False
     _repo_service: TorngitBaseAdapter | None = None
 
-    def get_pull(self):
+    def get_pull(self, do_log=True) -> EnrichedPull | None:
         repo_service = self.get_repo_service()
 
-        if self._pull is None:
+        if self._pull is False:
             self._pull = async_to_sync(
                 fetch_and_update_pull_request_information_from_commit
             )(repo_service, self.commit, self.commit_yaml)
 
+        if self._pull is None and do_log:
+            log.info(
+                "Not notifying since there is no pull request associated with this commit",
+                extra=dict(commitid=self.commit.commitid),
+            )
+
         return self._pull
 
     def get_repo_service(self) -> TorngitBaseAdapter:
@@ -46,7 +53,7 @@ def get_repo_service(self) -> TorngitBaseAdapter:
 
         return self._repo_service
 
-    def send_to_provider(self, pull, message):
+    def send_to_provider(self, pull: EnrichedPull, message: str) -> bool:
         repo_service = self.get_repo_service()
         assert repo_service
 
@@ -75,12 +82,6 @@ def build_message(self) -> str:
     def notify(self) -> NotifierResult:
         pull = self.get_pull()
         if pull is None:
-            log.info(
-                "Not notifying since there is no pull request associated with this commit",
-                extra=dict(
-                    commitid=self.commit.commitid,
-                ),
-            )
             return NotifierResult.NO_PULL
 
         message = self.build_message()

services/test_results.py

@@ -296,10 +296,6 @@ def build_message(self) -> str:
     def error_comment(self):
         pull = self.get_pull()
         if pull is None:
-            log.info(
-                "Not notifying since there is no pull request associated with this commit",
-                extra=dict(commitid=self.commit.commitid),
-            )
             return False, "no_pull"
 
         message = ":x: We are unable to process any of the uploaded JUnit XML files. Please ensure your files are in the right format."
@@ -313,10 +309,6 @@ def error_comment(self):
     def upgrade_comment(self):
         pull = self.get_pull()
         if pull is None:
-            log.info(
-                "Not notifying since there is no pull request associated with this commit",
-                extra=dict(commitid=self.commit.commitid),
-            )
             return False, "no_pull"
 
         db_pull = pull.database_pull

tasks/test_results_finisher.py

@@ -149,21 +149,14 @@ def process_impl_within_lock(
             db_session.add(totals)
             db_session.flush()
 
-        repo_service = get_repo_provider_service(repo)
-        pull = async_to_sync(fetch_and_update_pull_request_information_from_commit)(
-            repo_service, commit, commit_yaml
-        )
-
         if self.check_if_no_success(previous_result):
             # every processor errored, nothing to notify on
             queue_notify = False
 
             # if error is None this whole process should be a noop
             if totals.error is not None:
                 # make an attempt to make test results comment
-                notifier = TestResultsNotifier(
-                    commit, commit_yaml, _pull=pull, _repo_service=repo_service
-                )
+                notifier = TestResultsNotifier(commit, commit_yaml)
                 success, reason = notifier.error_comment()
 
                 # also make attempt to make coverage comment
@@ -237,6 +230,11 @@ def process_impl_within_lock(
                 "queue_notify": True,
             }
 
+        repo_service = get_repo_provider_service(repo)
+        pull = async_to_sync(fetch_and_update_pull_request_information_from_commit)(
+            repo_service, commit, commit_yaml
+        )
+
         if pull is not None:
             activate_seat_info = determine_seat_activation(pull)
 
@@ -283,22 +281,16 @@ def process_impl_within_lock(
             flaky_tests = self.get_flaky_tests(db_session, repoid, failures)
 
         failures = sorted(failures, key=lambda x: x.duration_seconds)[:3]
-
         payload = TestResultsNotificationPayload(
             failed_tests, passed_tests, skipped_tests, failures, flaky_tests
         )
         notifier = TestResultsNotifier(
             commit, commit_yaml, payload=payload, _pull=pull, _repo_service=repo_service
         )
         notifier_result = notifier.notify()
+        success = True if notifier_result is NotifierResult.COMMENT_POSTED else False
         TestResultsFlow.log(TestResultsFlow.TEST_RESULTS_NOTIFY)
 
-        match notifier_result:
-            case NotifierResult.COMMENT_POSTED:
-                success = True
-            case _:
-                success = False
-
         if len(flaky_tests):
             log.info(
                 "Detected failure on test that has been identified as flaky",