CI: gate the first jb build so notebook errors cannot pass green - #1019
Merged
Conversation
The sphinx-tojupyter step is the first `jb build` in this workflow, and
`execute_notebooks: "cache"` means only the first build executes notebooks
— the later HTML build reads the cache. So this step is where a
CellExecutionError surfaces, and it could not fail.
It already carried `-n -W`, but that was inert: the step runs three
commands under `shell: bash -l {0}`, and GitHub only injects `-eo pipefail`
for the bare `shell: bash` shorthand. An explicit custom shell spec gets
neither `-e` nor `-o pipefail`, so a failing `jb build` was followed by
`mkdir` and `cp`, and the step exited with `cp`'s status. `--keep-going`
makes that worse rather than better, since it forces Sphinx to emit output
despite the errors, guaranteeing the .ipynb files exist for `cp`.
One line fixes it. The flags were already right.
Refs QuantEcon/meta#340
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes a CI reliability gap where the first jb build (the only build that actually executes notebooks due to execute_notebooks: "cache") could fail but still allow the workflow step to exit successfully because subsequent mkdir/cp commands overwrite the step exit code under shell: bash -l {0}.
Changes:
- Add
set -eo pipefailto ensure the “Build Download Notebooks (sphinx-tojupyter)” step fails immediately ifjb buildfails. - Add an in-step comment explaining why this step is the gating point and why
set -eo pipefailis necessary with a custom shell spec.
📖 Netlify Preview Ready!Preview URL: https://pr-1019--sunny-cactus-210e3e.netlify.app Commit: Build Info
|
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
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.
Part of QuantEcon/meta#340.
The problem
The
Build Download Notebooks (sphinx-tojupyter)step is the firstjb buildin this workflow, andlectures/_config.ymlsetsexecute_notebooks: "cache"— so it is the only build that actually executes notebooks; the later HTML build reads the cache. That makes it the step where aCellExecutionErrorsurfaces.It could not fail.
The flags were already correct (
-n -W --keep-going), but inert. The step runs three commands undershell: bash -l {0}, and GitHub only injects-eo pipefailfor the bareshell: bashshorthand — an explicit custom shell spec gets neither-enor-o pipefail. So a failingjb buildwas followed bymkdirandcp, and the step exited withcp's status.--keep-goingmakes this worse rather than better: it forces Sphinx to emit output despite the errors, guaranteeing the.ipynbfiles exist forcpto succeed on.The change
One line —
set -eo pipefail— plus a comment recording why it is needed and why this step is the one that matters. No flag changes, no_config.ymlchange.Note on which step
The rule is "gate the first
jb build", not "gate a particular step name". Where a repo'sBuild PDF from LaTeXstep is active it runs first and is the one to gate (see QuantEcon/lecture-python-intro#809); here the LaTeX step comes later, so the notebooks step is first. The audit table in QuantEcon/meta#340 records which step applies per repo.Verified before the change
This repo already uses
raises-exceptiontags where cells are meant to error, so nothing is relying on an error being tolerated silently.Refs QuantEcon/meta#340
🤖 Generated with Claude Code