Skip to content

Commit 1d4e99b

Browse files
Allow choosing custom versions of python for graders (#92)
## Motivation * Sets us up for supporting Java (notably this DOES NOT add official Java support) * Allows us to offer higher versions of python, without breaking backwards compatibility with graders relying on python 3.10 behavior This PR also does not add a warning banner if an assignment is using a deprecated version of a language, in order to reduce the already large and complex diff. This would be better as a "good first issue". ## Implementation This adds a new model, `Language`, which stores information about the language, binary path, and more. The field `Assignment.language` has been moved inside said model. All the information about the specific language is stored in a dictionary called `info`. The `info` dictionary must contain a `version` key that is sortable. For the `python` grader language, it must also contain the key `python3` which has the absolute path to a python executable. The addition of another language to Tin can then consist of simply adding other keys to the `info` dictionary as needed. > [!IMPORTANT] > Custom migrations were written in order to facilitate this transition with minimal problems. The rest is just some restructuring: * Changes the `run_submission` task to use the assignments executable * Filters the edit assignment/create venv to only show non-deprecated language options. * Changes the `create_venv` task to use the correct executable
1 parent 592ca28 commit 1d4e99b

17 files changed

Lines changed: 324 additions & 44 deletions

File tree

tin/apps/assignments/admin.py

Lines changed: 34 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,15 @@
22

33
import datetime
44

5-
from django.contrib import admin
5+
from django.contrib import admin, messages
6+
from django.utils.translation import ngettext
67

78
from .models import (
89
Assignment,
910
CooldownPeriod,
1011
FileAction,
1112
Folder,
13+
Language,
1214
MossResult,
1315
Quiz,
1416
QuizLogMessage,
@@ -34,7 +36,7 @@ def assignments(self, obj):
3436
class AssignmentAdmin(admin.ModelAdmin):
3537
date_hierarchy = "due"
3638
list_display = ("name", "course_name", "folder", "due", "visible", "quiz_icon")
37-
list_filter = ("language", "course", "due")
39+
list_filter = ("language_details", "course", "due")
3840
ordering = ("-due",)
3941
save_as = True
4042
search_fields = ("name",)
@@ -53,6 +55,36 @@ def quiz_icon(self, obj):
5355
return bool(obj.is_quiz)
5456

5557

58+
@admin.register(Language)
59+
class LanguageAdmin(admin.ModelAdmin):
60+
list_display = ("name", "language", "info", "is_deprecated")
61+
search_fields = ("name",)
62+
ordering = ("-language", "is_deprecated", "name")
63+
save_as = True
64+
list_filter = ("language",)
65+
actions = ["make_deprecated"]
66+
67+
@admin.action(description="Mark languages as deprecated")
68+
def make_deprecated(self, request, queryset) -> None:
69+
changed = 0
70+
for language in queryset:
71+
language.is_deprecated = True
72+
language.name = f"{language.name} (Deprecated)"
73+
language.save()
74+
changed += 1
75+
76+
self.message_user(
77+
request,
78+
ngettext(
79+
"Successfully marked %d language as deprecated.",
80+
"Successfully marked %d languages as deprecated.",
81+
changed,
82+
)
83+
% changed,
84+
messages.SUCCESS,
85+
)
86+
87+
5688
@admin.register(CooldownPeriod)
5789
class CooldownPeriodAdmin(admin.ModelAdmin):
5890
date_hierarchy = "start_time"

tin/apps/assignments/forms.py

Lines changed: 19 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,10 @@
66
from django import forms
77
from django.conf import settings
88
from django.core.exceptions import ValidationError
9+
from django.db.models import Q
910

1011
from ..submissions.models import Submission
11-
from .models import Assignment, Folder, MossResult, SubmissionCap
12+
from .models import Assignment, Folder, Language, MossResult, SubmissionCap
1213

1314
logger = getLogger(__name__)
1415

@@ -23,19 +24,29 @@ def __init__(self, course, *args, **kwargs):
2324
super().__init__(*args, **kwargs)
2425
self.fields["folder"].queryset = Folder.objects.filter(course=course)
2526

26-
# Prevent changing the language of an assignment after it has been created
2727
instance = getattr(self, "instance", None)
2828
if instance and instance.pk:
29-
self.fields["language"].help_text = (
30-
"Changing this after uploading a grader script is not recommended and will cause "
31-
"issues."
29+
# allow them to change from a deprecated language to a non-deprecated one
30+
# but it must be the same (e.g. python -> python, or java -> java)
31+
self.fields["language_details"].queryset = Language.objects.filter(
32+
Q(is_deprecated=False) & Q(language=instance.language_details.language)
33+
# just nicer UI to show the deprecated language choice
34+
| Q(id=instance.language_details.id)
3235
)
3336

3437
cap = instance.submission_caps.filter(student__isnull=True).first()
3538
if cap is not None:
3639
self.fields["submission_cap"].initial = cap.submission_cap
3740
self.fields["submission_cap_after_due"].initial = cap.submission_cap_after_due
3841

42+
else:
43+
self.fields["language_details"].queryset = Language.objects.filter(is_deprecated=False)
44+
self.fields[
45+
"language_details"
46+
].help_text = (
47+
"Keep in mind you cannot swap between languages after the assignment has been created."
48+
)
49+
3950
# prevent description from getting too big
4051
self.fields["description"].widget.attrs.update({"id": "description"})
4152

@@ -65,7 +76,7 @@ class Meta:
6576
"description",
6677
"markdown",
6778
"folder",
68-
"language",
79+
"language_details",
6980
"filename",
7081
"venv",
7182
"points_possible",
@@ -98,6 +109,7 @@ class Meta:
98109
"is_quiz": "Is this a quiz?",
99110
"quiz_autocomplete_enabled": "Enable code autocompletion?",
100111
"quiz_description_markdown": "Use markdown?",
112+
"language_details": "Grader language",
101113
}
102114
sections = (
103115
{
@@ -116,7 +128,7 @@ class Meta:
116128
"description": "",
117129
"fields": (
118130
"folder",
119-
"language",
131+
"language_details",
120132
"filename",
121133
"venv",
122134
),
Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,81 @@
1+
# Generated by Django 4.2.16 on 2024-11-11 01:40
2+
3+
from django.db import migrations, models
4+
5+
6+
def migrate_to_foreignkey(apps, schema_editor):
7+
"""Creates a default :class:`.Language` model for each assignment.
8+
9+
This converts the old language field to a foreign key to the new language model.
10+
"""
11+
12+
Language = apps.get_model("assignments", "Language")
13+
db_alias = schema_editor.connection.alias
14+
python_310, py_created = Language.objects.using(db_alias).get_or_create(
15+
name="Python 3.10",
16+
info={"python3": "/usr/bin/python3.10", "version": 3.10},
17+
language="P",
18+
)
19+
20+
Assignment = apps.get_model("assignments", "Assignment")
21+
Assignment.objects.using(db_alias).update(language_details=python_310)
22+
23+
# avoid creating empty models
24+
if py_created and not python_310.assignment_set.exists():
25+
python_310.delete()
26+
27+
28+
def revert_default_language(apps, schema_editor):
29+
"""Converts the foreign key :class:`.Language` back to the old ``language`` field."""
30+
31+
Assignment = apps.get_model("assignments", "Assignment")
32+
db_alias = schema_editor.connection.alias
33+
Assignment.objects.using(db_alias).update(language="P")
34+
35+
36+
# fmt: off
37+
class Migration(migrations.Migration):
38+
39+
dependencies = [
40+
('assignments', '0033_submissioncap_submissioncap_unique_type_and_more'),
41+
]
42+
43+
operations = [
44+
migrations.CreateModel(
45+
name='Language',
46+
fields=[
47+
('id', models.BigAutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')),
48+
('is_deprecated', models.BooleanField(default=False)),
49+
('language', models.CharField(choices=[('P', 'Python 3'), ('J', 'Java')], max_length=1)),
50+
('name', models.CharField(help_text='The name of the language', max_length=50)),
51+
('info', models.JSONField()),
52+
],
53+
options={'ordering': ['-language', "-info__version"]},
54+
55+
),
56+
migrations.AddField(
57+
model_name="assignment",
58+
name="language_details",
59+
field=models.ForeignKey(
60+
null=True,
61+
on_delete=models.deletion.CASCADE,
62+
related_name="assignment_set",
63+
to="assignments.language",
64+
),
65+
),
66+
migrations.RunPython(migrate_to_foreignkey, revert_default_language),
67+
migrations.AlterField(
68+
model_name="assignment",
69+
name="language_details",
70+
field=models.ForeignKey(
71+
null=False,
72+
on_delete=models.deletion.CASCADE,
73+
related_name="assignment_set",
74+
to="assignments.language",
75+
),
76+
),
77+
migrations.RemoveField(
78+
model_name='assignment',
79+
name='language',
80+
),
81+
]

tin/apps/assignments/models.py

Lines changed: 57 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,12 @@
44
import logging
55
import os
66
import subprocess
7+
from pathlib import Path
78
from typing import Literal
89

910
from django.conf import settings
1011
from django.contrib.auth import get_user_model
12+
from django.core.exceptions import ValidationError
1113
from django.core.validators import MinValueValidator
1214
from django.db import models, transaction
1315
from django.db.models import Q
@@ -95,10 +97,10 @@ def filter_editable(self, user):
9597
def upload_grader_file_path(assignment, _): # pylint: disable=unused-argument
9698
"""Get the location of the grader file for an assignment"""
9799
assert assignment.id is not None
98-
if assignment.language == "P":
99-
return f"assignment-{assignment.id}/grader.py"
100-
else:
100+
if assignment.grader_language == "J":
101101
return f"assignment-{assignment.id}/Grader.java"
102+
else:
103+
return f"assignment-{assignment.id}/grader.py"
102104

103105

104106
class Assignment(models.Model):
@@ -122,11 +124,12 @@ class Assignment(models.Model):
122124
description = models.CharField(max_length=4096)
123125
markdown = models.BooleanField(default=False)
124126

125-
LANGUAGES = (
126-
("P", "Python 3"),
127-
("J", "Java"),
127+
language_details = models.ForeignKey(
128+
"Language",
129+
on_delete=models.CASCADE,
130+
related_name="assignment_set",
131+
null=False,
128132
)
129-
language = models.CharField(max_length=1, choices=LANGUAGES, default="P")
130133

131134
filename = models.CharField(max_length=50, default="main.py")
132135

@@ -195,6 +198,11 @@ def __repr__(self):
195198
def get_absolute_url(self):
196199
return reverse("assignments:show", args=(self.id,))
197200

201+
@property
202+
def grader_language(self) -> Literal["P", "J"]:
203+
"""The language of the assignment (Python or Java)"""
204+
return self.language_details.language
205+
198206
def within_submission_limit(self, student) -> bool:
199207
"""Check if a student is within the submission limit for an assignment."""
200208
# teachers should have infinite submissions
@@ -262,6 +270,14 @@ def make_assignment_dir(self) -> None:
262270
assignment_path = os.path.join(settings.MEDIA_ROOT, f"assignment-{self.id}")
263271
os.makedirs(assignment_path, exist_ok=True)
264272

273+
def grader_exists(self) -> bool:
274+
"""Check if a grader file exists."""
275+
if self.grader_file is None or self.grader_file.name is None:
276+
return False
277+
278+
fpath = Path(settings.MEDIA_ROOT) / self.grader_file.name
279+
return fpath.exists()
280+
265281
def save_grader_file(self, grader_text: str) -> None:
266282
"""Save the grader file to the correct location.
267283
@@ -773,3 +789,37 @@ def run(self, assignment: Assignment):
773789

774790
assignment.last_action_output = output
775791
assignment.save()
792+
793+
794+
class Language(models.Model):
795+
"""Which version of a language is used for an assignment."""
796+
797+
LANGUAGES = (
798+
("P", "Python 3"),
799+
("J", "Java"),
800+
)
801+
802+
name = models.CharField(max_length=50, help_text="The name of the language")
803+
info = models.JSONField()
804+
language = models.CharField(max_length=1, choices=LANGUAGES)
805+
is_deprecated = models.BooleanField(default=False)
806+
807+
class Meta:
808+
ordering = ["-language", "-info__version"]
809+
810+
def __str__(self) -> str:
811+
return self.name
812+
813+
def __repr__(self) -> str:
814+
return f"<{type(self).__name__}: {self.name} ({self.language}) >"
815+
816+
def clean(self) -> None:
817+
super().clean()
818+
if not isinstance(self.info, dict):
819+
raise ValidationError("Info must be a dictionary")
820+
if self.language == "P" and (
821+
"python3" not in self.info or not isinstance(self.info["version"], float)
822+
):
823+
raise ValidationError(
824+
"Python 3 language must have a 'python3' key and a (float) version key!"
825+
)

tin/apps/assignments/tests/test_assignments.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,11 +10,10 @@
1010

1111

1212
@login("teacher")
13-
def test_create_assignment(client, course) -> None:
13+
def test_create_assignment(client, course, python) -> None:
1414
data = {
1515
"name": "Write a Vertex Shader",
1616
"description": "See https://learnopengl.com/Getting-started/Shaders",
17-
"language": "P",
1817
"filename": "vertex.glsl",
1918
"points_possible": "300",
2019
"due": "04/16/2025",
@@ -24,6 +23,7 @@ def test_create_assignment(client, course) -> None:
2423
"submission_limit_cooldown": "30",
2524
"is_quiz": False,
2625
"quiz_action": "2",
26+
"language_details": python.id,
2727
}
2828
response = client.post(
2929
reverse("assignments:add", args=[course.id]),

tin/apps/assignments/tests/test_grader.py

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,3 +48,8 @@ def test_download_grader(client, assignment):
4848

4949
assert response.status_code == 200
5050
assert response.content.decode("utf-8") == code
51+
52+
53+
def test_grader_save_file(assignment):
54+
assignment.save_grader_file("print('hello, world')")
55+
assert assignment.grader_exists()

tin/apps/assignments/views.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -286,8 +286,9 @@ def edit_view(request, assignment_id):
286286
)
287287

288288
course = assignment.course
289+
initial = {"language_details": assignment.language_details}
289290

290-
assignment_form = AssignmentForm(course, instance=assignment)
291+
assignment_form = AssignmentForm(course, instance=assignment, initial=initial)
291292
if request.method == "POST":
292293
assignment_form = AssignmentForm(course, data=request.POST, instance=assignment)
293294
if assignment_form.is_valid():

tin/apps/submissions/models.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ def filter_editable(self, user):
5252
def upload_submission_file_path(submission, _) -> str: # pylint: disable=unused-argument
5353
"""Get the path to a submission"""
5454
assert submission.assignment.id is not None
55-
if submission.assignment.language == "P":
55+
if submission.assignment.grader_language == "P":
5656
return "assignment-{}/{}/submission_{}.py".format(
5757
submission.assignment.id,
5858
slugify(submission.student.username),

0 commit comments

Comments
 (0)