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

ci: cancel previous CI runs on Pull Requests #961

Merged
merged 3 commits into from
Jul 13, 2023

Conversation

inFocus7
Copy link
Contributor

@inFocus7 inFocus7 commented Jul 10, 2023

Fixes #959

Please provide a detailed description of the changes made in this pull request.

Added github concurrency the workflow level in order to cancel previous runs.

concurrency:
  group: ${{ github.workflow }}-${{ github.head_ref || github.run_id }}
  cancel-in-progress: true

Added to all workflows that run on PRs which could technically face this issue.

Explanation:

Concurrency works by only running a workflow (or job) one at a time. They are normally queued (and cancelled) based on their group name.

  • Each workflow has a unique prefix (github.workflow) in order for different workflows to not interfere with one another.
  • The github.head_ref || github.run_id ensures that concurrency only run on pull requests. How?
    • github.head_ref is pull_request specific based on source branch. So only during pull requests will it get a value.
    • If head_ref is empty, then it's not a pull requests and resorts to run_id. run_id is a unique id generated per-run, so on non-PR runs it'll be unique, and multiple pushes to main's runs won't affect one-another.
  • The cancel-in-progress: true ensures that when multiple workflows are ran in the same group, the in-progress ones get cancelled in favor of the latest runs.

Therefore, only per PR run it'll have name workflowName-prHeadRef, and on sequential runs on the same PR, each new workflow will be part of their respective groups and be affected by the concurrency.

Checklists...

Contributors Checklist

  • Added new tests, or not needed, or not feasible
  • Provided an example (e.g. screenshot) to aid review or the PR is self-explanatory
  • Updated the official documentation or not needed
  • No breaking changes were made, or a BREAKING CHANGE: xxx message was included in the description
  • Added references to related issues and PRs
  • Provided any useful hints for running manual tests
  • Added new benchmarks to generated graphs, if any. More info here.

Maintainers Checklist

  • Checked that the author followed the guidelines in CONTRIBUTING.md
  • Checked the conventional-commit (especially PR title and verb, presence of BREAKING CHANGE: in the body)
  • Ensured that this PR is not a significant change or confirmed that the review/consideration process was appropriate for the change

@inFocus7 inFocus7 requested a review from a team as a code owner July 10, 2023 02:13
@inFocus7
Copy link
Contributor Author

To test this we'd probably want to run all workflows and push a noop commit after a few seconds to make sure the workflows cancel.

@ajnavarro
Copy link
Contributor

@inFocus7 Hi! Could you explain what is the problem you are trying to solve? (like making this we are going to decrease the CI bill, or executing CI on PRs will be much faster). Thanks!

@harry-hov harry-hov changed the title ci: Cancel Previous CI Runs on Pull Requests ci: cancel previous CI runs on Pull Requests Jul 10, 2023
@inFocus7
Copy link
Contributor Author

inFocus7 commented Jul 10, 2023

@ajnavarro It would help in decreasing CI bill by cancelling unnecessary/older workflow runs - assuming it costs to runs the workflows. At the very least, this helps with not using unnecessary resources.

@moul
Copy link
Member

moul commented Jul 12, 2023

To be honest, the bill itself is not the primary concern. The real issue lies in the queues and the requirement to wait for outdated changes to be completed before addressing the most recent ones. This becomes problematic during rush periods, particularly when we need to prioritize reviews.

@moul moul merged commit 72a5732 into gnolang:master Jul 13, 2023
73 checks passed
Doozers pushed a commit to Doozers/gno that referenced this pull request Aug 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: 🔵 Not Needed for Launch
Archived in project
Development

Successfully merging this pull request may close these issues.

CI: cancel previous running jobs
4 participants