Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add config to raise error when (any) notebook execution fails #248

Closed
poldrack opened this issue Jul 13, 2020 · 10 comments · Fixed by #404
Closed

add config to raise error when (any) notebook execution fails #248

poldrack opened this issue Jul 13, 2020 · 10 comments · Fixed by #404

Comments

@poldrack
Copy link

Is your feature request related to a problem? Please describe.
When I try to build my book using Github actions, there is no way to specifically cause the build to return an error when a notebook fails to execute. The -W flag would achieve this, but it also would error out for other warnings (e.g. those about heading levels) that are not fatal.

Describe the solution you'd like
Either add a flag that causes jb build to raise an error when any of the notebooks fail to properly execute, or (preferably) change the default behavior so that the build fails whenever a notebook fails to execute. The latter is what I would have expected.

@choldgraf
Copy link
Member

choldgraf commented Jul 13, 2020

Thanks for bringing this up. I'm not quite sure the right path forward here, since the way that Sphinx generally handles this is the -W pattern you describe.

I wonder if you could suppress some "expected warnings" using something like either or these:

Otherwise, I guess we could special-case myst-nb execution even if -W isn't given...

@chrisjsewell
Copy link
Member

With the execution data now captured in: https://myst-nb.readthedocs.io/en/latest/use/execute.html#execution-statistics, we could add a configuration and build-finished event call back in the myst-nb settings to raise a (Sphinx)Error when env.nb_execution_data indicates a failed build

@chrisjsewell
Copy link
Member

I'm going to move this to myst-nb then, as I think we can do that there 😄

@welcome
Copy link

welcome bot commented Aug 24, 2020

Thanks for opening your first issue here! Engagement like this is essential for open source projects! 🤗

If you haven't done so already, check out EBP's Code of Conduct. Also, please try to follow the issue template as it helps other community members to contribute more effectively.

If your issue is a feature request, others may react to it, to raise its prominence (see Feature Voting).

Welcome to the EBP community! 🎉

@chrisjsewell chrisjsewell transferred this issue from jupyter-book/jupyter-book Aug 24, 2020
@chrisjsewell chrisjsewell changed the title add flag to build to raise error when notebook execution fails add config to raise error when (any) notebook execution fails Aug 24, 2020
@rossbar
Copy link
Contributor

rossbar commented Dec 2, 2020

I'd just like to second this feature request. We have a very similar use-case where we're relying on sphinx+myst-nb to execute the notebooks in a CI system, and would like the CI to fail if notebook execution fails. I too initially tried the SPHINXOPTS=-W route, but then we get failures on other (more benign) warnings like the non-consecutive header warnings from myst-parser (executablebooks/MyST-Parser#262)

I've tried several workarounds, including the two listed above (AFAICT these only work for nitpicky mode and built-in sphinx warnings, respectively) and adding warnings.filterwarning("ignore", "Non-consecutive heading") to conf.py, but none of these has worked.

@jakevdp
Copy link
Contributor

jakevdp commented Jan 20, 2021

We're exploring using myst-nb in the JAX project (see jax-ml/jax#5426), and this issue is a hard blocker: we need failed notebook executions to trigger a CI failure, otherwise we risk publishing broken code & outputs to the docs. Like other users, in our case the -W flag is not a viable option; the reason is we have hundreds of innocuous warnings stemming from pre-existing formatting issues in numpy and scipy docstrings, which are automatically included in docstrings of JAX's wrapped functions.

Please let me know if you have any other suggestions for surfacing notebook execution errors to CI - thanks!

@chrisjsewell
Copy link
Member

Thanks for the feedback, this definitely in the plans.
Hopefully in the next week or two

@jakevdp
Copy link
Contributor

jakevdp commented Jan 20, 2021

Thanks!

@jakevdp
Copy link
Contributor

jakevdp commented Jan 27, 2021

I started working on this in #296. Feedback appreciated!

@jakevdp
Copy link
Contributor

jakevdp commented Jan 28, 2021

In the meantime, I've found one (quite brittle) workaround: monkeypatch the logger used in myst_nb.execution by adding the following to your sphinx conf.py file:

from myst_nb import execution

class _FakeLogger:
    def __init__(self, logger):
        self._logger = logger

    def verbose(self, *args, **kwargs):
        return self._logger.verbose(*args, **kwargs)

    def info(self, *args, **kwargs):
        return self._logger.info(*args, **kwargs)

    def error(self, *args, **kwargs):
        if str(args[0]).lower().startswith("execution failed"):
            raise ValueError(args[0])
        return self._logger.error(*args, **kwargs)

execution.LOGGER = _FakeLogger(execution.LOGGER)

It's not pretty, but it gets the job done 😁

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants