Conversation
/project/delete was returning 204 when content was expected
…ished job_task statuses to cancelled when task is cancelled
…k not passing cancelled count to resolve_job_status()
client/src/pages/ProjectPage.tsx
Outdated
| completedCount={jobToCancel.completedCount} | ||
| totalCount={jobToCancel.totalCount} | ||
| onClose={() => setJobToCancel(null)} | ||
| onKeepData={async () => { |
There was a problem hiding this comment.
Using pure async functions as React component props are not best practice, instead x.then().catch() pattern should be used. useCallback could memoize the functions to optimize rendering. Also, see useTransition (https://react.dev/reference/react/useTransition) if it could also be used.
| completedCount: number; | ||
| totalCount: number; | ||
| onClose: () => void; | ||
| onKeepData: () => void; |
There was a problem hiding this comment.
The type here should use Promise<void>, as async functions were passed to the prop. As a better alternative, the prop should still use type () => void but triggering the async prop done in a different way.
client/src/pages/ProjectPage.tsx
Outdated
| import { FileDropArea } from "../components/FileDropArea"; | ||
| import { ExpandableToast } from "../components/ExpandableToast"; | ||
| import { TruncatedFileNames } from "../components/TruncatedFileNames"; | ||
| import { ConrimationModal } from "../components/ConfirmationModal"; |
There was a problem hiding this comment.
Conrimation -> Confirmation
| async (actions, { jobUuid, projectUuid }, { injections }) => { | ||
| const { jobService } = injections; | ||
| await jobService.cancelJob(jobUuid); | ||
| await actions.fetchJobsForProject(projectUuid); // TODO: Maybe no fetch here |
There was a problem hiding this comment.
You are right. Optimally fetching jobs could be done outside of the thunk. Also, using abortcontroller to cancel requests if the page unmounts could be a good idea. Can be done later on
server/src/celery/tasks.py
Outdated
| EventName.JOB_TASK_RUNNING, | ||
| {"job_task_id": job_task.id, "status": JobTaskStatus.RUNNING}, | ||
| ) | ||
| # await _publish_redis_event( |
There was a problem hiding this comment.
These can be removed from code if they are not needed anymore
2 participants
Adds the ability to cancel on going tasks:
/job/{uuid}/cancel?delete_data={'false' OR 'true'}. Cancellation happens in the frontend throughCancelJobModal.If
delete_data=trueis passed the whole job will be deleted after cancellation.The status of a
Jobis handled according to the status ofJobTasksrelated to it. So when a job is cancelled and data is not deleted all of the not-finishedJobTaskswill get a status ofCANCELLED. The state management is kind of complicated but having to somehow syncJobStatusandJobTaskStatuswould probably be even more complicated. Now it is only calculated on demand.Tasks are cancelled by revoking the celery task (
SIGTERM), meaning open db transactions and other processes will instantly fail. This will show in the db logs asunexpected EOF on client connection with an open transaction. Most likely isn't an issue but still something to investigate.