nisse-review: a stop ends a review in progress; quieter repo failures
SIGTERM used to stop only new rounds: a review in progress ran on until systemd killed it 90s later. Now the scheduler's stop sets the driver's cancel flag, so the run ends at its next turn, and a model call in flight is broken off (Interruptible, raising Stopped from the signal handler, only ever inside a model call). The item goes back to ready, with nothing drafted posted and no failure counted; replies already posted carry their markers and are not repeated. intake.repo_failed is logged when a repository starts failing or the reason changes, and intake.repo_ok when it lists again, not every round. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01J5Utyfzw7gF7UhjRDEtjTk
This commit is contained in:
1 parent
de1356cd17
commit
080205db7e
8 files changed
+287
-10
No files matched your search
@@ -18,7 +18,9 @@ comment quiets it until their `review` (R5), which asks for a fresh review.
|
||||
Crash safety (L8): an item found ``running`` was interrupted. Whether its
|
||||
review got posted is asked of Gitea, never assumed; until Gitea can say, the
|
||||
item waits rather than risk a second review. A reply carries the key of the
|
||||
point it answers, so a re-run never answers it twice.
|
||||
point it answers, so a re-run never answers it twice. A stop (SIGTERM) ends
|
||||
a run at its next turn, or breaks off its model call, and the item goes back
|
||||
to ready, with nothing drafted posted and no failure counted.
|
||||
"""
|
||||
|
||||
import re
|
||||
@@ -30,7 +32,9 @@ from dataclasses import replace
|
||||
from pathlib import Path
|
||||
|
||||
from nisse_core.gitea import GiteaClient, GiteaError, PullRequest
|
||||
from nisse_core.models.interruptible import Stopped
|
||||
from nisse_core.oplog import OpLog
|
||||
from nisse_core.state.cancel import Cancel
|
||||
from nisse_core.state.store import WorkItem
|
||||
from nisse_review.access import check, reviewed
|
||||
from nisse_review.reader import Repository
|
||||
@@ -96,6 +100,7 @@ class Scheduler:
|
||||
ci_grace_s: float = 120.0,
|
||||
operators: frozenset[str] = frozenset(),
|
||||
max_rounds: int = 4,
|
||||
cancel: Cancel | None = None,
|
||||
clock: Callable[[], float] = time.time,
|
||||
) -> None:
|
||||
self._store = store
|
||||
@@ -114,6 +119,9 @@ class Scheduler:
|
||||
self._clock = clock
|
||||
self._said_waiting: set[str] = set() # "item@head", logged once
|
||||
self._stop = threading.Event()
|
||||
# The reviewer's driver's flag: a stop ends a run at its next turn.
|
||||
self._cancel = cancel or Cancel()
|
||||
self._repo_failing: dict[str, str] = {} # repo: why, logged on change
|
||||
|
||||
# -- the loop ---------------------------------------------------------------
|
||||
|
||||
@@ -123,7 +131,10 @@ class Scheduler:
|
||||
self._stop.wait(self._poll_s)
|
||||
|
||||
def stop(self) -> None:
|
||||
"""No more rounds, and a run in progress ends at its next turn,
|
||||
back to ready: a stop is no failure of the review."""
|
||||
self._stop.set()
|
||||
self._cancel.set()
|
||||
|
||||
def round(self) -> bool:
|
||||
"""The wall, intake, then one run if one is due; True if one ran.
|
||||
@@ -158,9 +169,14 @@ class Scheduler:
|
||||
except GiteaError as exc:
|
||||
# One repository Gitea won't list (an empty one, one without
|
||||
# pull requests) holds up none of the others; its items stay
|
||||
# as they are.
|
||||
self._oplog.emit("intake.repo_failed", cause=str(exc)[:300], repo=repo)
|
||||
# as they are. Said when it starts, or the reason changes.
|
||||
why = str(exc)[:300]
|
||||
if self._repo_failing.get(repo) != why:
|
||||
self._repo_failing[repo] = why
|
||||
self._oplog.emit("intake.repo_failed", cause=why, repo=repo)
|
||||
continue
|
||||
if self._repo_failing.pop(repo, None) is not None:
|
||||
self._oplog.emit("intake.repo_ok", cause="listed again", repo=repo)
|
||||
seen: set[str] = set()
|
||||
for pr in prs:
|
||||
key = item_id(repo, pr.number)
|
||||
@@ -392,6 +408,9 @@ class Scheduler:
|
||||
"review.partial", cause=outcome.note, work_item=item.id
|
||||
)
|
||||
tool, summary = "post_review", PARTIAL.format(n=len(draft.comments))
|
||||
elif outcome.kind == "stopped":
|
||||
self._interrupted(item, known, outcome.note)
|
||||
return
|
||||
elif outcome.kind != "wait":
|
||||
self._failed(item, known, pr, outcome.note or outcome.kind)
|
||||
return
|
||||
@@ -404,6 +423,10 @@ class Scheduler:
|
||||
posted = self._reviewer.post(
|
||||
item.id, known.repo, pr, summary, kept, n=known.posts + 1
|
||||
)
|
||||
except Stopped as exc:
|
||||
# A stop broke off a model call: a stop all the same.
|
||||
self._interrupted(item, known, str(exc))
|
||||
return
|
||||
except GiteaError as exc:
|
||||
self._failed(item, known, None, f"Gitea: {exc}")
|
||||
return
|
||||
@@ -462,6 +485,18 @@ class Scheduler:
|
||||
cause=f"{tool}",
|
||||
)
|
||||
|
||||
def _interrupted(self, item: WorkItem, known: PR, why: str) -> None:
|
||||
"""A stop ended the run: nothing drafted is posted, no attempt is
|
||||
counted, and it runs again as it was. Replies it posted carry their
|
||||
markers, so the next run doesn't repeat them."""
|
||||
self._oplog.emit("review.interrupted", cause=why[:300], work_item=item.id)
|
||||
self._store.update_work_item(
|
||||
item.id,
|
||||
position=READY,
|
||||
data=data(replace(known, run="")),
|
||||
cause="stopped mid-run",
|
||||
)
|
||||
|
||||
def _failed(
|
||||
self, item: WorkItem, known: PR, pr: PullRequest | None, why: str
|
||||
) -> None:
|
||||
|
||||
Reference in new issue
Block a user