-
Notifications
You must be signed in to change notification settings - Fork 6
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
ExtractArchiveJob: Fixes job not stopping when cancelled by user, also changes state to use "cancelled" instead of "terminated" #2008
Conversation
Fixes: SIRI-972
Fixes: SIRI-972
If it was cancelled we set the state to "cancelled" instead of "terminated" Fixes: SIRI-972
@@ -599,7 +599,12 @@ protected boolean markCompleted(String processId, | |||
return modify(processId, process -> process.getState() != ProcessState.TERMINATED, process -> { | |||
if (process.getState() != ProcessState.STANDBY) { | |||
process.setErrorneous(process.isErrorneous() || !TaskContext.get().isActive()); | |||
process.setState(ProcessState.TERMINATED); | |||
//Do not alter the job state if the job was previously cancelled by the user | |||
if (process.getState() != ProcessState.CANCELED) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
on a second thought, it will be a bit more robust if we specifically check for cancelations.
If the job was cancelled, both date and status will be already set.
if (process.getCancelled() == null) {
process.setState(ProcessState.TERMINATED);
process.setCompleted(LocalDateTime.now());
}
OR
if (process.getState() != ProcessState.CANCELED) {
process.setState(ProcessState.TERMINATED);
process.setCompleted(LocalDateTime.now());
}
process.setState(ProcessState.TERMINATED); | ||
} else { | ||
process.setState(ProcessState.CANCELED); | ||
} | ||
process.setCompleted(LocalDateTime.now()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moved in the if above as the date goes along with the status
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(subject to the suggestions by @idlira)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Optional: Could we only set the state message "Das Archiv wurde vollständig entpackt" when the job wasn't cancelled, too? In my opinion showing this is a bit misleading, as not all contains were extracted in that case.
Fixes: SIRI-972
Fixes: SIRI-972
Fixes: SIRI-972
Fixes: SIRI-972
Description
Before:
Still unpacking:
After unpacking:
After fixes:
Additional Notes
Checklist