Add concurrency. #640
Add concurrency. #640
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR adds concurrency control to GitHub Actions workflows to automatically cancel in-progress workflow runs when new commits are pushed to pull requests, preventing resource waste from outdated CI runs.
Key Changes:
- Added concurrency groups based on workflow name and git ref to both test and documentation workflows
- Configured automatic cancellation for in-progress runs except on the main branch
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
.github/workflows/docs_build_and_deploy.yml |
Added concurrency configuration to cancel duplicate documentation build runs on PRs |
.github/workflows/code_test_and_deploy.yml |
Added concurrency configuration to cancel duplicate test and deployment runs on PRs |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # Cancel in-progress runs if a new run is triggered, except for main branch. | ||
| concurrency: | ||
| group: ${{ github.workflow }}-${{ github.ref }} | ||
| cancel-in-progress: ${{ github.ref != 'refs/heads/main' }} |
There was a problem hiding this comment.
The cancel-in-progress condition should also exclude tag pushes to prevent cancellation of release workflows. Tag pushes have refs like refs/tags/v* and trigger deployment jobs, which should not be cancelled.
Consider updating to: cancel-in-progress: ${{ github.ref != 'refs/heads/main' && !startsWith(github.ref, 'refs/tags/') }}
| cancel-in-progress: ${{ github.ref != 'refs/heads/main' }} | |
| cancel-in-progress: ${{ github.ref != 'refs/heads/main' && !startsWith(github.ref, 'refs/tags/') }} |
There was a problem hiding this comment.
I dont think this is required as we only make releases from main anyway.
There was a problem hiding this comment.
Hey @sfmig adding this too! Copilot suggested the above but I don't think it's necessary because we only make releases from main branch anyway?
There was a problem hiding this comment.
Hi Joe!
I think you are right! Just some points below.
we only make releases from main branch anyway?
Do you mean that in practice you don't add tags with Git, and just tag commits in main via the releases interface in GitHub?
Copilot suggested the above but I don't think it's necessary
Yeah I think the suggestion is not necessary, even if you didn't just tag commits in main using the GitHub releases interface. This is because the git ref is different for each tag. So different tags don't cancel each other.
Without the copilot suggestion, runs triggered by a push to a non-main branch will cancel any previous runs triggered in the same way. And force-pushes of a tag while the workflow is running will cancel any previous runs triggered by _the same tag _. I think this behaviour is what we want.
The above comes from the definition of the group parameter in concurrency. It defines workflows that can cancel each other. So for code_test_and_deploy.yml:
- a push to
branch-1creates a grouptests-refs/heads/branch-1 - a pushed tag (e.g.
v1.0.0) creates a grouptests-refs/tags/v1.0.0(note that the group name is specific to that tag).
Different groups do not interfere each other. So "tag v1.0.0 pushes" won't cancel "pushes to branch-1", and "tag v1.0.0 pushes" won't cancel "tag v2.0.0 pushes".
Hope this helps!
CI from past commits on a PR are not cancelled when a new commit is pushed.