-
Notifications
You must be signed in to change notification settings - Fork 137
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
Preempt/Cancel on executor #4022
Conversation
} | ||
|
||
cmd.Flags().StringSliceP("match-queues", "q", []string{}, "Cancel jobs on executor matching the specified queue names. If no queues are provided, jobs across all queues will be cancelled.") | ||
return cmd |
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.
Suggest
- rename
--match-queues
to just--queues
- for consistency, if
--queues
requires a command line argument, then priority classes should require a--priority-classes
arg. - comment should specify what the delimiter is if you want to cancel multiple queues
|
||
return a.PreemptOnExecutor(onExecutor, matchQueues, onPriorityClasses) | ||
}, | ||
} |
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.
Similar to last comment
internal/armadactl/preempt.go
Outdated
queueMsg = "all" | ||
} | ||
|
||
fmt.Fprintf(a.Out, "Requesting preemption of jobs matching executor: %s, queues: %s, priority-classes: %s\n", executor, queueMsg, priorityClasses) |
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.
You need to string.join(priorityClasses)
as you did with queues
ON jr.job_id = j.job_id | ||
WHERE jr.executor = $1 | ||
AND j.queue = ANY($2::text[]) | ||
AND jr.succeeded = false AND jr.failed = false AND jr.cancelled = 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.
Also AND jr.preempted = false
…i/job-controls-on-executor
… formatted priority classes message to Armadactl app
Summary of changes: