Skip to content

[Feature:System] Validate autograding_workers.json schema#12559

Merged
bmcutler merged 3 commits into
mainfrom
feature/validate-autograding-config-schemas
Jun 4, 2026
Merged

[Feature:System] Validate autograding_workers.json schema#12559
bmcutler merged 3 commits into
mainfrom
feature/validate-autograding-config-schemas

Conversation

@prestoncarman

@prestoncarman prestoncarman commented Mar 10, 2026

Copy link
Copy Markdown
Contributor

Why is this Change Important & Necessary?

A typo or missing field in `autograding_workers.json` currently causes a cryptic `KeyError` or `TypeError` deep inside the shipper when the first job is submitted — not at startup. This makes misconfiguration difficult to diagnose, especially for administrators adding or reconfiguring a worker machine for the first time.

What is the New Behavior?

`load_autograding_workers_json()` now validates the parsed JSON against a schema immediately after loading. If the file is invalid, the shipper exits with a clear, actionable error at startup. For example:

  • Missing field: `ERROR: autograding_workers.json is invalid: 'num_autograding_workers' is a required property (path: ['primary'])`
  • Wrong type: `ERROR: autograding_workers.json is invalid: 'five' is not of type 'integer' (path: ['primary', 'num_autograding_workers'])`
  • Typo in key name: `ERROR: autograding_workers.json is invalid: Additional properties are not allowed ('capabilites' was unexpected) (path: ['primary'])`

The schema enforces that every worker entry has: `address` (string), `username` (string), `num_autograding_workers` (integer ≥ 1), `enabled` (boolean), and `capabilities` (non-empty array of strings). `jsonschema` is already a pinned dependency (`jsonschema==4.26.0` in `.setup/pip/system_requirements.txt`), so no new dependency is introduced.

What steps should a reviewer take to reproduce or test the bug or new feature?

  1. Edit `autograding_workers.json` to remove the `num_autograding_workers` field from the primary entry
  2. Restart the shipper: `sudo systemctl restart submitty_autograding_shipper`
  3. Before: shipper starts, accepts jobs, then fails with a `KeyError` when the first job is processed
  4. After: shipper exits immediately with a descriptive error identifying the missing field and which worker entry contains it

Automated Testing & Documentation

8 unit tests added to `autograder/tests/test_submitty_autograding_shipper.py` covering:

  • Valid config loads successfully
  • Missing `num_autograding_workers` raises `SystemExit` with field name in message
  • Missing `enabled` raises `SystemExit` with field name in message
  • Wrong type for `num_autograding_workers` raises `SystemExit`
  • `num_autograding_workers` of 0 raises `SystemExit`
  • Empty `capabilities` list raises `SystemExit`
  • Unknown/misspelled key raises `SystemExit` with the bad key name in message
  • Multiple valid worker entries load successfully

Tests use `tempfile` and `unittest.mock.MagicMock` — each is self-contained with no dependency on global state or test ordering. No documentation changes required.

Other information

Not a breaking change for correctly configured installations. No migrations required. No security concerns.


This change was written by Claude Code (claude-sonnet-4-6).

Add JSON schema validation for autograding_workers.json in
load_autograding_workers_json(). Misconfigured worker entries
(missing required fields, wrong types, unknown keys) now produce
a clear error at shipper startup rather than a cryptic failure
when the first job is submitted.

jsonschema is already a pinned project dependency.

Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
@codecov

codecov Bot commented Mar 10, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 21.66%. Comparing base (ae76ef5) to head (f8831be).
⚠️ Report is 21 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff            @@
##               main   #12559   +/-   ##
=========================================
  Coverage     21.66%   21.66%           
  Complexity     9638     9638           
=========================================
  Files           268      268           
  Lines         36233    36233           
  Branches        486      486           
=========================================
  Hits           7849     7849           
  Misses        27902    27902           
  Partials        482      482           
Flag Coverage Δ
autograder 21.39% <ø> (ø)
js 2.04% <ø> (ø)
migrator 100.00% <ø> (ø)
php 20.68% <ø> (ø)
python_submitty_utils 80.08% <ø> (ø)
submitty_daemon_jobs 91.13% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

prestoncarman and others added 2 commits March 9, 2026 21:13
Tests cover: valid config, missing required fields (num_autograding_workers,
enabled), wrong types, num_autograding_workers below minimum, empty
capabilities list, unknown/misspelled keys, and multiple worker entries.

Each test is self-contained using tempfile and unittest.mock.MagicMock,
with no dependency on global state or test ordering.

Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
@github-actions github-actions Bot added the Abandoned PR - Needs New Owner No activity on PR for more than 2 weeks -- seeking new owner to complete label Mar 26, 2026

@dandrecollins07-ctrl dandrecollins07-ctrl left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On this PR branch I tested invalid configs by removing 'num_autograding_workers', changing it to the wrong type and misspelling 'capabilities'. In each case the shipper failed at startup and logged a clear validation error.
After restoring the config, the shipper started normally again. Approving.

@github-project-automation github-project-automation Bot moved this from Seeking Reviewer to Awaiting Maintainer Review in Submitty Development Jun 4, 2026
@bmcutler
bmcutler merged commit bd7cb0b into main Jun 4, 2026
25 checks passed
@bmcutler
bmcutler deleted the feature/validate-autograding-config-schemas branch June 4, 2026 18:45
@github-project-automation github-project-automation Bot moved this from Awaiting Maintainer Review to Done in Submitty Development Jun 4, 2026
bmcutler added a commit that referenced this pull request Jul 10, 2026
### Why is this Change Important & Necessary?

The autograding workers schema introduced in PR #12559 is missing the
display_environment_variable field.
This causes the autograding shipper to fail.

### What is the New Behavior?

The field is added to the schema, and the shipper should now work.

---------

Co-authored-by: Barb Cutler <Barb Cutler>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Abandoned PR - Needs New Owner No activity on PR for more than 2 weeks -- seeking new owner to complete

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

3 participants