diff --git a/.github/workflows/cookbook_preproc.yaml b/.github/workflows/cookbook_preproc.yaml index 0c1705a3..6bb86a67 100644 --- a/.github/workflows/cookbook_preproc.yaml +++ b/.github/workflows/cookbook_preproc.yaml @@ -93,7 +93,13 @@ jobs: - name: Pre-process and execute notebooks shell: bash -l {0} run: | - python source/_ext/proc_examples.py --prefix=deploy/ --cache-branch=${DEPLOY_BRANCH} + set -e + python source/_ext/proc_examples.py --prefix=deploy/ --cache-branch=${DEPLOY_BRANCH} --log-failures=notebooks_log.json + + - name: Read notebooks log + if: always() + shell: bash -l {0} + run: echo "NOTEBOOKS_LOG=$(cat 'notebooks_log.json')" >> "$GITHUB_ENV" - name: Deploy cache shell: bash -l {0} @@ -121,6 +127,7 @@ jobs: curl -X POST -d "branches=$GITHUB_REF_NAME" -d "token=${{ secrets.RTD_WEBHOOK_TOKEN }}" https://readthedocs.org/api/v2/webhook/openff-docs/243876/ - name: Report status to PR + id: reportStatusToPr if: always() && github.event_name == 'workflow_dispatch' && inputs.pr_number != '' uses: thollander/actions-comment-pull-request@v2 with: @@ -137,10 +144,49 @@ jobs: - Deployment branch: ${{ env.DEPLOY_BRANCH }} - - Status: **${{ job.status }}** + - Job status: **${{ job.status }}** + + - Notebooks status: ${{fromJSON(env.NOTEBOOKS_LOG).n_successful}} / ${{fromJSON(env.NOTEBOOKS_LOG).n_total}} notebooks successfully executed (${{fromJSON(env.NOTEBOOKS_LOG).n_ignored}} failures ignored) + + ${{(fromJSON(env.NOTEBOOKS_LOG).failed || fromJSON(env.NOTEBOOKS_LOG).ignored) && '- Failing notebooks: + - ' || ''}}${{join(fromJSON(env.NOTEBOOKS_LOG).failed, ' + - ')}}${{fromJSON(env.NOTEBOOKS_LOG).ignored && ' + - [ignored] ' || ''}}${{join(fromJSON(env.NOTEBOOKS_LOG).ignored, ' + - [ignored] ')}} + + + Changes will only be visible in the ReadTheDocs + preview after it has been [rebuilt]. + + + [${{ github.run_id }}]: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }} + + [rebuilt]: https://readthedocs.org/projects/openff-docs/builds/ + + + - name: Report status to PR on templating failure + if: always() && steps.reportStatusToPr.outcome == 'failure' + uses: thollander/actions-comment-pull-request@v2 + with: + pr_number: ${{ inputs.pr_number }} + message: > + A workflow dispatched to regenerate the cookbook cache for this PR has just finished. + + + - Run ID: [${{ github.run_id }}] + + - Triggering actor: ${{ github.triggering_actor }} + + - Target branch: ${{ github.ref_name }} + + - Deployment branch: ${{ env.DEPLOY_BRANCH }} + + - Job status: **${{ job.status }}** + + - Notebooks status: N/A - If the workflow was successful, changes will only be visible in the ReadTheDocs + Changes will only be visible in the ReadTheDocs preview after it has been [rebuilt]. diff --git a/devtools/conda-envs/examples_env.yml b/devtools/conda-envs/examples_env.yml index 739f4be3..088fd958 100644 --- a/devtools/conda-envs/examples_env.yml +++ b/devtools/conda-envs/examples_env.yml @@ -13,8 +13,8 @@ dependencies: - requests - packaging # Examples - - openff-toolkit-examples>=0.15.2 - - openff-interchange >=0.3.25 + - openff-toolkit-examples>=0.16.0 + - openff-interchange>=0.3.26 - openff-nagl # - openff-fragmenter # - openff-qcsubmit diff --git a/source/_ext/cookbook/globals_.py b/source/_ext/cookbook/globals_.py index c83df0b8..f628e119 100644 --- a/source/_ext/cookbook/globals_.py +++ b/source/_ext/cookbook/globals_.py @@ -81,17 +81,30 @@ DEFAULT_CACHE_BRANCH = "_cookbook_data_main" """Branch of the openff-docs repository where cached notebooks are stored.""" -SKIP_NOTEBOOKS: set[str] = { - "openforcefield/openff-interchange/experimental/openmmforcefields/gaff.ipynb", -} +SKIP_NOTEBOOKS: set[str] = {} """ Notebooks that will not be processed. -This is intended to be used as a way of temporarily disabling broken notebooks -without taking down an entire source repository or the examples page itself. - Specified as a path relative to a notebook search path, eg ``SRC_IPYNB_ROOT``. This is something like ``{repo_owner}/{repo_name}/{path_from_examples_dir}``. These notebooks are skipped at the proc_examples stage, but they will still be rendered if they're in a cache. """ + +OPTIONAL_NOTEBOOKS: list[str] = [ + # ".*/experimental/.*", + "openforcefield/openff-interchange/experimental/openmmforcefields/gaff.ipynb", +] +""" +Notebooks whose execution failure will not cause notebook processing to fail. + +This is intended to be used as a way of temporarily disabling broken notebooks +without taking down an entire source repository or the examples page itself. +They will be executed and included in the examples page if successful, but if +they fail they will be removed. + +Specified as a regex matching a path relative to a notebook search path, eg +``SRC_IPYNB_ROOT``. This is something like ``{repo_owner}/{repo_name}/ +{path_from_examples_dir}``. These notebooks are skipped at the proc_examples +stage, but they will still be rendered if they're in a cache. +""" diff --git a/source/_ext/cookbook/utils.py b/source/_ext/cookbook/utils.py index d8c4b268..76f3adb7 100644 --- a/source/_ext/cookbook/utils.py +++ b/source/_ext/cookbook/utils.py @@ -1,9 +1,23 @@ +from functools import partial from types import FunctionType -from typing import Callable, Iterable, Iterator, Optional, TypeVar, Generator +from typing import ( + Callable, + Generic, + Iterable, + Iterator, + Optional, + TypeVar, + Generator, + ParamSpec, + Union, +) import contextlib import os +import re T = TypeVar("T") +E = TypeVar("E") +P = ParamSpec("P") def flatten(iterable: Iterable[Iterable[T]]) -> Generator[T, None, None]: @@ -20,14 +34,20 @@ def next_or_none(iterator: Iterator[T]) -> Optional[T]: return ret -def result(fn, exception=Exception): - def ret(*args, **kwargs): - try: - return fn(*args, **kwargs) - except exception as e: - return e +def result( + fn: Callable[P, T], exception: type[E], *args: P.args, **kwargs: P.kwargs +) -> Union[T, E]: + try: + return fn(*args, **kwargs) + except exception as e: + return e - return ret + +def to_result( + fn: Callable[P, T], exception: type[E] = Exception +) -> Callable[P, Union[T, E]]: + # partial's type stub is not precise enough to work here + return partial(result, fn, exception) # type: ignore [reportReturnType] @contextlib.contextmanager @@ -54,3 +74,10 @@ def set_env(**environ): finally: os.environ.clear() os.environ.update(old_environ) + + +def in_regexes(s: str, regexes: Iterable[str]) -> bool: + for regex in regexes: + if re.match(regex, s): + return True + return False diff --git a/source/_ext/proc_examples.py b/source/_ext/proc_examples.py index b27e50e7..03ff6d2c 100644 --- a/source/_ext/proc_examples.py +++ b/source/_ext/proc_examples.py @@ -1,5 +1,6 @@ """Script to execute and pre-process example notebooks""" +import re from typing import Tuple, List, Final from zipfile import ZIP_DEFLATED, ZipFile from pathlib import Path @@ -10,6 +11,7 @@ import sys import tarfile from functools import partial +import traceback import nbformat from nbconvert.preprocessors.execute import ExecutePreprocessor @@ -30,7 +32,14 @@ ) from cookbook.github import download_dir, get_tag_matching_installed_version from cookbook.globals_ import * -from cookbook.utils import set_env +from cookbook.utils import set_env, to_result, in_regexes + + +class NotebookExceptionError(ValueError): + def __init__(self, src: str, exc: Exception): + self.src: str = str(src) + self.exc: Exception = exc + self.tb: str = "".join(traceback.format_exception(exc, chain=False)) def needed_files(notebook_path: Path) -> List[Tuple[Path, Path]]: @@ -208,7 +217,8 @@ def execute_notebook( try: executor.preprocess(nb, {"metadata": {"path": src.parent}}) except Exception as e: - raise ValueError(f"Exception encountered while executing {src_rel}") + print("Failed to execute", src.relative_to(SRC_IPYNB_ROOT)) + raise NotebookExceptionError(str(src_rel), e) # Store the tag used to execute the notebook in metadata set_metadata(nb, "src_repo_tag", tag) @@ -230,7 +240,7 @@ def execute_notebook( EXEC_IPYNB_ROOT / thumbnail_path.relative_to(SRC_IPYNB_ROOT), ) - print("Done executing", src.relative_to(SRC_IPYNB_ROOT)) + print("Successfully executed", src.relative_to(SRC_IPYNB_ROOT)) def delay_iterator(iterator, seconds=1.0): @@ -260,6 +270,8 @@ def main( do_exec=True, prefix: Path | None = None, processes: int | None = None, + failed_notebooks_log: Path | None = None, + allow_failures: bool = False, ): print("Working in", Path().resolve()) @@ -287,7 +299,6 @@ def main( for notebook in find_notebooks(dst_path) if str(notebook.relative_to(SRC_IPYNB_ROOT)) not in SKIP_NOTEBOOKS ) - print(notebooks, SKIP_NOTEBOOKS) # Create Colab and downloadable versions of the notebooks if do_proc: @@ -299,6 +310,7 @@ def main( create_download(notebook) # Execute notebooks in parallel for rendering as HTML + execution_failed = False if do_exec: shutil.rmtree(EXEC_IPYNB_ROOT, ignore_errors=True) # Context manager ensures the pool is correctly terminated if there's @@ -306,13 +318,55 @@ def main( with Pool(processes=processes) as pool: # Wait a second between launching subprocesses # Workaround https://github.com/jupyter/nbconvert/issues/1066 - _ = [ - *pool.imap_unordered( - partial(execute_notebook, cache_branch=cache_branch), + exec_results = [ + *pool.imap( + to_result( + partial(execute_notebook, cache_branch=cache_branch), + NotebookExceptionError, + ), delay_iterator(notebooks), ) ] + exceptions: list[NotebookExceptionError] = [ + result for result in exec_results if isinstance(result, Exception) + ] + ignored_exceptions = [ + exc for exc in exceptions if in_regexes(exc.src, OPTIONAL_NOTEBOOKS) + ] + + if exceptions: + for exception in exceptions: + print( + "-" * 80 + + "\n" + + f"{exception.src} failed. Traceback:\n\n{exception.tb}" + ) + if not in_regexes(exception.src, OPTIONAL_NOTEBOOKS): + execution_failed = True + print(f"The following {len(exceptions)}/{len(notebooks)} notebooks failed:") + for exception in exceptions: + print(" ", exception.src) + print("For tracebacks, see above.") + + if failed_notebooks_log is not None: + print(f"Writing log to {failed_notebooks_log.absolute()}") + failed_notebooks_log.write_text( + json.dumps( + { + "n_successful": len(notebooks) - len(exceptions), + "n_total": len(notebooks), + "n_ignored": len(ignored_exceptions), + "failed": [ + exc.src + for exc in exceptions + if exc not in ignored_exceptions + ], + "ignored": [exc.src for exc in ignored_exceptions], + } + ) + ) + if isinstance(prefix, Path): prefix.mkdir(parents=True, exist_ok=True) @@ -327,6 +381,9 @@ def main( prefix / directory.relative_to(OPENFF_DOCS_ROOT), ) + if execution_failed: + exit(1) + if __name__ == "__main__": import sys, os @@ -365,10 +422,41 @@ def main( "Specify cache branch in a single argument: `--cache-branch=`" ) + # --log-failures is the path to store a list of failing notebooks in + failed_notebooks_log = None + for arg in sys.argv: + if arg.startswith("--log-failures="): + failed_notebooks_log = Path(arg[15:]) + if "--log-failures" in sys.argv: + raise ValueError( + "Specify path to log file in a single argument: `--log-failures=`" + ) + + # if --allow-failures is True, do not exit with error code 1 if a + # notebook fails + allow_failures = "false" + for arg in sys.argv: + if arg.startswith("--allow-failures="): + allow_failures = arg[17:].lower() + if allow_failures in ["true", "1", "y", "yes", "t"]: + allow_failures = True + elif allow_failures in ["false", "0", "n", "no", "false"]: + allow_failures = False + else: + raise ValueError( + f"Didn't understand value of --allow-failures {allow_failures}; try `true` or `false`" + ) + if "--log-failures" in sys.argv: + raise ValueError( + "Specify value in a single argument: `--allow-failures=true` or `--allow-failures=false`" + ) + main( cache_branch=cache_branch, do_proc=not "--skip-proc" in sys.argv, do_exec=not "--skip-exec" in sys.argv, prefix=prefix, processes=processes, + failed_notebooks_log=failed_notebooks_log, + allow_failures=allow_failures, )