DEVPROD-10420: do not link patch_optional dependencies for alias versions #8348
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Remove TODOs and debugging
hadjri
approved these changes
Sep 27, 2024
repotracker/repotracker_test.go
Outdated
|
|
||
| tasks, err := task.Find(task.ByVersion(v.Id)) | ||
| s.NoError(err) | ||
| s.Require().Len(tasks, 2) |
There was a problem hiding this comment.
nit: don't need require here since we don't index the slice
There was a problem hiding this comment.
Switched to assert.
model/lifecycle.go
Outdated
| @@ -742,6 +742,9 @@ func createTasksForBuild(ctx context.Context, creationInfo TaskCreationInfo) (ta | |||
| taskMap := make(map[string]*task.Task) | |||
| for _, t := range tasksToCreate { | |||
| id := execTable.GetId(creationInfo.Build.BuildVariant, t.Name) | |||
| if id == "" { | |||
There was a problem hiding this comment.
nit: can we move this into createOneTask for empty id protection against potential future uses of it?
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
DEVPROD-10420
Description
The tasks that have hit the task dependencies alert are tasks that have a dependency on a nonexistent task. They get stuck in the task queue because they're waiting to run on the nonexistent task.
This happens because of
patch_optionaldependencies in non-commit waterfall versions. For example, let's say there's ataskAand it has apatch_optionaldependency ontaskB. The expected behavior ofpatch_optionalis that the dependency will never be created automatically in the version, so the dependency link should only be created if bothtaskAandtaskBare created together. Furthermore, let's assume there's a periodic build that has an alias that selects onlytaskAto run. In this case,taskAshould be created andtaskBshouldn't (and we do see this happen, which is correct). However, even thoughtaskBwasn't created,taskAnonetheless has adepends_onlink to the nonexistenttaskB(this shouldn't happen).The cause of the bug is that, when creating a waterfall version, we have to make a pre-generated table of task IDs. Currently, the pre-generated task ID table creates task IDs for all possible variant-tasks, even those that might not be created upon version creation. This logic will skip the
patch_optionaldependency so the dependent task is not automatically created (correct), but this logic will still create the dependency link to the (nonexistent) task because the pre-generated task ID exists (not correct). To fix this, I filtered tasks from the task ID table so that for versions like periodic builds that create a subset of all possible tasks, they only generate task IDs for tasks that will actually be created.Testing
patch_optionaldependency was ignored.Documentation
Clarified in documentation that, despite the name of the field,
patch_optionalmeans the dependency is optional in all versions, not just patches. The docs already say this explicitly, but I thought it was worth clarifying that the name of the field doesn't correspond to what you'd expect it to do.