Skip to content

Commit

Permalink
Remove per-notebook timeout check
Browse files Browse the repository at this point in the history
This is now handled by noteburst when we set the notebook execution
timeout in the execution request. This is a better approach because we
don't actually see the in_progress state for a notebook execution
(again, because arq doesn't expose that to the job metadata, only to the
job result it seems(?)
  • Loading branch information
jonathansick committed Sep 11, 2024
1 parent 277fde6 commit cf5e3ef
Showing 1 changed file with 0 additions and 24 deletions.
24 changes: 0 additions & 24 deletions src/timessquare/services/githubcheckrun.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@

import asyncio
from collections import deque
from datetime import timedelta

from gidgethub.httpx import GitHubAPI
from httpx import AsyncClient
Expand Down Expand Up @@ -283,29 +282,6 @@ async def _poll_noteburst_results(
check.report_noteburst_completion(
page_execution=page_execution, job_result=job
)
elif (
job.status == NoteburstJobStatus.in_progress
and job.runtime
> timedelta(
config.github_checkrun_timeout
) # less than job timeout
):
# If the job is in progress but has exceeded the timeout,
# report a timeout error.
# Because of peculiarity with arq, noteburst doesn't seem to
# indicate that a job is in progress, so this condition
# may not be triggered. Instead, a global timeout is needed
# for all jobs, which unfortunately is less granular because
# it may penalize repos with lots of notebooks, where the
# individual notebooks still execute in a reasonable time.
self._logger.debug(
"Noteburst job is in progress but exeeding timeout",
path=page_execution.page.repository_source_path,
runtime=job.runtime.total_seconds(),
)
check.report_noteburst_timeout(
page_execution=page_execution, job_result=job
)
else:
pending_pages.append(page_execution)
self._logger.debug(
Expand Down

0 comments on commit cf5e3ef

Please sign in to comment.