Merge pull request #708 from chrispyles/master

CHANGELOG.md

@@ -1,5 +1,10 @@
 # Changelog
 
+**v5.1.2 (unreleased):**
+
+* Enabled the use of custom Jupyter kernels by enforcing the use of the `python3` kernel inside Otter grading containers per [#706](https://github.com/ucbds-infra/otter-grader/issues/706)
+* Fixed a bug that was preventing Otter from exiting when an error was thrown during notebook execution caused by the log capturing solution per [#707](https://github.com/ucbds-infra/otter-grader/issues/707)
+
 **v5.1.1:**
 
 * Fixed a bug in attempting to read the users from the submission metadata when validating the autograder notebook in Otter Assign per [#695](https://github.com/ucbds-infra/otter-grader/issues/695)

otter/execute/__init__.py

@@ -19,14 +19,15 @@ def grade_notebook(
     *,
     tests_glob=[],
     ignore_errors=True,
-    script=False, 
+    script=False,
     cwd=None,
     test_dir=None,
     seed=None,
     seed_variable=None,
     log=None,
-    variables=None, 
+    variables=None,
     plugin_collection=None,
+    force_python3_kernel=True,
 ):
     """
     Grade an assignment file and return grade information.
@@ -72,30 +73,32 @@ def grade_notebook(
 
         (host, port), stop_server = start_server()
 
-        # GradingPreprocessor config
-        c.GradingPreprocessor.cwd = cwd
-        c.GradingPreprocessor.test_dir = test_dir
-        c.GradingPreprocessor.tests_glob = tests_glob
-        c.GradingPreprocessor.results_path = ntf.name
-        c.GradingPreprocessor.seed = seed
-        c.GradingPreprocessor.seed_variable = seed_variable
-        c.GradingPreprocessor.otter_log = log
-        c.GradingPreprocessor.variables = variables
-        c.GradingPreprocessor.logging_server_host = host
-        c.GradingPreprocessor.logging_server_port = port
-
-        # ExecutePreprocessor config
-        c.ExecutePreprocessor.allow_errors = ignore_errors
-
-        gp = GradingPreprocessor(config=c)
-        ep = ExecutePreprocessor(config=c)
-
-        nb, _ = gp.preprocess(nb)
-        executed_nb, _ = ep.preprocess(nb)
-
-        stop_server()
-
-        gp.cleanup()
+        try:
+            # GradingPreprocessor config
+            c.GradingPreprocessor.cwd = cwd
+            c.GradingPreprocessor.test_dir = test_dir
+            c.GradingPreprocessor.tests_glob = tests_glob
+            c.GradingPreprocessor.results_path = ntf.name
+            c.GradingPreprocessor.seed = seed
+            c.GradingPreprocessor.seed_variable = seed_variable
+            c.GradingPreprocessor.otter_log = log
+            c.GradingPreprocessor.variables = variables
+            c.GradingPreprocessor.logging_server_host = host
+            c.GradingPreprocessor.logging_server_port = port
+            c.GradingPreprocessor.force_python3_kernel = force_python3_kernel
+
+            # ExecutePreprocessor config
+            c.ExecutePreprocessor.allow_errors = ignore_errors
+
+            gp = GradingPreprocessor(config=c)
+            ep = ExecutePreprocessor(config=c)
+
+            nb, _ = gp.preprocess(nb)
+            executed_nb, _ = ep.preprocess(nb)
+
+        finally:
+            stop_server()
+            gp.cleanup()
 
         results = pickle.load(ntf)
 

otter/execute/preprocessor.py

@@ -8,7 +8,7 @@
 from nbconvert.exporters import PythonExporter
 from nbconvert.preprocessors import Preprocessor
 from textwrap import dedent
-from traitlets import Dict, Instance, Integer, List, Unicode
+from traitlets import Bool, Dict, Instance, Integer, List, Unicode
 from typing import Optional, Tuple
 
 from ..check.logs import Log
@@ -72,6 +72,8 @@ class GradingPreprocessor(Preprocessor):
 
     logging_server_port = Integer().tag(config=True)
 
+    force_python3_kernel = Bool().tag(config=True)
+
     @property
     def from_log(self):
         return self.otter_log is not None
@@ -83,6 +85,7 @@ def preprocess(self, nb, resources = None):
         self.add_checks(nb)
         self.add_seeds(nb)
         self.add_cwd_to_path(nb)
+        self.update_kernel(nb)
         self.add_init_and_export_cells(nb)  # this should always be the last call
         return nb, resources
 
@@ -194,9 +197,15 @@ def filter_out_non_imports(self, nb):
         ic.visit(tree)
         nb.cells = [nbf.v4.new_code_cell(ic.to_script())]
 
+    def update_kernel(self, nb):
+        if not self.force_python3_kernel:
+            return
+        nb["metadata"].get("kernelspec", {})["name"] = "python3"
+
     def cleanup(self):
         if self._log_temp_file:
             os.close(self._log_temp_file[0])
+            os.remove(self._log_temp_file[1])
 
 
 class ImportCollector(ast.NodeVisitor):

otter/run/__init__.py

@@ -56,7 +56,7 @@ def main(submission, *, autograder="./autograder.zip", output_dir="./", no_logo=
             shutil.copy(submission, os.path.join(ag_dir, "submission"))
 
         logo = not no_logo
-        run_autograder_main(ag_dir, logo=logo, debug=debug)
+        run_autograder_main(ag_dir, logo=logo, debug=debug, otter_run=True)
 
         results_path = os.path.join(ag_dir, "results", "results.json")
         shutil.copy(results_path, output_dir)

otter/run/run_autograder/__init__.py

@@ -16,14 +16,16 @@
 LOGGER = loggers.get_logger(__name__)
 
 
-def main(autograder_dir, **kwargs):
+def main(autograder_dir, otter_run=False, **kwargs):
     """
     Run the autograding process.
 
     Args:
         autograder_dir (``str``): the absolute path of the directory in which autograding is occurring
             (e.g. on Gradescope, this is ``/autograder``)
-        **kwargs: keyword arguments for updating autograder configurations=; these values override
+        otter_run (``bool``): whether this function is being invoked by Otter Run (i.e. without
+            containerization)
+        **kwargs: keyword arguments for updating autograder configurations; these values override
             anything present in ``otter_config.json``
     """
     dill = import_or_raise("dill")
@@ -38,6 +40,7 @@ def main(autograder_dir, **kwargs):
     config["autograder_dir"] = autograder_dir
 
     runner = create_runner(config, **kwargs)
+    runner.ag_config._otter_run = otter_run
 
     if runner.get_option("log_level") is not None:
         loggers.set_level(runner.get_option("log_level"))

otter/run/run_autograder/autograder_config.py

@@ -162,3 +162,6 @@ class AutograderConfig(fica.Config):
             "if a PDF cannot be generated from the submission",
         default=False,
     )
+
+    _otter_run = False
+    """whether this autograder run is being run by Otter Run (i.e. without containerization)"""

otter/run/run_autograder/runners/python_runner.py

@@ -134,6 +134,7 @@ def run(self):
                 variables = self.ag_config.serialized_variables,
                 plugin_collection = plugin_collection,
                 script = os.path.splitext(subm_path)[1] == ".py",
+                force_python3_kernel = not self.ag_config._otter_run,
             )
 
             # verify the scores against the log

test/test_execute/test_integration.py

@@ -5,6 +5,8 @@
 import shutil
 import tempfile
 
+from glob import glob
+
 from otter.check.logs import EventType, Log, LogEntry
 from otter.execute import grade_notebook
 
@@ -21,6 +23,31 @@ def temp_dir():
     shutil.rmtree(d)
 
 
+def test_kernel_override(temp_dir):
+    """
+    Tests that ``otter.execute.grade_notebook`` overrides a custom kernel name when indicated.
+    """
+    nb = nbf.v4.new_notebook(cells=[nbf.v4.new_code_cell("x = 2")])
+    nb["metadata"]["kernelspec"] = {"name": "somekernel"}
+    subm_path = os.path.join(temp_dir, "submission.ipynb")
+    nbf.write(nb, subm_path)
+
+    test_dir = os.path.join(temp_dir, "tests")
+    os.makedirs(test_dir)
+
+    write_ok_test(os.path.join(test_dir, "q1.py"), ">>> assert x == 2")
+
+    results = grade_notebook(
+        subm_path,
+        test_dir=test_dir,
+        tests_glob=glob(os.path.join(test_dir, "*.py")),
+        ignore_errors=False,
+    )
+
+    assert results.notebook["metadata"]["kernelspec"]["name"] == "python3"
+    assert results.total == 1
+
+
 def test_log_execution(temp_dir):
     """
     Test for ``otter.execute.grade_notebook`` when a log is provided.