Implement concurrency control in spdk-common-tests #122
Conversation
| default: '' | ||
|
|
||
| concurrency: | ||
| group: ${{ github.workflow }}-${{ github.ref_name }}-${{ inputs.client_payload != '' && fromJson(inputs.client_payload).patchSet.number || inputs.gerrit_ref }} |
There was a problem hiding this comment.
github.ref_name won't this always evaluate to main since this is spdk-ci branch, not spdk ? If so, I'm not sure what purpose it serves.
fromJson(inputs.client_payload).patchSet.number - this is incorrect as it evaluates to n'th number of git push of a specific patch for review (ex. patch set 1, 2, 3...). At the very least we need fromJson(inputs.client_payload).change.number which is an id unique to specific patch in review (ex. 17495).
Combination of those two values represents a patch that is worth running through CI once.
Ideally just change.number should be sufficient to catch testing same patch twice, but we need to extra sure that we never cancel newer patchSet.number for older one. I think patch_set_status should catch that, but I'm not 100% sure.
Please, either double check there is no case where older patch set cancels newer patch set - or just use combination of both values as concurrency group.
There was a problem hiding this comment.
- I added
github.ref_nameto protect from the case when somebody create a new PR here in this repo. in this case it will not bemainand will not cancel running tests. we can remove it if not a real use case patchSetgood catch I meantchange. fixed- let me think how can we prove that
olderpatch setnevercancelsnewerpatch set ...
Add concurrency configuration to workflow This will cancel jobs in progress for the same patchset if new version was uploaded. No reason to test old outdated patch Signed-off-by: Boris Glimcher <Boris.Glimcher@emc.com>
2 participants
Add concurrency configuration to workflow
This will cancel jobs in progress for the same patchset if new version was uploaded. No reason to test old outdated patch