Skip to content

Commit e9cc5bd

Browse files
authored
[CRCR] Add nightly/periodic callback handler (Phase 1) (#8302)
## Summary Phase 1 of the CRCR nightly/periodic self-report implementation ([RFC 98](pytorch/rfcs#98)). Adds a new code path in the callback Lambda for `nightly` and `periodic` event types. Unlike PR/push callbacks which require a `DISPATCHED` record in Redis and follow a two-step state machine (`in_progress` → `completed`), nightly/periodic callbacks use a **single-callback model**: - Downstream repo self-triggers via cron - Runs CI against a `pytorch/pytorch` SHA (from `nightly` branch or `main`) - Reports the final result in **one callback** — no `in_progress` step **What's different from PR/push:** - No Redis writes, no state machine, no zombie tracking - Status must be `completed` (rejects `in_progress`) - No upstream check runs (nightly results are informational only) - Timing metrics (queue_time, execution_time) are `null` — no dispatch record to measure against **Changes:** | File | Change | |------|--------| | `callback/callback_handler.py` | New `_handle_nightly_callback()` + routing in `handle()` | | `tests/test_callback_handler.py` | 8 new tests covering the nightly path | **Remaining phases:** - Phase 2: Update callback action YAML with `delivery-id` / `event-type` inputs - Phase 3: Downstream cron workflow in `TorchedHat/pytorch-redhat-ci` - Phase 4: ClickHouse query for nightly results - Phase 5-6: HUD display ## Test plan - [x] 8 unit tests: forward to HUD, skip Redis, no timing metrics, periodic variant, reject `in_progress`, allowlist gating, rate limiting, missing `delivery_id` - [ ] CI passes on this PR
1 parent 58cd1a9 commit e9cc5bd

2 files changed

Lines changed: 191 additions & 14 deletions

File tree

aws/lambda/cross_repo_ci_relay/callback/callback_handler.py

Lines changed: 69 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,23 @@
2222

2323
logger = logging.getLogger(__name__)
2424

25+
_NIGHTLY_EVENT_TYPES = frozenset({"nightly", "periodic"})
26+
27+
28+
def _build_trusted(
29+
verified_repo: str,
30+
repo_level: AllowlistLevel,
31+
ci_metrics: dict | None = None,
32+
) -> dict:
33+
"""Build the trusted payload block forwarded to HUD."""
34+
if ci_metrics is None:
35+
ci_metrics = {"queue_time": None, "execution_time": None}
36+
return {
37+
"ci_metrics": ci_metrics,
38+
"verified_repo": verified_repo,
39+
"downstream_repo_level": repo_level.value,
40+
}
41+
2542

2643
def _safe_delta(
2744
start_ts: float | None, end_ts: float | None, label: str
@@ -234,6 +251,42 @@ def _create_upstream_check_run(
234251
)
235252

236253

254+
def _handle_nightly_callback(
255+
config: RelayConfig,
256+
body: dict,
257+
verified_repo: str,
258+
repo_level: AllowlistLevel,
259+
) -> dict:
260+
"""Handle a nightly/periodic self-report callback.
261+
262+
Unlike PR/push callbacks, nightly/periodic have no prior dispatch record
263+
and no state machine. The downstream repo self-triggers via cron, runs CI
264+
against a pytorch/pytorch SHA (from the nightly branch or main), and
265+
reports the final result in a single callback.
266+
267+
No Redis writes, no zombie tracking, no upstream check runs.
268+
"""
269+
delivery_id, status, *_ = _parse_callback_body(body)
270+
271+
if status != "completed":
272+
raise HTTPException(
273+
400,
274+
f"nightly/periodic callbacks must have status 'completed', got {status!r}",
275+
)
276+
277+
trusted = _build_trusted(verified_repo, repo_level)
278+
untrusted = {"callback_payload": body}
279+
280+
forward_to_hud(config, trusted, untrusted)
281+
logger.info(
282+
"nightly callback forwarded delivery_id=%s repo=%s event_type=%s",
283+
delivery_id,
284+
verified_repo,
285+
body.get("event_type", "unknown"),
286+
)
287+
return {"ok": True, "status": status}
288+
289+
237290
def handle(config: RelayConfig, body: dict, verified_repo: str) -> dict:
238291
"""Forward a downstream callback to HUD.
239292
@@ -245,7 +298,11 @@ def handle(config: RelayConfig, body: dict, verified_repo: str) -> dict:
245298
for allowlist / timing lookups, and surfaced to HUD as ``verified_repo``
246299
so HUD can trust it over anything self-reported in the body.
247300
248-
State machine ensures:
301+
For nightly/periodic event types, the state machine is bypassed entirely
302+
and the result is forwarded to HUD in a single callback (no Redis, no
303+
in_progress step, no zombie tracking).
304+
305+
For PR/push events, the state machine ensures:
249306
- Callbacks without prior dispatch are rejected
250307
- Timestamps (started_at, completed_at) are recorded once only
251308
- Duplicate callbacks are handled gracefully
@@ -256,6 +313,15 @@ def handle(config: RelayConfig, body: dict, verified_repo: str) -> dict:
256313
return {"ok": True, "status": "ignored"}
257314
allowlist, repo_level = result
258315

316+
# NOTE: event_type is untrusted (comes from the callback body, not from the
317+
# OIDC token). It only selects among safe code paths — the nightly path
318+
# never grants additional capability (no check runs, no Redis writes, no
319+
# state machine bypass for PR events). HUD treats nightly rows as
320+
# informational, attributed to the OIDC-verified repo.
321+
event_type = body.get("event_type", "")
322+
if event_type in _NIGHTLY_EVENT_TYPES:
323+
return _handle_nightly_callback(config, body, verified_repo, repo_level)
324+
259325
delivery_id, status, run_id, run_attempt, workflow_name, job_name = (
260326
_parse_callback_body(body)
261327
)
@@ -278,14 +344,7 @@ def handle(config: RelayConfig, body: dict, verified_repo: str) -> dict:
278344
payload = None
279345
if status == "in_progress":
280346
payload = {
281-
"trusted": {
282-
"ci_metrics": {
283-
"queue_time": None,
284-
"execution_time": None,
285-
},
286-
"verified_repo": verified_repo,
287-
"downstream_repo_level": repo_level.value,
288-
},
347+
"trusted": _build_trusted(verified_repo, repo_level),
289348
"untrusted": {"callback_payload": body},
290349
}
291350

@@ -360,11 +419,7 @@ def handle(config: RelayConfig, body: dict, verified_repo: str) -> dict:
360419
config, delivery_id, verified_repo, run_id, run_attempt, job_name=job_name
361420
)
362421

363-
trusted = {
364-
"ci_metrics": ci_metrics,
365-
"verified_repo": verified_repo,
366-
"downstream_repo_level": repo_level.value,
367-
}
422+
trusted = _build_trusted(verified_repo, repo_level, ci_metrics)
368423
# downstream's payload is untrusted — provide it under the "callback_payload"
369424
# key so HUD receives it under the expected untrusted namespace.
370425
untrusted = {"callback_payload": body}

aws/lambda/cross_repo_ci_relay/tests/test_callback_handler.py

Lines changed: 122 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -459,5 +459,127 @@ def test_check_run_update_failure_does_not_break_response(self):
459459
self.assertEqual(result, {"ok": True, "status": "completed"})
460460

461461

462+
class TestNightlyCallback(unittest.TestCase):
463+
"""Nightly/periodic callbacks bypass the state machine entirely."""
464+
465+
def setUp(self):
466+
self.patcher_allowlist = patch("callback.callback_handler.load_allowlist")
467+
self.mock_load = self.patcher_allowlist.start()
468+
mock_map = MagicMock()
469+
mock_map.get_repo_level.return_value = AllowlistLevel.L2
470+
self.mock_load.return_value = mock_map
471+
472+
self.patcher_redis = patch("callback.callback_handler.redis_helper")
473+
self.mock_redis = self.patcher_redis.start()
474+
475+
self.patcher_rate = patch("callback.callback_handler.check_rate_limit")
476+
self.mock_rate = self.patcher_rate.start()
477+
self.mock_rate.return_value = True
478+
479+
self.patcher_hud = patch("callback.callback_handler.forward_to_hud")
480+
self.mock_hud = self.patcher_hud.start()
481+
482+
def tearDown(self):
483+
self.patcher_allowlist.stop()
484+
self.patcher_redis.stop()
485+
self.patcher_rate.stop()
486+
self.patcher_hud.stop()
487+
488+
def _nightly_body(
489+
self, event_type="nightly", status="completed", conclusion="success"
490+
):
491+
return {
492+
"event_type": event_type,
493+
"delivery_id": "abc123def456",
494+
"payload": {},
495+
"workflow": {
496+
"status": status,
497+
"conclusion": conclusion,
498+
"name": "CRCR Nightly CI",
499+
"url": "https://github.com/org/repo/actions/runs/12345",
500+
"run_id": "12345",
501+
"run_attempt": "1",
502+
"job_name": "nightly-build",
503+
"check_run_id": "67890",
504+
},
505+
}
506+
507+
def test_nightly_callback_forwards_to_hud(self):
508+
body = self._nightly_body()
509+
result = handle(_cfg(), body, verified_repo="org/repo")
510+
511+
self.assertEqual(result, {"ok": True, "status": "completed"})
512+
self.mock_hud.assert_called_once()
513+
_, trusted, untrusted = self.mock_hud.call_args[0]
514+
self.assertEqual(trusted["verified_repo"], "org/repo")
515+
self.assertIs(untrusted["callback_payload"], body)
516+
517+
def test_nightly_callback_skips_redis(self):
518+
handle(_cfg(), self._nightly_body(), verified_repo="org/repo")
519+
520+
self.mock_redis.get_callback_state.assert_not_called()
521+
self.mock_redis.set_callback_state.assert_not_called()
522+
self.mock_redis.add_in_progress_tracker.assert_not_called()
523+
self.mock_redis.remove_in_progress_tracker.assert_not_called()
524+
525+
def test_nightly_callback_has_no_timing_metrics(self):
526+
handle(_cfg(), self._nightly_body(), verified_repo="org/repo")
527+
528+
_, trusted, _ = self.mock_hud.call_args[0]
529+
self.assertIsNone(trusted["ci_metrics"]["queue_time"])
530+
self.assertIsNone(trusted["ci_metrics"]["execution_time"])
531+
532+
def test_periodic_callback_also_works(self):
533+
body = self._nightly_body(event_type="periodic")
534+
result = handle(_cfg(), body, verified_repo="org/repo")
535+
536+
self.assertEqual(result, {"ok": True, "status": "completed"})
537+
self.mock_hud.assert_called_once()
538+
539+
def test_nightly_rejects_in_progress_status(self):
540+
body = self._nightly_body(status="in_progress")
541+
with self.assertRaises(HTTPException) as ctx:
542+
handle(_cfg(), body, verified_repo="org/repo")
543+
self.assertEqual(ctx.exception.status_code, 400)
544+
self.assertIn("completed", str(ctx.exception.detail))
545+
546+
def test_nightly_not_in_allowlist_is_ignored(self):
547+
mock_map = MagicMock()
548+
mock_map.get_repo_level.return_value = None
549+
self.mock_load.return_value = mock_map
550+
551+
result = handle(_cfg(), self._nightly_body(), verified_repo="unknown/repo")
552+
553+
self.assertEqual(result, {"ok": True, "status": "ignored"})
554+
self.mock_hud.assert_not_called()
555+
556+
def test_nightly_rate_limited(self):
557+
self.mock_rate.return_value = False
558+
559+
with self.assertRaises(HTTPException) as ctx:
560+
handle(_cfg(), self._nightly_body(), verified_repo="org/repo")
561+
self.assertEqual(ctx.exception.status_code, 429)
562+
563+
def test_nightly_missing_delivery_id_returns_400(self):
564+
body = self._nightly_body()
565+
del body["delivery_id"]
566+
567+
with self.assertRaises(HTTPException) as ctx:
568+
handle(_cfg(), body, verified_repo="org/repo")
569+
self.assertEqual(ctx.exception.status_code, 400)
570+
571+
def test_nightly_failure_conclusion_forwards(self):
572+
body = self._nightly_body()
573+
body["workflow"]["conclusion"] = "failure"
574+
result = handle(_cfg(), body, verified_repo="org/repo")
575+
576+
self.assertEqual(result, {"ok": True, "status": "completed"})
577+
self.mock_hud.assert_called_once()
578+
_, trusted, untrusted = self.mock_hud.call_args[0]
579+
self.assertEqual(
580+
untrusted["callback_payload"]["workflow"]["conclusion"], "failure"
581+
)
582+
583+
462584
if __name__ == "__main__":
463585
unittest.main()

0 commit comments

Comments
 (0)