Merge pull request #661 from chrispyles/config-overrides

CHANGELOG.md

@@ -15,6 +15,8 @@
 * Enabled Otter Generate for all assignments created with Otter Assign
 * Updated Otter Assign to use Otter Run to validate that all tests in the assignment pass, allowing tests to be run for R assignments as well as Python, per [#427](https://github.com/ucbds-infra/otter-grader/issues/427)
 * Allow Markdown cells to be used instead of raw cells for Otter Assign per [#592](https://github.com/ucbds-infra/otter-grader/issues/592)
+* Use Otter Grade CLI flags to update `otter_config.json` values in the grading container per [#395](https://github.com/ucbds-infra/otter-grader/issues/395)
+* Removed `linux/amd64` platform specification for Docker images in Otter Grade
 
 **v4.4.0:**
 

otter/cli.py

@@ -137,6 +137,7 @@ def generate_cli(*args, **kwargs):
 @click.option("--timeout", type=click.INT, help="Submission execution timeout in seconds")
 @click.option("--no-network", is_flag=True, help="Disable networking in the containers")
 @click.option("--no-kill", is_flag=True, help="Do not kill containers after grading")
+@click.option("--debug", is_flag=True, help="Run in debug mode (without ignoring errors thrown during execution)")
 @click.option("--prune", is_flag=True, help="Prune all of Otter's grading images")
 @click.option("-f", "--force", is_flag=True, help="Force action (don't ask for confirmation)")
 def grade_cli(*args, **kwargs):

otter/grade/__init__.py

@@ -9,6 +9,7 @@
 from .containers import launch_containers
 from .utils import merge_csv, prune_images
 
+from ..run.run_autograder.autograder_config import AutograderConfig
 from ..utils import assert_path_exists, loggers
 
 
@@ -31,6 +32,7 @@ def main(
     force: bool = False,
     timeout: bool = None,
     no_network: bool = False,
+    debug: bool = False,
 ):
     """
     Run Otter Grade.
@@ -58,6 +60,7 @@ def main(
         force (``bool``): whether to force-prune the images (do not ask for confirmation)
         timeout (``int``): an execution timeout in seconds for each container
         no_network (``bool``): whether to disable networking in the containers
+        debug (``bool``): whether to run autograding in debug mode
 
     Returns:
         ``float | None``: the percentage scored by that submission if a single file was graded
@@ -114,6 +117,11 @@ def main(
         pdf_dir = pdf_dir,
         timeout = timeout,
         network = not no_network,
+        config = AutograderConfig({
+            "zips": ext == "zip",
+            "pdf": pdfs,
+            "debug": debug,
+        }),
     )
 
     LOGGER.info("Combining grades and saving")

otter/grade/containers.py

@@ -1,7 +1,9 @@
 """Docker container management for Otter Grade"""
 
+import json
 import os
 import pandas as pd
+import pathlib
 import pkg_resources
 import shutil
 import tempfile
@@ -14,21 +16,23 @@
 
 from .utils import OTTER_DOCKER_IMAGE_NAME
 
+from ..run.run_autograder.autograder_config import AutograderConfig
 from ..utils import loggers
 
 
-DOCKER_PLATFORM = "linux/amd64"
 LOGGER = loggers.get_logger(__name__)
 
 
-def build_image(ag_zip_path: str, base_image: str, tag: str):
+def build_image(ag_zip_path: str, base_image: str, tag: str, config: AutograderConfig):
     """
     Creates a grading image based on the autograder zip file and attaches a tag.
 
     Args:
         ag_zip_path (``str``): path to the autograder zip file
         base_image (``str``): base Docker image to build from
         tag (``str``): tag to be added when creating the image
+        config (``otter.run.run_autograder.autograder_config.AutograderConfig``): config overrides
+            for the autograder
 
     Returns:
         ``str``: the tag of the newly-build Docker image
@@ -42,13 +46,22 @@ def build_image(ag_zip_path: str, base_image: str, tag: str):
         with zipfile.ZipFile(ag_zip_path, 'r') as zip_ref:
             zip_ref.extractall(temp_dir)
 
+        # Update the otter_config.json file from the autograder zip with the provided config
+        # overrides.
+        config_path = pathlib.Path(temp_dir) / "otter_config.json"
+        old_config = AutograderConfig()
+        if config_path.exists():
+            old_config = AutograderConfig(json.loads(config_path.read_text("utf-8")))
+
+        old_config.update(config.get_user_config())
+        config_path.write_text(json.dumps(old_config.get_user_config()))
+
         docker.build(
             temp_dir,
             build_args={"BASE_IMAGE": base_image},
             tags=[image],
             file=dockerfile_path,
             load=True,
-            platforms=[DOCKER_PLATFORM],
         )
 
     return image
@@ -60,6 +73,7 @@ def launch_containers(
     num_containers: int,
     base_image: str,
     tag: str,
+    config: AutograderConfig,
     **kwargs,
 ):
     """
@@ -75,6 +89,8 @@ def launch_containers(
         num_containers (``int``): number of containers to run in parallel
         base_image (``str``): the name of a base image to use for building Docker images
         tag (``str``): a tag to use for the ``otter-grade`` image created for this assignment
+        config (``otter.run.run_autograder.autograder_config.AutograderConfig``): config overrides
+            for the autograder
         **kwargs: additional kwargs passed to ``grade_submission``
 
     Returns:
@@ -83,12 +99,16 @@ def launch_containers(
     """
     pool = ThreadPoolExecutor(num_containers)
     futures = []
-    image = build_image(ag_zip_path, base_image, tag)
+    image = build_image(ag_zip_path, base_image, tag, config)
 
     for subm_path in submission_paths:
-        futures += [
-            pool.submit(grade_submission, submission_path=subm_path, image=image, **kwargs)
-        ]
+        futures += [pool.submit(
+            grade_submission,
+            submission_path=subm_path,
+            image=image,
+            # config=config,
+            **kwargs,
+        )]
 
     # stop execution while containers are running
     finished_futures = wait(futures)
@@ -141,7 +161,7 @@ def grade_submission(
         if pdf_dir:
             volumes.append((pdf_path, f"/autograder/submission/{nb_name}.pdf"))
 
-        args = {'platform': DOCKER_PLATFORM}
+        args = {}
         if network is not None and not network:
             args['networks'] = 'none'
 

test/conftest.py

@@ -7,7 +7,7 @@
 from unittest import mock
 
 from otter import __file__ as OTTER_PATH
-from otter.grade.containers import build_image, DOCKER_PLATFORM
+from otter.grade.containers import build_image
 
 from .utils import TestFileManager
 
@@ -88,7 +88,6 @@ def build_image_with_local_changes(*args, **kwargs):
         tags=[image],
         file=FILE_MANAGER.get_path("Dockerfile"),
         load=True,
-        platforms=[DOCKER_PLATFORM],
     )
 
     return image

test/test_grade/test_integration.py

@@ -6,12 +6,14 @@
 import pytest
 import re
 import shutil
+import zipfile
 
 from glob import glob
 from unittest import mock
 
 from otter.generate import main as generate
 from otter.grade import main as grade
+from otter.run.run_autograder.autograder_config import AutograderConfig
 from otter.utils import loggers
 
 from ..utils import TestFileManager
@@ -20,6 +22,7 @@
 ASSIGNMENT_NAME = "otter-grade-test"
 FILE_MANAGER = TestFileManager(__file__)
 AG_ZIP_PATH = FILE_MANAGER.get_path("autograder.zip")
+ZIP_SUBM_PATH = "test/subm.zip"
 
 
 @pytest.fixture(autouse=True)
@@ -30,6 +33,8 @@ def cleanup_output(cleanup_enabled):
             os.remove("test/final_grades.csv")
         if os.path.exists("test/submission_pdfs"):
             shutil.rmtree("test/submission_pdfs")
+        if os.path.exists(ZIP_SUBM_PATH):
+            os.remove(ZIP_SUBM_PATH)
 
 
 @pytest.fixture(autouse=True, scope="module")
@@ -189,7 +194,8 @@ def test_single_notebook_grade(mocked_launch_grade):
         "no_kill": False,
         "pdf_dir": None,
         "timeout": None,
-        "network": True
+        "network": True,
+        "config": AutograderConfig(),
     }
 
     mocked_launch_grade.return_value = [df]
@@ -205,3 +211,79 @@ def test_single_notebook_grade(mocked_launch_grade):
 
     mocked_launch_grade.assert_called_with(notebook_path, [notebook_path], **kw_expected)
     assert output == 1.0
+
+
+@mock.patch("otter.grade.launch_containers")
+def test_config_overrides(mocked_launch_grade):
+    """
+    Checks that the CLI flags are converted to config overrides correctly.
+    """
+    mocked_launch_grade.return_value = [pd.DataFrame([{
+        "q1": 2.0,
+        "q2": 2.0,
+        "q3": 2.0,
+        "q4": 1.0,
+        "q6": 5.0,
+        "q2b": 2.0,
+        "q7": 1.0,
+        "percent_correct": 1.0,
+        "file": "passesAll.ipynb",
+    }])]
+
+    notebook_path = FILE_MANAGER.get_path("notebooks/passesAll.ipynb")
+    grade(
+        name = "foo",
+        paths = [notebook_path],
+        output_dir = "test/",
+        # the value of the autograder argument doesn't matter, it just needs to be a valid file path
+        autograder = notebook_path,
+        containers = 1,
+        pdfs = True,
+        ext = "zip",
+        debug = True,
+    )
+
+    assert mocked_launch_grade.call_args.kwargs["config"].get_user_config() == {
+        "zips": True,
+        "pdf": True,
+        "debug": True,
+    }
+
+
+def test_config_overrides_integration():
+    """
+    Checks that overriding otter_config.json configurations with CLI flags works.
+    """
+    notebook_path = FILE_MANAGER.get_path("notebooks/passesAll.ipynb")
+    with zipfile.ZipFile(ZIP_SUBM_PATH, "x") as zf:
+        zf.write(notebook_path, arcname="passesAll.ipynb")
+
+    output = grade(
+        name = "foo",
+        paths = [ZIP_SUBM_PATH],
+        output_dir = "test/",
+        # the value of the autograder argument doesn't matter, it just needs to be a valid file path
+        autograder = AG_ZIP_PATH,
+        ext = "zip",
+    )
+
+    assert output == 1.0
+
+    got = pd.read_csv("test/final_grades.csv")
+    want = pd.DataFrame([{
+        "q1": 0.0,
+        "q2": 2.0,
+        "q3": 2.0,
+        "q4": 1.0,
+        "q6": 5.0,
+        "q2b": 2.0,
+        "q7": 1.0,
+        "percent_correct": 1.0,
+        "file": ZIP_SUBM_PATH,
+    }])
+
+    # Sort the columns by label so the dataframes can be compared with ==.
+    got = got.reindex(sorted(got.columns), axis=1)
+    want = want.reindex(sorted(want.columns), axis=1)
+
+    assert got.equals(want)