Skip to content

[Refactor:System] Daemon job queue path from config#12558

Merged
bmcutler merged 2 commits intomainfrom
fix/daemon-job-queue-path-consistency
Mar 11, 2026
Merged

[Refactor:System] Daemon job queue path from config#12558
bmcutler merged 2 commits intomainfrom
fix/daemon-job-queue-path-consistency

Conversation

@prestoncarman
Copy link
Contributor

@prestoncarman prestoncarman commented Mar 10, 2026

Why is this Change Important & Necessary?

Several PHP controllers hardcoded /var/local/submitty/daemon_job_queue/ as a string literal rather than deriving the path from configuration. This means the path is duplicated across the codebase and would silently break if submitty_data_dir is ever configured to a non-default location. AdminGradeableController::enqueueGenerateRepos even had an existing FIXME comment acknowledging the problem.

What is the New Behavior?

All daemon job queue paths are now constructed using FileUtils::joinPaths($this->core->getConfig()->getSubmittyPath(), "daemon_job_queue"), consistent with the pattern already used in SubmissionController. The static method enqueueGenerateRepos now accepts a $submittyPath parameter, and all five call sites have been updated to pass $config->getSubmittyPath().

Files changed:

  • HomePageController.php — course creation job
  • ReportController.php — rainbow grades enqueue and status check
  • AdminGradeableController.php — build config job and generate repos static method
  • MiscController.php — bulk upload progress check
  • UsersController.php, SubmissionController.php, TeamController.php, Gradeable.php — callers of enqueueGenerateRepos

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

  • Create a new course and confirm the creation job appears in the daemon job queue
  • Trigger a rainbow grades run and confirm the job file is created and processed
  • Create or edit a gradeable with an autograding config and confirm the build config job is enqueued
  • Upload a bulk PDF submission and confirm the bulk progress endpoint responds correctly
  • Create a VCS gradeable and confirm the repo generation job is enqueued

Automated Testing & Documentation

No new automated tests added. The changed code paths are covered by existing integration tests that exercise course creation, gradeable building, and submission. No documentation changes required — this is an internal refactor with no user-visible behavior change.

Other information

Not a breaking change. No migrations required. No security concerns — the change only affects how the path is constructed, not what the path resolves to on a standard installation.

Generated with Claude Code

Replace hardcoded /var/local/submitty/daemon_job_queue/ string literals
with FileUtils::joinPaths($this->core->getConfig()->getSubmittyPath(), "daemon_job_queue")
across all PHP controllers and models.

Also resolves the FIXME in AdminGradeableController::enqueueGenerateRepos
by adding a $submittyPath parameter to the static method and updating all
five call sites.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@github-actions
Copy link

Hi @prestoncarman,

Thank you for contributing to Submitty by opening a pull request!

Please check that your PR follows our guidelines for How to Make a Pull Request and that you have provided the information requested in our pull request template. If your pull request is incomplete, insufficiently documented, or untested it may be closed by maintainers.

We encourage you to join our Zulip server to discuss your PR, bug reports, and technical implementation questions with other contributers and maintainers.

Click here to view the CI run for this branch

@prestoncarman prestoncarman changed the title [Refactor:System] Use config-derived path for daemon job queue [Refactor:System] Daemon job queue path from config Mar 10, 2026
@codecov
Copy link

codecov bot commented Mar 10, 2026

Codecov Report

❌ Patch coverage is 0% with 18 lines in your changes missing coverage. Please review.
✅ Project coverage is 21.66%. Comparing base (ae76ef5) to head (f70f956).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff            @@
##               main   #12558   +/-   ##
=========================================
  Coverage     21.66%   21.66%           
  Complexity     9638     9638           
=========================================
  Files           268      268           
  Lines         36233    36226    -7     
  Branches        486      486           
=========================================
  Hits           7849     7849           
+ Misses        27902    27895    -7     
  Partials        482      482           
Flag Coverage Δ
autograder 21.39% <ø> (ø)
js 2.04% <ø> (ø)
migrator 100.00% <ø> (ø)
php 20.68% <0.00%> (+<0.01%) ⬆️
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.

@github-project-automation github-project-automation bot moved this from Seeking Reviewer to Awaiting Maintainer Review in Submitty Development Mar 11, 2026
@bmcutler bmcutler merged commit 738fe3d into main Mar 11, 2026
25 checks passed
@bmcutler bmcutler deleted the fix/daemon-job-queue-path-consistency branch March 11, 2026 18:45
@github-project-automation github-project-automation bot moved this from Awaiting Maintainer Review to Done in Submitty Development Mar 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

3 participants