Skip to content

Restore Assignment 1 SQL execution feedback alongside Assignment 2 - #492

Closed
danielrazavi wants to merge 1 commit into
mainfrom
restore-a1-marking
Closed

danielrazavi wants to merge 1 commit into
mainfrom
restore-a1-marking

Conversation

@danielrazavi

Copy link
Copy Markdown
Member

What changes are you trying to make?

Restore Assignment 1 SQL execution feedback alongside the existing Assignment 2 grader. October 9 commit 3fe0023c0af279b275f746948ad08134a631a25f replaced the old shared runner. This restores A1 in a separate workflow and runner, leaving all existing A2 files unchanged.

  • Automatic A1 feedback for changed assignment1.sql files on student-owned repositories, on assignment-one / assignment-1 and A2 branch aliases. Mixed A1/A2 submissions on A2 branches run both workflows independently.
  • Separate A1 report, artifact, and PR feedback, plus manual branch/PR inputs. Multiple cohort files are rejected rather than selecting one arbitrarily.
  • Restore historical multistatement execution and three-row previews; handle missing answers, malformed markers, fresh failure reports, quoted semicolons, and bounded query execution on an isolated database.

What did you learn from the changes?

The March 24–April 5 runner (cf333a0, 0349f73, 3445382, f7e32c2, ae700d8) executed SQL and displayed results; it did not check correctness or award marks. Immediately before October 9, it was manual-only and prioritized A2 over A1 when both files changed. Restored feedback explicitly labels executable SQL as requiring instructor review.

Was there another approach?

A separate A1 workflow avoids modifying the current A2 workflow, grading rules, regression tests, or report format. Shared-workflow routing would have required changing current A2 behavior.

Challenges and conflicts

No merge conflicts against main at cf65aa97590eba771031c052e08db3b62f5b5d8f. This is a new branch adding four files; no current files are overwritten.

Automatic runs retain A2's student-repository owner restriction; upstream UofT-DSI use is manual. A2's existing branch restrictions are unchanged, so A2 does not automatically run from an A1 branch. GitHub token restrictions can prevent PR comments; manual runs with appropriate permissions are the fallback. The selected submission ref must include the restored runner/tests, and manual selection requires an unmerged assignment1.sql change against main.

How were these changes tested?

  • Local regression suite: 14 passed, 14 skipped (7 new A1 checks and 7 available A2 checks; 14 existing A2 tests require A2_SOLUTION_RUBRIC, which was unavailable).
  • Both checked-in A1 cohort templates produced 12-question reports.
  • Existing A2 runner end-to-end check produced a complete 12-question report; its pytest invocation passed.
  • New workflow YAML parsed; embedded feedback script tested for normal, malformed-report, and oversized-feedback cases.
  • Database fingerprints verified unchanged; external attachment denied; long SQL interrupted; mixed A1/A2 changed-file selection verified, including a path containing spaces.
  • git diff --check passed. Local tests used Python 3.9.6; workflow specifies Python 3.11. GitHub-hosted execution and actual PR comment delivery have not been exercised.

Related task

Urgent Trello restoration task

See .github/scripts/assignment1-feedback.md for provenance, usage, and limitations.

Checklist

  • Local checks confirm restored execution feedback and unchanged A2 behavior within the available tests.
  • Verify a student-repository GitHub Actions run and PR feedback delivery before deployment.

@github-actions

Copy link
Copy Markdown

Thanks for your contribution! 🎉

Please remember to tag or request a review from the DSI team. Give us up to 72 hours to review your pull request. We appreciate your patience and efforts.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant