Skip to content
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

Add cancel-in-progress #651

Merged
merged 1 commit into from
Sep 12, 2023
Merged

Conversation

erikbosch
Copy link
Contributor

Intention is to cancel ongoing/queued jobs if a new version of a PR is uploaded

Fixes #626

@erikbosch erikbosch marked this pull request as draft September 7, 2023 08:21
@erikbosch erikbosch force-pushed the erik_cancel branch 6 times, most recently from 456587f to 12a8aba Compare September 7, 2023 09:40
@@ -6,6 +6,11 @@ on:
jobs:
check-spdx-headers:
runs-on: ubuntu-latest

concurrency:
group: ${{ github.ref }}-${{ github.workflow }}-check-spdx-headers
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

General idea is to have a unique group-id for each job. I first tried including ${{ github.job }} but that is not set when this is evaluated, so now the pattern is ref-workflow-

@erikbosch
Copy link
Contributor Author

This is an example of what happens if a job is cancelled because a new PR version has been uploaded

image

Copy link
Contributor

@lukasmittag lukasmittag left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

couldn't we put it one layer up on the same level as jobs? because if one workflow will have one with a higher priority it will cancel all and we need less code. Should work too according to the docs.

@erikbosch
Copy link
Contributor Author

couldn't we put it one layer up on the same level as jobs? because if one workflow will have one with a higher priority it will cancel all and we need less code. Should work too according to the docs.

I tried to put the concurrent on workflow level like below, but then the cancel-in-progress seemed to cancel all but one of the jobs in that workflow. Will test a bit more

 concurrency:
      group: ${{ github.ref }}-${{ github.workflow }}
      cancel-in-progress: true

@erikbosch erikbosch force-pushed the erik_cancel branch 6 times, most recently from ffc6042 to a2b7e65 Compare September 12, 2023 09:34
Intention is to cancel ongoing/queued jobs if a new version of a PR is uploaded

Fixes eclipse#626
@erikbosch
Copy link
Contributor Author

couldn't we put it one layer up on the same level as jobs? because if one workflow will have one with a higher priority it will cancel all and we need less code. Should work too according to the docs.

I think I found the problem of having concurrency defined on workflow level - we can have it for all but the check_push_right workflows, as that is a reusable workflow. Also no need on that one as it is a very quick one, the biggest gain is in our "lengthy" workflows, especially the ARM ones

@erikbosch erikbosch marked this pull request as ready for review September 12, 2023 12:25
Copy link
Contributor

@SebastianSchildt SebastianSchildt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, and everything is green

If an actual (sequence of) cancel(s) later leads to some unforeseen problems, we will see and can fix it then :)

@SebastianSchildt SebastianSchildt merged commit 351dbc5 into eclipse:master Sep 12, 2023
18 checks passed
@erikbosch erikbosch deleted the erik_cancel branch September 29, 2023 09:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CI cancel in progress
3 participants