fix(warehouse-sources): make github workflow_runs webhook-only - #70634
Conversation
workflow_runs lacked the initial_lookback_days=0 floor that workflow_jobs and reviews use, so a freshly connected source crawls the repo's entire /actions/runs history before webhook mode can engage. On a busy repo that is a huge, rate-limited backfill against a shared GitHub App budget, and it delays the table (and anything reading it) for hours. Set the zero-day floor so the poll does no historical backfill and webhook mode activates from the first run, matching the sibling webhook-fed endpoints.
|
Reviews (1): Last reviewed commit: "fix(warehouse-sources): make github work..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
This PR makes the GitHub warehouse source’s workflow_runs endpoint behave as webhook-only on initial connect, avoiding an expensive historical crawl of /actions/runs before webhook ingestion can begin.
Changes:
- Set
initial_lookback_days=0on theworkflow_runsendpoint config to align it with other webhook-driven endpoints. - Add inline rationale explaining why historical backfill is undesirable for
workflow_runs.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
Note 🤖 stamphog reviewed Verified in source (source.py's source_for_pipeline + webhook_s3.py's webhook_enabled): the new "yield no rows" short-circuit fires for any workflow_runs schema just because the endpoint is webhook-capable, not because that schema is actually configured for webhook sync (schema.is_webhook). Existing production sources with workflow_runs still set to incremental/append poll mode (which was the only option before this PR, per the old comment/test this diff replaces) will silently stop receiving new rows after this ships — exactly the P1 regression Codex flagged, which the author's reply ("we only want schemas to ever be webhook based for runs") doesn't actually refute or fix.
Gate mechanics and policy version
Updated in place — this replaces 2 earlier stamphog review(s) on this PR. |
The initial_lookback_days=0 marker alone only flips webhook activation and marks the schema webhook_only — it does not bound the direct workflow_runs poll. If the poll path is ever taken (webhook function missing/disabled, or reset_pipeline forced it), get_rows still crawled the entire /actions/runs history, the exact backfill the marker is meant to prevent. Mirror the Slack/Stripe webhook-only sources: when a webhook-capable, zero-lookback endpoint has webhook mode inactive, yield no rows instead of polling. Also reword the settings comment to describe the marker accurately (it is not a poll floor) and add a regression test across the three webhook-only endpoints.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
SourceResponse.items returns Iterable | AsyncIterable; list() needs the sync branch. Assert isinstance Iterable to narrow (and to pin that the webhook-only fallback returns a sync empty iterator, not the async webhook stream).
Two pre-existing e2e tests encoded the old "workflow_runs keeps poll" behavior and failed once workflow_runs became webhook-only: - test_skip_check_scoped_to_zero_backfill_endpoint expected workflow_runs to keep the initial_sync_complete gate; it now skips it like the other webhook-only endpoints. - test_workflow_jobs_is_webhook_only_but_workflow_runs_keeps_poll asserted workflow_runs stays poll-backed; both runs and jobs are webhook-only now. Update the assertions, case names, and the stale source.py comment to match the intended behavior.
…hook The empty-iterator short-circuit fired for any webhook-capable workflow_runs schema, so an existing schema still configured for incremental/poll sync (the only mode before this change) would silently stop receiving rows once workflow_runs went webhook-only. Add WebhookSourceManager.schema_is_webhook and only yield no rows when the schema is actually in webhook mode; legacy poll-mode schemas keep polling. The extra read only runs on the poll fallback (webhook mode inactive). Add a regression test for the poll-mode path.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Same Iterable | AsyncIterable union as the webhook-only case — the poll path returns a sync iterator, so assert isinstance Iterable before list().
🤖 CI report
|
| File | Patch | Uncovered changed lines |
|---|---|---|
products/warehouse_sources/backend/temporal/data_imports/tests/e2e/github/test_github_source.py |
20.0% | 560, 1440–1442 |
products/warehouse_sources/backend/temporal/data_imports/sources/github/tests/test_github_webhook_only.py |
21.6% | 10–12, 17–19, 21–22, 30, 32–33, 35, 37–39, 46–48, 50–52, 54–55, 63–65, 67, 69–70 |
products/warehouse_sources/backend/temporal/data_imports/sources/github/github.py |
75.0% | 975 |
🤖 Agents: add a test covering the lines above, or note why under "How did you test this code?". Machine-readable gap list: the patch-coverage artifact on this run (gh run download 29330440980 -n patch-coverage), or the coverage-data block at the end of this comment.
Per-product line coverage (touched products)
| Product | Coverage | Lines |
|---|---|---|
warehouse_sources |
████████░░░░░░░░░░░░ 42.4% |
94,160 / 221,866 |
Report-only. Patch coverage = changed backend lines covered vs origin/master. Sorted lowest first.
Known gaps: lines covered only by Temporal tests show as uncovered; core line numbers may drift if master changed the same file.
…ithub-runs-webhook-only
#70640 (now in master) renamed the webhook-first flag to webhook_only; the poll-noop gate still referenced the old skip_initial_sync_complete_check name. Point it at the merged webhook_only variable.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7b62d68864
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| # makes the poll path yield no rows instead of crawling the full /actions/runs history. | ||
| # Crawling a busy repo's whole run history on connect is huge against a shared, rate-limited | ||
| # budget. History, if wanted, is a deliberate one-off backfill. | ||
| initial_lookback_days=0, |
There was a problem hiding this comment.
For existing GitHub sources whose workflow_runs schema is still incremental/append (schema.is_webhook is false), this zero-lookback marker makes github_source return SourceResponse.webhook_only=True; the pipeline passes that into handle_reset_or_full_refresh, whose reset_pipeline && webhook_only branch clears reset_pipeline without calling reset_table. Those legacy schemas still poll because of the new schema_is_webhook guard, so a user/admin reset now silently leaves the old Delta table/cursor in place instead of rebuilding. Gate the reset-preservation path on the DB schema actually being webhook sync, not only the endpoint config.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in the latest push. SourceResponse.webhook_only is now derived from the endpoint being webhook-first AND the schema actually being in webhook sync mode (webhook_enabled implies it; otherwise a schema_is_webhook DB check on the poll fallback). So a legacy poll-mode workflow_runs schema reports webhook_only=False and its resets still wipe+rebuild, while it keeps polling. Added assertions in both directions.
…a-aware The endpoint-level zero-lookback marker flagged SourceResponse.webhook_only=True even for a legacy poll-mode workflow_runs schema (is_webhook False). That schema keeps polling (via the schema_is_webhook guard), but the reset-preservation path in handle_reset_or_full_refresh would then skip the wipe on a user/admin reset, leaving stale data. Derive webhook_only from the endpoint AND the schema's actual sync mode, so legacy poll schemas keep wiping+rebuilding on reset. Assert both directions in the test.
Problem
Connecting a GitHub warehouse source kicks off a full historical crawl of
/actions/runsforworkflow_runs,even though the source is meant to run on webhooks.
On a busy repo that is tens of thousands of pages against a shared, rate-limited GitHub App budget,
and webhook mode only switches on once that initial sync finishes.
So the table can take hours to appear, and the crawl pressures every other consumer sharing the installation.
Changes
Make
workflow_runswebhook-first, matchingworkflow_jobsandreviews. Two parts:initial_lookback_days=0on theworkflow_runsendpoint config. This marks the schemawebhook_only(source.py) and activates webhook mode from the first sync (skips theinitial_sync_completegate ingithub.py).initial_lookback_daysonly bounds the fan-out children, not the directworkflow_runspoll, so without this the fallback (get_rows) still crawled all of/actions/runswhenever webhook mode was off (no webhook function, orreset_pipeline). Nowgithub_sourcereturns no rows in that case, same as the Slack/Stripe webhook-only sources.Note
Behavior change for all GitHub sources: new connects no longer backfill
workflow_runshistory.They get runs from webhook time onward, same as
workflow_jobstoday, andsupports_pollis false for the endpoint by design.Existing sources that already completed their backfill are in webhook steady-state and unaffected.
History, if wanted, is a deliberate one-off backfill.
How did you test this code?
I (Claude) did not run a live sync. I added a regression test (
test_github_webhook_only.py) that, across all three webhook-only endpoints (workflow_runs,workflow_jobs,reviews), asserts the poll yields zero rows and makes no_fetch_pagecalls when webhook mode is inactive — locking in that the fallback can't re-crawl history.hogli teston that file passes (3/3), andruff+hogli ci:preflightare clean.Automatic notifications
🤖 Agent context
Autonomy: Human-driven (agent-assisted)
Julian spotted a freshly connected source doing a long, egress-throttled
workflow_runsbackfill while its childworkflow_jobs(webhook-only) was not.The first cut was just
initial_lookback_days=0, but review (Codex, Greptile, Copilot) correctly pointed out that marker only flips webhook activation — the directworkflow_runspoll ignores it, so the fallback could still crawl full history, and the original comment overstated the effect.I (Claude) expanded the fix to make the poll fallback yield no rows for webhook-only endpoints (the pattern Slack/Stripe already use), reworded the comment, and added the regression test.
No skills invoked beyond reading the code paths; the change is source-config plus a source-layer poll short-circuit, not a DRF or migration surface.