Skip to content

Commit 1552b25

Browse files
thomasrockhu-codecovcursoragent
thomasrockhu-codecov
and
cursoragent
committed
refactor: extract repo service and CI methods from run_impl_within_lock
Extract the top half of NotifyTask.run_impl_within_lock() into focused methods, reducing the method from 322 lines to ~190 lines. - _fetch_commit(): fetches and validates commit from DB - _get_repository_service_for_notify(): gets repo service with error handling - _handle_no_configured_apps(): handles rate-limited/suspended GitHub apps - _fetch_ci_results_or_error(): fetches CI results with error handling - _handle_ci_wait_retry(): retry logic when CI hasn't finished Co-authored-by: Cursor <cursoragent@cursor.com>
1 parent 304d215 commit 1552b25

1 file changed

Lines changed: 203 additions & 153 deletions

File tree

‎apps/worker/tasks/notify.py‎

Lines changed: 203 additions & 153 deletions
Original file line numberDiff line numberDiff line change
@@ -220,22 +220,17 @@ def run_impl_within_lock(
220220
milestone = Milestones.NOTIFICATIONS_SENT
221221

222222
log.info("Starting notifications", extra={"commit": commitid, "repoid": repoid})
223-
commits_query = db_session.query(Commit).filter(
224-
Commit.repoid == repoid, Commit.commitid == commitid
225-
)
226-
commit: Commit = commits_query.first()
227-
assert commit, "Commit not found in database."
228-
223+
commit = self._fetch_commit(db_session, repoid, commitid)
229224
any_failures, all_tests_passed = get_test_status(commit.repoid, commit.commitid)
230225

231-
# This functionality is disabled for now because it's too noisy for customers
226+
# TA error messaging is disabled for now (too noisy for customers)
232227
ta_error_msg = None
233228

234229
if any_failures and not all_tests_passed:
235230
self._call_upload_breadcrumb_task(
236231
commit_sha=commit.commitid,
237232
repo_id=commit.repoid,
238-
milestone=Milestones.NOTIFICATIONS_SENT,
233+
milestone=milestone,
239234
error=Errors.SKIPPED_NOTIFICATIONS,
240235
)
241236
return {
@@ -244,158 +239,28 @@ def run_impl_within_lock(
244239
"reason": "test_failures",
245240
}
246241

247-
try:
248-
installation_name_to_use = get_installation_name_for_owner_for_task(
249-
self.name, commit.repository.author
250-
)
251-
repository_service = get_repo_provider_service_for_specific_commit(
252-
commit, installation_name_to_use
253-
)
254-
except RepositoryWithoutValidBotError:
255-
save_commit_error(
256-
commit, error_code=CommitErrorTypes.REPO_BOT_INVALID.value
257-
)
258-
259-
log.warning(
260-
"Unable to start notifications because repo doesn't have a valid bot",
261-
extra={"repoid": repoid, "commit": commitid},
262-
)
263-
self.log_checkpoint(UploadFlow.NOTIF_NO_VALID_INTEGRATION)
264-
self._call_upload_breadcrumb_task(
265-
commit_sha=commitid,
266-
repo_id=repoid,
267-
milestone=milestone,
268-
error=Errors.REPO_MISSING_VALID_BOT,
269-
)
270-
return {"notified": False, "notifications": None, "reason": "no_valid_bot"}
271-
except NoConfiguredAppsAvailable as exp:
272-
if exp.rate_limited_count > 0:
273-
# There's at least 1 app that we can use to communicate with GitHub,
274-
# but this app happens to be rate limited now. We try again later.
275-
# Min wait time of 1 minute
276-
retry_delay_seconds = max(60, get_seconds_to_next_hour())
277-
log.warning(
278-
"Unable to start notifications. Retrying again later.",
279-
extra={
280-
"repoid": repoid,
281-
"commit": commitid,
282-
"apps_available": exp.apps_count,
283-
"apps_rate_limited": exp.rate_limited_count,
284-
"apps_suspended": exp.suspended_count,
285-
"countdown_seconds": retry_delay_seconds,
286-
},
287-
)
288-
self._call_upload_breadcrumb_task(
289-
commit_sha=commitid,
290-
repo_id=repoid,
291-
milestone=milestone,
292-
error=Errors.INTERNAL_APP_RATE_LIMITED,
293-
)
294-
return self._attempt_retry(
295-
max_retries=10,
296-
countdown=retry_delay_seconds,
297-
current_yaml=current_yaml,
298-
commit=commit,
299-
**kwargs,
300-
)
301-
# Maybe we have apps that are suspended. We can't communicate with github.
302-
log.warning(
303-
"We can't find an app to communicate with GitHub. Not notifying.",
304-
extra={
305-
"repoid": repoid,
306-
"commit": commitid,
307-
"apps_available": exp.apps_count,
308-
"apps_suspended": exp.suspended_count,
309-
},
310-
)
311-
self.log_checkpoint(UploadFlow.NOTIF_NO_APP_INSTALLATION)
312-
return {
313-
"notified": False,
314-
"notifications": None,
315-
"reason": "no_valid_github_app_found",
316-
}
242+
result = self._get_repository_service_for_notify(
243+
commit, repoid, commitid, milestone, current_yaml=current_yaml, **kwargs
244+
)
245+
if isinstance(result, dict):
246+
return result
247+
repository_service, installation_name_to_use = result
317248

318249
if current_yaml is None:
319250
current_yaml = async_to_sync(get_current_yaml)(commit, repository_service)
320251
else:
321252
current_yaml = UserYaml.from_dict(current_yaml)
322253

323-
try:
324-
ci_results = self.fetch_and_update_whether_ci_passed(
325-
repository_service, commit, current_yaml
326-
)
327-
except TorngitClientError as ex:
328-
log.info(
329-
"Unable to fetch CI results due to a client problem. Not notifying user",
330-
extra={
331-
"repoid": commit.repoid,
332-
"commit": commit.commitid,
333-
"code": ex.code,
334-
},
335-
)
336-
self.log_checkpoint(UploadFlow.NOTIF_GIT_CLIENT_ERROR)
337-
self._call_upload_breadcrumb_task(
338-
commit_sha=commit.commitid,
339-
repo_id=commit.repoid,
340-
milestone=milestone,
341-
error=Errors.GIT_CLIENT_ERROR,
342-
)
343-
return {
344-
"notified": False,
345-
"notifications": None,
346-
"reason": "not_able_fetch_ci_result",
347-
}
348-
except TorngitServerFailureError:
349-
log.info(
350-
"Unable to fetch CI results due to server issues. Not notifying user",
351-
extra={"repoid": commit.repoid, "commit": commit.commitid},
352-
)
353-
self.log_checkpoint(UploadFlow.NOTIF_GIT_SERVICE_ERROR)
354-
self._call_upload_breadcrumb_task(
355-
commit_sha=commit.commitid,
356-
repo_id=commit.repoid,
357-
milestone=milestone,
358-
error=Errors.GIT_CLIENT_ERROR,
359-
)
360-
return {
361-
"notified": False,
362-
"notifications": None,
363-
"reason": "server_issues_ci_result",
364-
}
254+
ci_result = self._fetch_ci_results_or_error(
255+
repository_service, commit, current_yaml, milestone
256+
)
257+
if isinstance(ci_result, dict):
258+
return ci_result
259+
ci_results = ci_result
260+
365261
if self.should_wait_longer(current_yaml, commit, ci_results):
366-
log.info(
367-
"Not sending notifications yet because we are waiting for CI to finish",
368-
extra={"repoid": commit.repoid, "commit": commit.commitid},
369-
)
370-
ghapp_default_installations = list(
371-
filter(
372-
lambda obj: obj.name == installation_name_to_use
373-
and obj.is_configured(),
374-
commit.repository.author.github_app_installations or [],
375-
)
376-
)
377-
rely_on_webhook_ghapp = ghapp_default_installations != [] and any(
378-
obj.is_repo_covered_by_integration(commit.repository)
379-
for obj in ghapp_default_installations
380-
)
381-
rely_on_webhook_legacy = commit.repository.using_integration
382-
if (
383-
rely_on_webhook_ghapp
384-
or rely_on_webhook_legacy
385-
or commit.repository.hookid
386-
):
387-
# rely on the webhook, but still retry in case we miss the webhook
388-
max_retries = 5
389-
countdown = (60 * 3) * 2**self.request.retries
390-
else:
391-
max_retries = 10
392-
countdown = 15 * 2**self.request.retries
393-
return self._attempt_retry(
394-
max_retries=max_retries,
395-
countdown=countdown,
396-
current_yaml=current_yaml,
397-
commit=commit,
398-
**kwargs,
262+
return self._handle_ci_wait_retry(
263+
commit, current_yaml, installation_name_to_use, **kwargs
399264
)
400265

401266
report_service = ReportService(
@@ -531,6 +396,191 @@ def run_impl_within_lock(
531396
)
532397
return {"notified": False, "notifications": None}
533398

399+
def _fetch_commit(self, db_session: Session, repoid: int, commitid: str) -> Commit:
400+
"""Fetch and validate the commit from the database."""
401+
commit = (
402+
db_session.query(Commit)
403+
.filter(Commit.repoid == repoid, Commit.commitid == commitid)
404+
.first()
405+
)
406+
assert commit, "Commit not found in database."
407+
return commit
408+
409+
def _get_repository_service_for_notify(
410+
self, commit, repoid, commitid, milestone, current_yaml=None, **kwargs
411+
):
412+
"""Get the repository service for notification, handling bot/app errors.
413+
414+
Returns (repository_service, installation_name) on success, or a dict
415+
result for early return on error.
416+
"""
417+
try:
418+
installation_name_to_use = get_installation_name_for_owner_for_task(
419+
self.name, commit.repository.author
420+
)
421+
repository_service = get_repo_provider_service_for_specific_commit(
422+
commit, installation_name_to_use
423+
)
424+
return (repository_service, installation_name_to_use)
425+
except RepositoryWithoutValidBotError:
426+
save_commit_error(
427+
commit, error_code=CommitErrorTypes.REPO_BOT_INVALID.value
428+
)
429+
log.warning(
430+
"Unable to start notifications because repo doesn't have a valid bot",
431+
extra={"repoid": repoid, "commit": commitid},
432+
)
433+
self.log_checkpoint(UploadFlow.NOTIF_NO_VALID_INTEGRATION)
434+
self._call_upload_breadcrumb_task(
435+
commit_sha=commitid,
436+
repo_id=repoid,
437+
milestone=milestone,
438+
error=Errors.REPO_MISSING_VALID_BOT,
439+
)
440+
return {"notified": False, "notifications": None, "reason": "no_valid_bot"}
441+
except NoConfiguredAppsAvailable as exp:
442+
return self._handle_no_configured_apps(
443+
commit, exp, repoid, commitid, milestone, current_yaml, **kwargs
444+
)
445+
446+
def _handle_no_configured_apps(
447+
self, commit, exp, repoid, commitid, milestone, current_yaml, **kwargs
448+
):
449+
"""Handle the case where no configured GitHub apps are available.
450+
451+
Either retries (if apps are rate-limited) or returns an error result.
452+
"""
453+
if exp.rate_limited_count > 0:
454+
# There's at least 1 app that we can use to communicate with GitHub,
455+
# but this app happens to be rate limited now. We try again later.
456+
# Min wait time of 1 minute
457+
retry_delay_seconds = max(60, get_seconds_to_next_hour())
458+
log.warning(
459+
"Unable to start notifications. Retrying again later.",
460+
extra={
461+
"repoid": repoid,
462+
"commit": commitid,
463+
"apps_available": exp.apps_count,
464+
"apps_rate_limited": exp.rate_limited_count,
465+
"apps_suspended": exp.suspended_count,
466+
"countdown_seconds": retry_delay_seconds,
467+
},
468+
)
469+
self._call_upload_breadcrumb_task(
470+
commit_sha=commitid,
471+
repo_id=repoid,
472+
milestone=milestone,
473+
error=Errors.INTERNAL_APP_RATE_LIMITED,
474+
)
475+
return self._attempt_retry(
476+
max_retries=10,
477+
countdown=retry_delay_seconds,
478+
current_yaml=current_yaml,
479+
commit=commit,
480+
**kwargs,
481+
)
482+
# Maybe we have apps that are suspended. We can't communicate with github.
483+
log.warning(
484+
"We can't find an app to communicate with GitHub. Not notifying.",
485+
extra={
486+
"repoid": repoid,
487+
"commit": commitid,
488+
"apps_available": exp.apps_count,
489+
"apps_suspended": exp.suspended_count,
490+
},
491+
)
492+
self.log_checkpoint(UploadFlow.NOTIF_NO_APP_INSTALLATION)
493+
return {
494+
"notified": False,
495+
"notifications": None,
496+
"reason": "no_valid_github_app_found",
497+
}
498+
499+
def _fetch_ci_results_or_error(
500+
self, repository_service, commit, current_yaml, milestone
501+
):
502+
"""Fetch CI results, handling provider errors.
503+
504+
Returns ci_results on success, or a dict result for early return on error.
505+
"""
506+
try:
507+
return self.fetch_and_update_whether_ci_passed(
508+
repository_service, commit, current_yaml
509+
)
510+
except TorngitClientError as ex:
511+
log.info(
512+
"Unable to fetch CI results due to a client problem. Not notifying user",
513+
extra={
514+
"repoid": commit.repoid,
515+
"commit": commit.commitid,
516+
"code": ex.code,
517+
},
518+
)
519+
self.log_checkpoint(UploadFlow.NOTIF_GIT_CLIENT_ERROR)
520+
self._call_upload_breadcrumb_task(
521+
commit_sha=commit.commitid,
522+
repo_id=commit.repoid,
523+
milestone=milestone,
524+
error=Errors.GIT_CLIENT_ERROR,
525+
)
526+
return {
527+
"notified": False,
528+
"notifications": None,
529+
"reason": "not_able_fetch_ci_result",
530+
}
531+
except TorngitServerFailureError:
532+
log.info(
533+
"Unable to fetch CI results due to server issues. Not notifying user",
534+
extra={"repoid": commit.repoid, "commit": commit.commitid},
535+
)
536+
self.log_checkpoint(UploadFlow.NOTIF_GIT_SERVICE_ERROR)
537+
self._call_upload_breadcrumb_task(
538+
commit_sha=commit.commitid,
539+
repo_id=commit.repoid,
540+
milestone=milestone,
541+
error=Errors.GIT_CLIENT_ERROR,
542+
)
543+
return {
544+
"notified": False,
545+
"notifications": None,
546+
"reason": "server_issues_ci_result",
547+
}
548+
549+
def _handle_ci_wait_retry(
550+
self, commit, current_yaml, installation_name_to_use, **kwargs
551+
):
552+
"""Handle retrying when CI hasn't finished yet."""
553+
log.info(
554+
"Not sending notifications yet because we are waiting for CI to finish",
555+
extra={"repoid": commit.repoid, "commit": commit.commitid},
556+
)
557+
ghapp_default_installations = list(
558+
filter(
559+
lambda obj: obj.name == installation_name_to_use
560+
and obj.is_configured(),
561+
commit.repository.author.github_app_installations or [],
562+
)
563+
)
564+
rely_on_webhook_ghapp = ghapp_default_installations != [] and any(
565+
obj.is_repo_covered_by_integration(commit.repository)
566+
for obj in ghapp_default_installations
567+
)
568+
rely_on_webhook_legacy = commit.repository.using_integration
569+
if rely_on_webhook_ghapp or rely_on_webhook_legacy or commit.repository.hookid:
570+
# rely on the webhook, but still retry in case we miss the webhook
571+
max_retries = 5
572+
countdown = (60 * 3) * 2**self.request.retries
573+
else:
574+
max_retries = 10
575+
countdown = 15 * 2**self.request.retries
576+
return self._attempt_retry(
577+
max_retries=max_retries,
578+
countdown=countdown,
579+
current_yaml=current_yaml,
580+
commit=commit,
581+
**kwargs,
582+
)
583+
534584
def is_using_codecov_commenter(
535585
self, repository_service: TorngitBaseAdapter
536586
) -> bool:

0 commit comments

Comments
 (0)