Allow for other campuses to claim expired jobs. #44
Conversation
| // Throw an exception | ||
| throw IllegalStateException("Attempted to submit a job that would overwrite an existing job owned by another campus (${job.jobID})") | ||
| throw IllegalStateException("Attempted to submit a job that would overwrite an existing, non-expired job owned by another campus (${job.jobID})") |
There was a problem hiding this comment.
So, this is an exception here because we should already have returned a non-new, non-expired status in the caller, correct?
There was a problem hiding this comment.
I'm not sure I understand the question. This is here to prevent us from starting a non-expired job that is owned by another campus. The caller should verify that submitting a job is a sane action before calling this method, likely by calling getJob() and at least ensuring job ownership (or non-existence).
Maybe this logic should be pulled out to another method like canSubmitJob(jobID): Boolean?
Right now it would be something like:
val job = AsyncPlatform.getJob(...)
if (job == null || job.isOwned)
AsyncPlatform.submitJob(...)
else
throw ForbiddenException("refusing to overwrite job owned by another campus")|
|
||
| // If the job already exists | ||
| if (exists) | ||
| if (existingJob != null && existingJob.owned) | ||
| // Reset the job status to queued and update the queue name | ||
| QueueDB.markJobAsQueued(job.jobID, queue) |
There was a problem hiding this comment.
Curious about this line. Why would we mark as queued here?
There was a problem hiding this comment.
Oh, so this covers our expired and failed cases where we want to restart?
There was a problem hiding this comment.
Uh oh. I think the else below this is not doing what we think any more because of the added conditional? Like if job exists but is not owned, we don't want to add it right? Maybe only if it is expired? Or is that case handled above this call?
There was a problem hiding this comment.
If it exists, and was previously owned by this campus, then there is no need to re-create the job records so we just update the existing record to say queued.
If it doesn't exist, or was not previously owned by this campus, we have to actually create the job record
Description
Allow for other campuses to claim expired jobs.
Tested locally by
Changes
PR Checklist