Add killswitch for tasks #140
Conversation
| @@ -70,6 +70,12 @@ func routes(h Handler, conf *config.Config) Routes { | |||
| Handler: h.GetFromSandbox("stderr"), | |||
| }, | |||
| Route{ | |||
| Name: "Kill", | |||
| Method: "POST", | |||
There was a problem hiding this comment.
Wouldn't DELETE /task/{taskId} make more sense here?
There was a problem hiding this comment.
I thought of it first, but it's not actually deleting the task, it'll still be viewable through the API, so semantically DELETE felt wrong
There was a problem hiding this comment.
DELETE should imho only be used when you're deleting the resource identified by the url. However in this case the task will still be in the database because we want to be able to track that it was killed with later GETs
There was a problem hiding this comment.
I believe DELETE can be used even if you're doing a soft delete on the resource, still being available for inspection by an admin (i.e. I could still be in the database). Although I think you're right about that in that case it shouldn't be available on GET /task/{taskId}. I'm fine with this.
| @@ -13,7 +13,8 @@ const ( | |||
| TaskState_TASK_LOST TaskState = "TASK_LOST" | |||
| TaskState_TASK_ERROR TaskState = "TASK_ERROR" | |||
| // Custom eremetic states | |||
| TaskState_TASK_QUEUED TaskState = "TASK_QUEUED" | |||
| TaskState_TASK_QUEUED TaskState = "TASK_QUEUED" | |||
| TaskState_TASK_TERMINATING TaskState = "TASK_TERMINATING" | |||
There was a problem hiding this comment.
Is there a reason for having two concepts for the same thing, e.g. kill vs terminate? This signals to me that there might be other reasons for the task to terminate than manually killing it.
Will the task be marked as finished when successfully terminated? I think it would be more helpful to the developer if you could see which tasks that were forcibly stopped.
There was a problem hiding this comment.
When it's successfully killed it ends up in TASK_LOST
TASK_KILLED is also used by mesos as a final state, and I intended this state to more log the state change that the kill command was received, but it is not yet terminated
There was a problem hiding this comment.
Agreed that we should keep the naming consistent.
I believe it will become TASK_KILLED but we should test that.
| return nil | ||
| } | ||
|
|
||
| _, err = s.driver.KillTask(&mesos.TaskID{Value: proto.String(taskId)}) |
There was a problem hiding this comment.
This would try to kill tasks even if they are not yet running (and likely fail) and it would then later become running when we get to it in the queue.
So it's not actually giving us a way to clear tasks from the queue that we are not able to launch due to bad resource specs like we wanted 🍶
| @@ -13,7 +13,8 @@ const ( | |||
| TaskState_TASK_LOST TaskState = "TASK_LOST" | |||
| TaskState_TASK_ERROR TaskState = "TASK_ERROR" | |||
| // Custom eremetic states | |||
| TaskState_TASK_QUEUED TaskState = "TASK_QUEUED" | |||
| TaskState_TASK_QUEUED TaskState = "TASK_QUEUED" | |||
| TaskState_TASK_TERMINATING TaskState = "TASK_TERMINATING" | |||
There was a problem hiding this comment.
Agreed that we should keep the naming consistent.
I believe it will become TASK_KILLED but we should test that.
alde
force-pushed
the
add-killswitch-for-tasks
branch
2 times, most recently
from
4a118ca to
847dd42
Compare
|
Rebased on master |
|
needs rebase 😅 |
alde
force-pushed
the
add-killswitch-for-tasks
branch
from
847dd42 to
736dd86
Compare
Use either the REST API or hermit to kill a task you no longer wish to
run.
POST /task/{taskID}/kill
hermit kill {taskID}
Add button in the GUI to kill a task
alde
force-pushed
the
add-killswitch-for-tasks
branch
from
736dd86 to
113c07b
Compare
|
rebased again |
Add possibility to kill tasks
Add button in GUI for killing tasks