-
Notifications
You must be signed in to change notification settings - Fork 13
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
[teraslice] - Add ability to soft delete a job #3711
Conversation
|
|
|
ba9e2b9
to
e06dd26
Compare
208d26e
to
2db646a
Compare
bump: (minor) [email protected], [email protected] bump: (minor) @terascope/[email protected], @terascope/[email protected] bump: (minor) @terascope/[email protected], @terascope/[email protected] bump: (minor) [email protected], [email protected] bump: (minor) [email protected], [email protected] bump: (minor) [email protected], @terascope/[email protected]
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.
Please, at the very least make the requested doc change. The e2e changes are optional.
e2e/test/cases/cluster/api-spec.ts
Outdated
await terasliceHarness.teraslice.jobs.post(`/jobs/${jobId}/_stop`); | ||
await terasliceHarness.waitForExStatus(ex, 'stopped', 100, 1000); | ||
|
||
await expect(terasliceHarness.teraslice.jobs.delete(`/jobs/${jobId}`)).resolves.toMatchObject(deletedJobProperties); |
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.
At this point, the job should not show up in /txt/jobs
, if there's an easy way to test that, it might be worth doing.
You can test any of the scenarios that being deleted causes, like no updating or starting.
@@ -563,6 +572,33 @@ $ curl 'localhost:5678/v1/jobs/5a50580c-4a50-48d9-80f8-ac70a00f3dbd/errors' | |||
] | |||
``` | |||
|
|||
## DELETE /v1/jobs/\{jobId\}; | |||
|
|||
Issues a delete command, deleting the job and all related execution contexts. Any orphaned K8s resources associated with the job will also be deleted. The job must have a terminal status to be deleted. The `active` field will automatically be set to `false`. |
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.
Please extend this with the consequences of deleting a job:
A delete job cannot be updated ....
LGTM |
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.
LGTM
This PR makes the following changes:
v1routes.delete('/jobs/:jobId')
endpointjobsService
to throw when trying to start, update, or recover a deleted job/jobs
,/txt/jobs
,/ex
, and/txt/ex
endpoints to filter bydeleted
teraslice-cli jobs delete
andteraslice-cli tjm delete
commands and necessary helper functions.teraslice-cli jobs list
to adddeleted
andactive
flagsteraslice-cli jobs ex
to adddeleted
flagteraslice-cli helpers.ts
andteraslice-client-js Jobs.list() to remove
status` query. Status inside a job was removed in job refactor #303. Fix Jobs.list in teraslice-client-js and remove jobs method #880 may also be relevant.ref: #3680