From 6bdc54b900bdc0a02012b1a7761e76a6dbc295dd Mon Sep 17 00:00:00 2001 From: Chris Pyles Date: Tue, 16 Jan 2024 20:26:59 -0800 Subject: [PATCH 1/3] make no PDF ack handle errors in addition to silent failures --- CHANGELOG.md | 4 ++++ otter/check/notebook.py | 11 +++++++---- otter/check/utils.py | 10 +++++++--- otter/test_files/__init__.py | 9 ++------- otter/utils.py | 14 ++++++++++++++ 5 files changed, 34 insertions(+), 14 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c91aa13b..bf3c3a9e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,9 @@ # Changelog +**v5.2.3:** + +* Fixed the no PDF acknowledgement feature to handle when PDF exports throw an error instead of failing silently + **v5.2.2:** * Fixed an `AttributeError` when using `Notebook.check_all` per [#746](https://github.com/ucbds-infra/otter-grader/issues/746) diff --git a/otter/check/notebook.py b/otter/check/notebook.py index 562611af..5e5070f9 100644 --- a/otter/check/notebook.py +++ b/otter/check/notebook.py @@ -457,10 +457,11 @@ def export( zf = zipfile.ZipFile(zip_path, mode="w") zf.write(nb_path) - pdf_created = True + pdf_path, pdf_created, pdf_error = None, True, None if pdf: - pdf_path = export_notebook(nb_path, filtering=filtering, pagebreaks=pagebreaks) - if os.path.isfile(pdf_path): + try: pdf_path = export_notebook(nb_path, filtering=filtering, pagebreaks=pagebreaks) + except Exception as e: pdf_error = e + if pdf_path and os.path.isfile(pdf_path): pdf_created = True zf.write(pdf_path) self._logger.debug(f"Wrote PDF to zip file: {pdf_path}") @@ -520,10 +521,12 @@ def continue_export(): display(HTML(out_html)) if pdf_created or not self._nbmeta_config.require_no_pdf_confirmation: + if pdf_error is not None: + raise pdf_error continue_export() else: display_pdf_confirmation_widget( - self._nbmeta_config.export_pdf_failure_message, continue_export) + self._nbmeta_config.export_pdf_failure_message, pdf_error, continue_export) @grading_mode_disabled @logs_event(EventType.END_CHECK_ALL) diff --git a/otter/check/utils.py b/otter/check/utils.py index 80f6413c..4b6f259e 100644 --- a/otter/check/utils.py +++ b/otter/check/utils.py @@ -19,7 +19,7 @@ from ..nbmeta_config import NBMetadataConfig from ..test_files import GradingResults -from ..utils import import_or_raise +from ..utils import format_exception, import_or_raise T = TypeVar("T") @@ -332,7 +332,7 @@ def resolve_test_info( return test_path, test_name -def display_pdf_confirmation_widget(message: Optional[str], callback: Callable) -> None: +def display_pdf_confirmation_widget(message: Optional[str], pdf_error: Optional[Exception], callback: Callable) -> None: """ Display a widget to the user to acknowledge that a PDF will not be included in their submission zip. @@ -349,7 +349,11 @@ def wrapped_callback(*args): message = "Your notebook could not be exported as a PDF. To continue exporting your " \ "submission, please click the button below." - t = HTML(f"""

{message}

""") + message_html = f"""

{message}

""" + if pdf_error is not None: + message_html += f"""
{format_exception(pdf_error)}
""" + + t = HTML(message_html) b = Button(description="Continue export", button_style="warning") b.on_click(wrapped_callback) m = HTML("""
""") diff --git a/otter/test_files/__init__.py b/otter/test_files/__init__.py index 7ff4dd8b..eea55729 100644 --- a/otter/test_files/__init__.py +++ b/otter/test_files/__init__.py @@ -5,7 +5,6 @@ import nbformat as nbf import os import pickle -import traceback from typing import Any, Dict, List, Optional @@ -16,7 +15,7 @@ from .ottr_test import OttrTestFile from ..nbmeta_config import NBMetadataConfig, OK_FORMAT_VARNAME -from ..utils import QuestionNotInLogException +from ..utils import format_exception, QuestionNotInLogException __all__ = [ @@ -414,11 +413,7 @@ def to_gradescope_dict(self, ag_config): "output": "The autograder failed to produce any results. Please alert your instructor to this failure for assistance in debugging it.", "status": "failed", }) - tb = "".join(traceback.format_exception( - type(self._catastrophic_error), - self._catastrophic_error, - self._catastrophic_error.__traceback__, - )) + tb = format_exception(self._catastrophic_error) output["tests"].append({ "name": "Autograder Exception", "visibility": "hidden", diff --git a/otter/utils.py b/otter/utils.py index dc0a411b..5fc677da 100644 --- a/otter/utils.py +++ b/otter/utils.py @@ -10,6 +10,7 @@ import string import shutil import tempfile +import traceback import yaml from contextlib import contextmanager @@ -398,3 +399,16 @@ class QuestionNotInLogException(Exception): """ Exception that indicates that a specific question was not found in any entry in the log """ + + +def format_exception(e: Exception) -> str: + """ + Formats an exception for display with its traceback using the ``traceback`` module. + + Args: + e (``Exception``): the exception to format + + Returns: + ``str``: the formatted exception + """ + return "".join(traceback.format_exception(type(e), e, e.__traceback__)) From 79b8d16013b36046f63645cb2e00b07ef00b7bb3 Mon Sep 17 00:00:00 2001 From: Chris Pyles <40970945+chrispyles@users.noreply.github.com> Date: Tue, 16 Jan 2024 20:30:34 -0800 Subject: [PATCH 2/3] Allow releases off of a temp branch for patches --- .github/workflows/release.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 3be729eb..ac22656c 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -22,9 +22,9 @@ jobs: with: python-version: 3.8 - - name: Only allow releases off of master + - name: Only allow releases off of master (or temp for patches) run: | - python3 -c 'import os, sys; sys.exit(os.environ["GITHUB_REF"] != "refs/heads/master")' + python3 -c 'import os, sys; sys.exit(os.environ["GITHUB_REF"] != "refs/heads/master" and os.environ["GITHUB_REF"] != "refs/heads/temp")' - uses: actions/checkout@v2 From fbfe69eb36db6358ba9ad7a300d70ca59dacb83d Mon Sep 17 00:00:00 2001 From: Chris Pyles Date: Tue, 16 Jan 2024 20:31:43 -0800 Subject: [PATCH 3/3] run tests on PRs to temp branch --- .github/workflows/run-tests.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/run-tests.yml b/.github/workflows/run-tests.yml index 56a9cf6c..ec2d88d1 100644 --- a/.github/workflows/run-tests.yml +++ b/.github/workflows/run-tests.yml @@ -13,6 +13,7 @@ on: - master - beta - release + - temp workflow_dispatch: jobs: