Fail PRs when a source query does not build - #150
Merged
Conversation
`evidence sources` logs query errors but still exits 0, so a broken source SQL file reaches main unnoticed and only surfaces as a failed Netlify build. Nothing in CI ran the source build at all. Add scripts/check-sources.mjs, which compares sources/**/*.sql against the per-query directories Evidence writes under .evidence/meta/ (created only on success), and a workflow that runs the build plus this check on PRs touching sources/. Fork PRs cannot read secrets, so the job skips with a notice instead of a spurious failure. Found while reviewing #139, where sources/nfcore_db/pipeline_timeline.sql references a column the ingestion pipeline has not created yet; the build error was invisible to CI.
The Evidence CLI already exposes `sources --strict` ("Fail when a source
query fails"), which is what the hand-rolled script reimplemented. It is
also more accurate: it fails at the query with the database error, while
comparing .evidence/meta directories only reports which file produced no
output and can read stale output from an earlier build.
The fork check was a shell step that read the secret to find out whether secrets were readable, then guarded all three remaining steps on its output. GitHub reports the same thing directly, and a skipped job already shows as neutral rather than failed.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
evidence sourceslogs a failing query and then exits 0. Nothing in CI runs thesource build, so broken source SQL merges cleanly and first surfaces as a failed
Netlify build, with the affected page down until someone notices.
This adds a workflow that runs
evidence sources --stricton PRs touchingsources/. The CLI already has that flag; it fails on the first query errorinstead of logging and continuing.
Fork PRs cannot read secrets, so the job skips with a notice rather than
reporting a failure the contributor cannot act on. That is the deliberate gap:
a source change arriving from a fork is still unguarded, and catching it would
need
pull_request_targetand the credential exposure that comes with it.Found while reviewing #139, whose
pipeline_timeline.sqlreads a column theingestion pipeline has not created yet. Against the production database that
query fails today, and CI reports nothing.