Skip to content

Cancel duplicate workflow runs#4207

Merged
rdblue merged 2 commits intoapache:masterfrom
snazy:cancel-duplicate-workflow-runs
Mar 8, 2022
Merged

Cancel duplicate workflow runs#4207
rdblue merged 2 commits intoapache:masterfrom
snazy:cancel-duplicate-workflow-runs

Conversation

@snazy
Copy link
Member

@snazy snazy commented Feb 23, 2022

Take 2, using GitHub's native functionality.
Basically reverts #3004.

See https://docs.github.com/en/actions/using-workflows/workflow-syntax-for-github-actions#concurrency

@github-actions github-actions bot added the INFRA label Feb 23, 2022
@snazy
Copy link
Member Author

snazy commented Feb 23, 2022

/cc @kbendick

Copy link
Contributor

@kbendick kbendick left a comment

Choose a reason for hiding this comment

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

Thanks for tagging me @snazy. I looked into this when I made the original cancel duplicate workloads file, but it was pretty beta at the time and I couldn't get the group such that it would cancel the same PR (I didn't try that hard as it was pretty new functionality and the apache/spark repo uses the other project).

Have you tested this configuration in your own repo, or in any other repo? Does it work the same?

@kbendick
Copy link
Contributor

kbendick commented Feb 23, 2022

Ahhh I see the docs have been updated and this concurrency group is explicitly called out: https://docs.github.com/en/actions/using-workflows/workflow-syntax-for-github-actions#example-only-cancel-in-progress-jobs-or-runs-for-the-current-workflow

My (very limited) experience was that it would cancel any workflow (outside of the current PR or not), but I guess if you have github.ref then that should be ok. github.ref didn't exist at the time iirc.

This looks to cover all of the workflows that were part of the old cancel-duplicates.

Copy link
Contributor

@kbendick kbendick left a comment

Choose a reason for hiding this comment

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

Gonna give this a cautious +1 for the reasons mentioned.

If you've used this concurrency group and it worked as expected (cancels duplicates from the same branch), then I'm good with it. Like I said, when I tried it before it was very beta but it would be nice to use the native functionality.

If you don't have experience with it, we should maybe test it quickly in a fork or something (or have a committer on hand to revert).

@kbendick
Copy link
Contributor

Looking at this Stack Overflow post, I'm much more confident in this now: https://stackoverflow.com/questions/58033366/how-to-get-the-current-branch-within-github-actions

@snazy
Copy link
Member Author

snazy commented Feb 23, 2022

We've merged the same change into Nessie today and I realized that Quarkus is also using this technique now - they have more complex conditions though: https://github.com/quarkusio/quarkus/blob/main/.github/workflows/ci-actions-incremental.yml#L36-L38

@snazy
Copy link
Member Author

snazy commented Feb 24, 2022

Realized that the proposed change would cancel previous CI runs on the master branch as well, which is rather not so good. So I've pushed another commit.


concurrency:
group: ${{ github.workflow }}-${{ github.ref }}
cancel-in-progress: ${{ github.event_name == 'pull_request' }}
Copy link
Contributor

@kbendick kbendick Feb 24, 2022

Choose a reason for hiding this comment

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

Good call 👍 .

Maybe I would add a default value of ${{ github.event_name == 'pull_request' || false }}. But I don't think that's needed because event_name should be defined for all.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yea, no event without a name (I hope so ;) )

@rdblue
Copy link
Contributor

rdblue commented Mar 8, 2022

Thanks for fixing this, @snazy!

@rdblue rdblue merged commit 3e019bb into apache:master Mar 8, 2022
@snazy snazy deleted the cancel-duplicate-workflow-runs branch March 8, 2022 08:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants