Non-Container Grading Path#45
Conversation
keiferms3
force-pushed
the
non-container-grading-path
branch
from
10fead1 to
e3c9dd5
Compare
jessehartloff
left a comment
jessehartloff
left a comment
There was a problem hiding this comment.
This is good. I left some comments, but they're mostly for future discussions and will be addressed when we expand this endpoint with container graders
|
|
||
| import { GenericResponse } from '../../utils/apiResponse.utils' | ||
|
|
||
| //THIS TEST FAILS, |
There was a problem hiding this comment.
?? Why write and commit a test that fails?
| * description: | ||
| * /grade/{id}: | ||
| * post: | ||
| * summary: Grade a submission, currently only with non-container autograders |
There was a problem hiding this comment.
Should make it clear the the id is a submission id
| } | ||
| allScores.push(await submissionProblemScoreService.create(problemScoreObj)) | ||
| } | ||
| } |
There was a problem hiding this comment.
This is fine for now, but it does assume that there's <=1 grader per question. In most cases this should be true, but we shouldn't rely on it. When we expand to Container graders we can update
| score += result.score | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
right, this is just for future use/modification
| const scoreObj: SubmissionScore = { | ||
| submissionId: id, | ||
| score: score, | ||
| feedback: '' //graders currently don't return feedback, will be the concatination of SubmissionProblemScore feedbacks |
There was a problem hiding this comment.
Future: Should NonContainerAutoGraders provide feedback? There's no obvious answer for what it would return. For multiple choice, the instructor can add feedback for each answer, but for any open-ended question it gets harder. Maybe instructors can give a "feedback if correct" and "feedback if incorrect"
| const submissionScore = scores.pop() | ||
| return { | ||
| submissionScore: submissionScore as SubmissionScore, | ||
| submissionProblemScores: scores as SubmissionProblemScore[] |
There was a problem hiding this comment.
-Needs further discussion-
I don't like this at all. If I'm reading things right, your pushing the submission score as the first element of the array and the rest of the array is the problem scores. This needs to be cleaner and clearer.
SubmissionScore and SubmissionProblemScore already have serializers. If anyone wants those values, they should hit the existing endpoints and use that logic.
The real question is, what should the grading endpoint return? Should it return anything besides an ACK?.. Grading will take a while when we start using containers. The best thing might be to immediately return something like "got your request and we're starting grading"
2 participants
Implemented POST /grade/{submission-id} path. Currently only runs non-container graders on the submission's content, though the endpoint can be modified later to support both kinds of graders. As of making this PR tests have not yet been written and the grader's return type needs to be properly defined rather than just hijacking the model file. I intend to finish these by end of day friday, just wanted to get the code review started as the intended functionality is otherwise there.
Resolves #40