Add a concurrency group to try jobs #30756
Conversation
This should prevent multiple `parse-comment` jobs from running at the same time for the same pull request, reducing the number of duplicate builds when multiple labels are applied.
|
Wouldn't this also prevent running two different try runs for different PRs? |
| @@ -8,6 +8,8 @@ jobs: | |||
| parse-comment: | |||
| name: Trigger Try | |||
| runs-on: ubuntu-latest | |||
| concurrency: | |||
| group: try-${{ github.event.number }} | |||
There was a problem hiding this comment.
Can we also use cancel-in-progress: true to allow more recently triggered workflow's labels for the run?
https://docs.github.com/en/actions/using-workflows/workflow-syntax-for-github-actions#concurrency
There was a problem hiding this comment.
I did consider this, but it would add a subtle race condition. If a new job was triggered while the first was running, it could lead to a label being ignored. In this way, the second job will run and simply exit early if no applicable label is found.
There was a problem hiding this comment.
Ah, does this happen because we remove the labels when the job starts? Is there any advantage for doing it this way and not removing labels once it completes successfully?
There was a problem hiding this comment.
The issue isn't so much the time after the labels are removed, but the time between when the action and queued and when the labels are finally removed. I'm not sure that removing the labels at the end of the job would help, because if a label was added in the meantime another job would still need to be triggered for those new labels.
There was a problem hiding this comment.
My concern was not to reduce the need for new jobs when new labels are added, but to keep the logic simple so that we don't have to worry about such subtle races when extending this workflow in the future.
I am not sure I have a full picture of this workflow's lifecycle and event payloads to understand the race here, so I'm good with merging this PR as is and revisit this if it becomes a problem.
@sagudev , I believe that is avoided here by using a group id derived from the PR number i.e |
This should prevent multiple
parse-commentjobs from running at thesame time for the same pull request, reducing the number of duplicate
builds when multiple labels are applied.
./mach build -ddoes not report any errors./mach test-tidydoes not report any errors