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

Don't cancel GHA runs on subsequent push events #9917

Merged
merged 1 commit into from
Apr 6, 2023

Conversation

jakelishman
Copy link
Member

Summary

With our switch to using the GitHub merge queue with speculative merging, it's now possible for us to have two push events to a protected branch (like main) within a couple of minutes. If this happens, the eager cancellation rules we have set for the coverage and Neko runs in GHA trigger, causing a spurious build failure to appear in the commit logs, and for the coverage history to be lost for one commit.

Instead, we can set the cancel-on-update behaviour to only trigger on PR sync events, not branch-push events.

Details and comments

We also likely want to set the workflows to only trigger on pushes to Qiskit repos so users' notifications don't get spammed, and we're not just burning the planet running unnecessary CI, but that's a separate logical change.

With our switch to using the GitHub merge queue with speculative
merging, it's now possible for us to have two push events to a protected
branch (like `main`) within a couple of minutes.  If this happens, the
eager cancellation rules we have set for the coverage and Neko runs in
GHA trigger, causing a spurious build failure to appear in the commit
logs, and for the coverage history to be lost for one commit.

Instead, we can set the cancel-on-update behaviour to only trigger on PR
sync events, not branch-push events.
@jakelishman jakelishman added type: qa Issues and PRs that relate to testing and code quality Changelog: None Do not include in changelog labels Apr 5, 2023
@jakelishman jakelishman requested a review from a team as a code owner April 5, 2023 21:57
@qiskit-bot
Copy link
Collaborator

Thank you for opening a new pull request.

Before your PR can be merged it will first need to pass continuous integration tests and be reviewed. Sometimes the review process can be slow, so please be patient.

While you're waiting, please feel free to review other open PRs. While only a subset of people are authorized to approve pull requests for merging, everyone is encouraged to review open pull requests. Doing reviews helps reduce the burden on the core team and helps make the project's code better for everyone.

One or more of the the following people are requested to review this:

  • @Qiskit/terra-core

@jakelishman
Copy link
Member Author

I tested this by pushing an empty commit to the branch (and then force-pushed it away again). You can see that the coverage and Neko runs were suitably cancelled, for example here: https://github.com/Qiskit/qiskit-terra/actions/runs/4623222644.

@coveralls
Copy link

Pull Request Test Coverage Report for Build 4623231438

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 85.431%

Totals Coverage Status
Change from base Build 4621025983: 0%
Covered Lines: 67646
Relevant Lines: 79182

💛 - Coveralls

@mtreinish mtreinish added this pull request to the merge queue Apr 6, 2023
Merged via the queue into Qiskit:main with commit fe2f0f2 Apr 6, 2023
@jakelishman jakelishman deleted the better-gha-cancellation branch April 12, 2023 17:34
giacomoRanieri pushed a commit to giacomoRanieri/qiskit-terra that referenced this pull request Apr 16, 2023
With our switch to using the GitHub merge queue with speculative
merging, it's now possible for us to have two push events to a protected
branch (like `main`) within a couple of minutes.  If this happens, the
eager cancellation rules we have set for the coverage and Neko runs in
GHA trigger, causing a spurious build failure to appear in the commit
logs, and for the coverage history to be lost for one commit.

Instead, we can set the cancel-on-update behaviour to only trigger on PR
sync events, not branch-push events.
king-p3nguin pushed a commit to king-p3nguin/qiskit-terra that referenced this pull request May 22, 2023
With our switch to using the GitHub merge queue with speculative
merging, it's now possible for us to have two push events to a protected
branch (like `main`) within a couple of minutes.  If this happens, the
eager cancellation rules we have set for the coverage and Neko runs in
GHA trigger, causing a spurious build failure to appear in the commit
logs, and for the coverage history to be lost for one commit.

Instead, we can set the cancel-on-update behaviour to only trigger on PR
sync events, not branch-push events.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Changelog: None Do not include in changelog type: qa Issues and PRs that relate to testing and code quality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants