feat(auto-cancel): server-side logic for auto canceling obsolete builds #911
Conversation
Codecov Report
@@ Coverage Diff @@
## main #911 +/- ##
==========================================
- Coverage 71.10% 70.39% -0.72%
==========================================
Files 313 315 +2
Lines 13028 13195 +167
==========================================
+ Hits 9263 9288 +25
- Misses 3299 3440 +141
- Partials 466 467 +1
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
|
|
||
| req, err := http.NewRequestWithContext(context.Background(), "DELETE", u, nil) | ||
| if err != nil { | ||
| return err |
There was a problem hiding this comment.
what are your thoughts on attempting the other cancels in the event of an error on 1 of them? maybe track an error in the outer scope and check that at the end? idk
There was a problem hiding this comment.
Do you mean like if there are multiple running builds to auto-cancel, and one of those attempts fails?
There was a problem hiding this comment.
yeah, i was looking at the loop over executors
There was a problem hiding this comment.
Only one build will match, so there should only be one attempt at cancellation.
But we do loop over pending/running builds: https://github.com/go-vela/server/pull/911/files#diff-2e48a4018b70aac91235c0a2f5ec50d7ae186de39e3bd603d7e0fcfe6fd5ab07R25-R65.
I was unsure as to whether or not to immediately error out upon failing one of those (provided they match all the other criteria). What do you think? If this policy was implemented, there really should only be one build at a time that gets the boot. Each new build will supersede the older build, which superseded a previous build, and so on.
All's that to say, even though there are a couple loops here, we're really just performing one operation overall — I think... 😅
There was a problem hiding this comment.
It seems like the current implementation has the potential to take some time before the current build makes it onto the queue dependent on how many builds need to be canceled and how many containers each build spun up.
It waits until all builds are canceled before pushing the current build to the queue. Should we send current build to queue and, in best effort, send off cancellation attempts to the builds that we need to cancel?
Depends on go-vela/types#299