Skip to content

Commit 396b6ac

Browse files
refactor: extract notification dispatch methods from run_impl_within_lock
Extract the bottom half of NotifyTask.run_impl_within_lock() into focused methods, completing the breakup to all methods under 50 lines. - _skip_notifications(): logs and returns skip result - _resolve_pull_and_base(): determines pull request and base commit - _prepare_and_send_notifications(): builds comparison context and dispatches Co-authored-by: Cursor <cursoragent@cursor.com>
1 parent 1552b25 commit 396b6ac

1 file changed

Lines changed: 162 additions & 123 deletions

File tree

apps/worker/tasks/notify.py

Lines changed: 162 additions & 123 deletions
Original file line numberDiff line numberDiff line change
@@ -269,132 +269,25 @@ def run_impl_within_lock(
269269
head_report = report_service.get_existing_report_for_commit(
270270
commit, report_class=ReadOnlyReport
271271
)
272-
if self.should_send_notifications(
272+
273+
if not self.should_send_notifications(
273274
current_yaml, commit, ci_results, head_report
274275
):
275-
enriched_pull = async_to_sync(
276-
fetch_and_update_pull_request_information_from_commit
277-
)(repository_service, commit, current_yaml)
278-
if enriched_pull and enriched_pull.database_pull:
279-
pull = enriched_pull.database_pull
280-
base_commit = self.fetch_pull_request_base(pull)
281-
else:
282-
pull = None
283-
base_commit = self.fetch_parent(commit)
284-
285-
if (
286-
enriched_pull
287-
and not self.send_notifications_if_commit_differs_from_pulls_head(
288-
commit, enriched_pull, current_yaml
289-
)
290-
and empty_upload is None
291-
):
292-
log.info(
293-
"Not sending notifications for commit when it differs from pull's most recent head",
294-
extra={
295-
"commit": commit.commitid,
296-
"repoid": commit.repoid,
297-
"current_yaml": current_yaml.to_dict(),
298-
"pull_head": enriched_pull.provider_pull["head"]["commitid"],
299-
},
300-
)
301-
self.log_checkpoint(UploadFlow.NOTIF_STALE_HEAD)
302-
self._call_upload_breadcrumb_task(
303-
commit_sha=commit.commitid,
304-
repo_id=commit.repoid,
305-
milestone=milestone,
306-
error=Errors.SKIPPED_NOTIFICATIONS,
307-
)
308-
return {
309-
"notified": False,
310-
"notifications": None,
311-
"reason": "User doesnt want notifications warning them that current head differs from pull request most recent head.",
312-
}
313-
314-
if base_commit is not None:
315-
base_report = report_service.get_existing_report_for_commit(
316-
base_commit, report_class=ReadOnlyReport
317-
)
318-
else:
319-
base_report = None
320-
if head_report is None and empty_upload is None:
321-
self.log_checkpoint(UploadFlow.NOTIF_ERROR_NO_REPORT)
322-
self._call_upload_breadcrumb_task(
323-
commit_sha=commit.commitid,
324-
repo_id=commit.repoid,
325-
milestone=milestone,
326-
error=Errors.REPORT_NOT_FOUND,
327-
)
328-
return {
329-
"notified": False,
330-
"notifications": None,
331-
"reason": "no_head_report",
332-
}
333-
334-
if commit.repository.service == "gitlab":
335-
gitlab_extra_shas_to_notify = self.get_gitlab_extra_shas_to_notify(
336-
commit, repository_service
337-
)
338-
else:
339-
gitlab_extra_shas_to_notify = None
276+
return self._skip_notifications(commit, milestone)
340277

341-
log.info(
342-
"We are going to be sending notifications",
343-
extra={
344-
"commit": commit.commitid,
345-
"repoid": commit.repoid,
346-
"current_yaml": current_yaml.to_dict(),
347-
},
348-
)
349-
350-
notifications = self.submit_third_party_notifications(
351-
current_yaml,
352-
base_commit,
353-
commit,
354-
base_report,
355-
head_report,
356-
enriched_pull,
357-
repository_service,
358-
empty_upload,
359-
all_tests_passed=all_tests_passed,
360-
test_results_error=ta_error_msg,
361-
installation_name_to_use=installation_name_to_use,
362-
gh_is_using_codecov_commenter=self.is_using_codecov_commenter(
363-
repository_service
364-
),
365-
gitlab_extra_shas_to_notify=gitlab_extra_shas_to_notify,
366-
)
367-
self.log_checkpoint(UploadFlow.NOTIFIED)
368-
self._call_upload_breadcrumb_task(
369-
commit_sha=commit.commitid,
370-
repo_id=commit.repoid,
371-
milestone=milestone,
372-
)
373-
log.info(
374-
"Notifications done",
375-
extra={
376-
"notifications": notifications,
377-
"notification_count": len(notifications),
378-
"commit": commit.commitid,
379-
"repoid": commit.repoid,
380-
"pullid": pull.pullid if pull is not None else None,
381-
},
382-
)
383-
db_session.commit()
384-
return {"notified": True, "notifications": notifications}
385-
else:
386-
log.info(
387-
"Not sending notifications at all",
388-
extra={"commit": commit.commitid, "repoid": commit.repoid},
389-
)
390-
self.log_checkpoint(UploadFlow.SKIPPING_NOTIFICATION)
391-
self._call_upload_breadcrumb_task(
392-
commit_sha=commit.commitid,
393-
repo_id=commit.repoid,
394-
milestone=milestone,
395-
error=Errors.SKIPPED_NOTIFICATIONS,
396-
)
397-
return {"notified": False, "notifications": None}
278+
return self._prepare_and_send_notifications(
279+
db_session=db_session,
280+
commit=commit,
281+
current_yaml=current_yaml,
282+
head_report=head_report,
283+
repository_service=repository_service,
284+
report_service=report_service,
285+
installation_name_to_use=installation_name_to_use,
286+
empty_upload=empty_upload,
287+
all_tests_passed=all_tests_passed,
288+
ta_error_msg=ta_error_msg,
289+
milestone=milestone,
290+
)
398291

399292
def _fetch_commit(self, db_session: Session, repoid: int, commitid: str) -> Commit:
400293
"""Fetch and validate the commit from the database."""
@@ -581,6 +474,152 @@ def _handle_ci_wait_retry(
581474
**kwargs,
582475
)
583476

477+
def _skip_notifications(self, commit, milestone):
478+
"""Log and return when we decide not to send any notifications."""
479+
log.info(
480+
"Not sending notifications at all",
481+
extra={"commit": commit.commitid, "repoid": commit.repoid},
482+
)
483+
self.log_checkpoint(UploadFlow.SKIPPING_NOTIFICATION)
484+
self._call_upload_breadcrumb_task(
485+
commit_sha=commit.commitid,
486+
repo_id=commit.repoid,
487+
milestone=milestone,
488+
error=Errors.SKIPPED_NOTIFICATIONS,
489+
)
490+
return {"notified": False, "notifications": None}
491+
492+
def _resolve_pull_and_base(self, commit, enriched_pull):
493+
"""Determine the pull request and base commit for comparison."""
494+
if enriched_pull and enriched_pull.database_pull:
495+
pull = enriched_pull.database_pull
496+
base_commit = self.fetch_pull_request_base(pull)
497+
else:
498+
pull = None
499+
base_commit = self.fetch_parent(commit)
500+
return pull, base_commit
501+
502+
def _prepare_and_send_notifications(
503+
self,
504+
db_session,
505+
commit,
506+
current_yaml,
507+
head_report,
508+
repository_service,
509+
report_service,
510+
installation_name_to_use,
511+
empty_upload,
512+
all_tests_passed,
513+
ta_error_msg,
514+
milestone,
515+
):
516+
"""Prepare comparison context and dispatch all notifications."""
517+
enriched_pull = async_to_sync(
518+
fetch_and_update_pull_request_information_from_commit
519+
)(repository_service, commit, current_yaml)
520+
pull, base_commit = self._resolve_pull_and_base(commit, enriched_pull)
521+
522+
if (
523+
enriched_pull
524+
and not self.send_notifications_if_commit_differs_from_pulls_head(
525+
commit, enriched_pull, current_yaml
526+
)
527+
and empty_upload is None
528+
):
529+
log.info(
530+
"Not sending notifications for commit when it differs from pull's most recent head",
531+
extra={
532+
"commit": commit.commitid,
533+
"repoid": commit.repoid,
534+
"current_yaml": current_yaml.to_dict(),
535+
"pull_head": enriched_pull.provider_pull["head"]["commitid"],
536+
},
537+
)
538+
self.log_checkpoint(UploadFlow.NOTIF_STALE_HEAD)
539+
self._call_upload_breadcrumb_task(
540+
commit_sha=commit.commitid,
541+
repo_id=commit.repoid,
542+
milestone=milestone,
543+
error=Errors.SKIPPED_NOTIFICATIONS,
544+
)
545+
return {
546+
"notified": False,
547+
"notifications": None,
548+
"reason": "User doesnt want notifications warning them that current head differs from pull request most recent head.",
549+
}
550+
551+
base_report = (
552+
report_service.get_existing_report_for_commit(
553+
base_commit, report_class=ReadOnlyReport
554+
)
555+
if base_commit is not None
556+
else None
557+
)
558+
if head_report is None and empty_upload is None:
559+
self.log_checkpoint(UploadFlow.NOTIF_ERROR_NO_REPORT)
560+
self._call_upload_breadcrumb_task(
561+
commit_sha=commit.commitid,
562+
repo_id=commit.repoid,
563+
milestone=milestone,
564+
error=Errors.REPORT_NOT_FOUND,
565+
)
566+
return {
567+
"notified": False,
568+
"notifications": None,
569+
"reason": "no_head_report",
570+
}
571+
572+
gitlab_extra_shas_to_notify = (
573+
self.get_gitlab_extra_shas_to_notify(commit, repository_service)
574+
if commit.repository.service == "gitlab"
575+
else None
576+
)
577+
578+
log.info(
579+
"We are going to be sending notifications",
580+
extra={
581+
"commit": commit.commitid,
582+
"repoid": commit.repoid,
583+
"current_yaml": current_yaml.to_dict(),
584+
},
585+
)
586+
587+
notifications = self.submit_third_party_notifications(
588+
current_yaml,
589+
base_commit,
590+
commit,
591+
base_report,
592+
head_report,
593+
enriched_pull,
594+
repository_service,
595+
empty_upload,
596+
all_tests_passed=all_tests_passed,
597+
test_results_error=ta_error_msg,
598+
installation_name_to_use=installation_name_to_use,
599+
gh_is_using_codecov_commenter=self.is_using_codecov_commenter(
600+
repository_service
601+
),
602+
gitlab_extra_shas_to_notify=gitlab_extra_shas_to_notify,
603+
)
604+
self.log_checkpoint(UploadFlow.NOTIFIED)
605+
self._call_upload_breadcrumb_task(
606+
commit_sha=commit.commitid,
607+
repo_id=commit.repoid,
608+
milestone=milestone,
609+
)
610+
log.info(
611+
"Notifications done",
612+
extra={
613+
"notifications": notifications,
614+
"notification_count": len(notifications),
615+
"commit": commit.commitid,
616+
"repoid": commit.repoid,
617+
"pullid": pull.pullid if pull is not None else None,
618+
},
619+
)
620+
db_session.commit()
621+
return {"notified": True, "notifications": notifications}
622+
584623
def is_using_codecov_commenter(
585624
self, repository_service: TorngitBaseAdapter
586625
) -> bool:

0 commit comments

Comments
 (0)