Skip to content

fix(cron): import run_job inside _run_cron_tracked to fix NameError - #1317

Closed
jasonjcwu wants to merge 1 commit into
nesquena:masterfrom
jasonjcwu:fix/cron-run-job-nameerror
Closed

jasonjcwu wants to merge 1 commit into
nesquena:masterfrom
jasonjcwu:fix/cron-run-job-nameerror

Conversation

@jasonjcwu

Copy link
Copy Markdown
Contributor

Problem

_run_cron_tracked() runs inside a worker thread (threading.Thread) at api/routes.py. It called run_job(job) but run_job was only imported as a local variable inside _handle_cron_run() — a completely separate function scope. When the thread executed, run_job was not in scope → NameError.

This caused "Run Now" in the cron panel to crash every time.

Fix

Moved from cron.scheduler import run_job into _run_cron_tracked() itself so the import resolves in the worker thread's scope. Removed the now-redundant import from _handle_cron_run().

Files changed:

  • api/routes.py — moved import into _run_cron_tracked, removed from _handle_cron_run
  • tests/test_cron_run_job_import.py — 3 AST-based regression tests

Testing

…esquena#1312)

_run_cron_tracked() runs inside a worker thread (threading.Thread), so it
cannot see the caller's local import of run_job from _handle_cron_run.
Moved the import into _run_cron_tracked and removed the now-redundant
import from the route handler.

Fixes nesquena#1312, Fixes nesquena#1310
@jasonjcwu

Copy link
Copy Markdown
Contributor Author

Fixes #1312 and Fixes #1310

@nesquena-hermes

Copy link
Copy Markdown
Collaborator

Thanks for the fix and especially for the regression tests — the AST-based check that run_job is actually imported inside _run_cron_tracked is a nice safety net for this exact category of bug.

Confirmation

Diagnosis is correct, and matches what's in master:

  • api/routes.py:95 — _run_cron_tracked() calls run_job(job) but has no import.
  • api/routes.py:3503 — _handle_cron_run() imports run_job at function-local scope. That import is invisible to _run_cron_tracked, which executes in a worker thread spawned at line 3514 (threading.Thread(target=_run_cron_tracked, ...)).

Result: NameError: name 'run_job' is not defined whenever a manual cron run is dispatched. The diff resolves both halves cleanly — local import added to _run_cron_tracked, redundant import removed from _handle_cron_run.

Overlap with #1312

This PR overlaps with the existing #1312 (same one-line import fix, same hunk). The substantive differences in your PR:

  1. You also remove the now-redundant from cron.scheduler import run_job from _handle_cron_run(). That cleanup is correct (the import is no longer needed there) and is a small improvement fix(cron): import run_job in _run_cron_tracked to fix NameError #1312 doesn't have.
  2. You add three regression tests in tests/test_cron_run_job_import.py. The AST-based approach is robust against future refactors and exactly the kind of test that would have caught this defect at PR-review time.

A small test-code note (non-blocking)

In TestRunCronTrackedImport.test_run_job_imported_inside_function there's some dead code:

ImportCollector = type(
    "ImportCollector",
    (ast.NodeVisitor,),
    {
        "imports": set(),
        "visit_ImportFrom": lambda self, node: (
            self.imports.add(a.name for a in node.names),
        ),
    },
)

This class is constructed but never instantiated or visited — the actual import collection happens in the loop below. You can drop the entire ImportCollector block; the test still passes and is easier to read. Also note set.add(<generator>) would put a generator object in the set rather than the names, so even if it were used it wouldn't behave as intended.

Recommendation

Either of #1312 or #1317 fixes the runtime bug. This PR is the more complete one (cleanup + tests). I'd suggest:

Maintainer will pick. Thanks for the careful work — the regression tests in particular are valuable.

🤖 Automated triage via nesquena-hermes

@nesquena-hermes

Copy link
Copy Markdown
Collaborator

Shipped in v0.50.245 via batch release PR #1334 (merge commit 52e1567). Thanks @fxd-jason! 🙏

Live now at https://github.com/nesquena/hermes-webui/releases/tag/v0.50.245.

Released alongside 9 other contributor fixes — see the v0.50.245 entry in CHANGELOG.md.

bsgdigital pushed a commit to bsgdigital/hermes-webui that referenced this pull request Apr 30, 2026
GeoffBao pushed a commit to GeoffBao/hermes-webui that referenced this pull request May 1, 2026
SysAdminDoc pushed a commit to SysAdminDoc/hermes-webui that referenced this pull request Jun 26, 2026
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.

3 participants