Skip to content

[cuebot] Reject job specs referencing undefined limits - #2514

Open
DiegoTavares wants to merge 1 commit into
AcademySoftwareFoundation:masterfrom
DiegoTavares:joblaunch_missing_limits
Open

[cuebot] Reject job specs referencing undefined limits#2514
DiegoTavares wants to merge 1 commit into
AcademySoftwareFoundation:masterfrom
DiegoTavares:joblaunch_missing_limits

Conversation

@DiegoTavares

@DiegoTavares DiegoTavares commented Aug 28, 2026

Copy link
Copy Markdown
Collaborator

A job spec that referenced a limit which had never been created failed deep inside createJob with a raw EmptyResultDataAccessException. On the async launchSpec path the RPC returned success and the failure was only reported by email, while launchSpecAndWait surfaced a misleading "Incorrect result size: expected 1, actual 0".

Validate limits in JobLauncher.verifyJobSpec instead, so both launch paths reject the spec before any job is created and the error names every missing limit along with the layers using it. JobManagerService keeps a fail-closed guard for direct createJob callers and for limits deleted between verification and insert.

Summary by CodeRabbit

  • Bug Fixes
    • Job launches now reject specifications that reference undefined limits.
    • Error messages identify missing limits and the layers that use them.
    • Direct job creation also fails safely when referenced limits do not exist.
    • Duplicate limit names are consolidated while preserving their original order.

A job spec that referenced a limit which had never been created failed
deep inside createJob with a raw EmptyResultDataAccessException. On the
async launchSpec path the RPC returned success and the failure was only
reported by email, while launchSpecAndWait surfaced a misleading
"Incorrect result size: expected 1, actual 0".

Validate limits in JobLauncher.verifyJobSpec instead, so both launch
paths reject the spec before any job is created and the error names
every missing limit along with the layers using it. JobManagerService
keeps a fail-closed guard for direct createJob callers and for limits
deleted between verification and insert.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013vjCKwvKgD59YNdHnj25nA
Entire-Checkpoint: 550498d6ea46
@coderabbitai

coderabbitai Bot commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 3e099717-59d5-486c-8b78-3c18d8936219

📥 Commits

Reviewing files that changed from the base of the PR and between 9bd87bb and 4afe35b.

📒 Files selected for processing (8)
  • cuebot/src/main/java/com/imageworks/spcue/dao/LimitDao.java
  • cuebot/src/main/java/com/imageworks/spcue/dao/postgres/LimitDaoJdbc.java
  • cuebot/src/main/java/com/imageworks/spcue/service/AdminManager.java
  • cuebot/src/main/java/com/imageworks/spcue/service/AdminManagerService.java
  • cuebot/src/main/java/com/imageworks/spcue/service/JobLauncher.java
  • cuebot/src/main/java/com/imageworks/spcue/service/JobManagerService.java
  • cuebot/src/test/java/com/imageworks/spcue/test/dao/postgres/LimitDaoTests.java
  • cuebot/src/test/java/com/imageworks/spcue/test/service/JobLauncherLimitTests.java

Included review availability: Your plan provides up to 4 included reviews per hour; 3 remain after this review.


📝 Walkthrough

Walkthrough

The change adds a missing-limit lookup through the DAO and admin service layers. Job launch validation now reports undefined limits and their referencing layers. Direct job creation rejects missing limits before linking layer limits.

Changes

Limit validation

Layer / File(s) Summary
Missing-limit lookup
cuebot/src/main/java/com/imageworks/spcue/dao/LimitDao.java, cuebot/src/main/java/com/imageworks/spcue/dao/postgres/LimitDaoJdbc.java, cuebot/src/test/java/com/imageworks/spcue/test/dao/postgres/LimitDaoTests.java
The DAO returns unique missing limit names in input order. JDBC lookup handles empty input and queries matching records. Tests cover existing, missing, and duplicate names.
Admin service delegation
cuebot/src/main/java/com/imageworks/spcue/service/AdminManager.java, cuebot/src/main/java/com/imageworks/spcue/service/AdminManagerService.java
The admin service exposes findMissingLimitNames and delegates the lookup to LimitDao.
Job limit enforcement
cuebot/src/main/java/com/imageworks/spcue/service/JobLauncher.java, cuebot/src/main/java/com/imageworks/spcue/service/JobManagerService.java, cuebot/src/test/java/com/imageworks/spcue/test/service/JobLauncherLimitTests.java
JobLauncher validates limits across jobs and post jobs. JobManagerService validates layer limits before linking them. Spring tests cover rejection, error reporting, successful launches, persistence, and direct job creation.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Merge Risk: ⚪ Minimal · up to 4afe3

The change rejects job specs with undefined limits before job creation and preserves a fail-closed guard for direct or concurrent callers; no actionable merge-blocking risk remains after normal checks and review.

Sequence Diagram(s)

sequenceDiagram
  participant JobLauncher
  participant AdminManagerService
  participant LimitDaoJdbc
  participant limit_record
  JobLauncher->>AdminManagerService: findMissingLimitNames(limit names)
  AdminManagerService->>LimitDaoJdbc: findMissingLimitNames(limit names)
  LimitDaoJdbc->>limit_record: query existing limit names
  limit_record-->>LimitDaoJdbc: matching names
  LimitDaoJdbc-->>AdminManagerService: missing names
  AdminManagerService-->>JobLauncher: missing names
  JobLauncher-->>JobLauncher: reject spec with referencing layers
Loading

Suggested reviewers: lithorus, ramonfigueiredo

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 22.22% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 27 functions across 8 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: Cuebot rejects job specifications that reference undefined limits.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@DiegoTavares
DiegoTavares marked this pull request as ready for review August 28, 2026 23:37
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