-
Notifications
You must be signed in to change notification settings - Fork 415
[CI] Changed the Workflow Triggers #2589
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] Changed the Workflow Triggers #2589
Conversation
Changed the workflow triggers for the test and container workflows such that they only run when there is a push to master. Also added concurrency such that if a CI was called on a particular PR or branch that is already running the CI, it will cancel the old run. This will prevent useless runs of the CI.
@vaughnbetz I think we should merge this in. Currently the CI is full again, this time its someone's PR: This is exactly what the concurrency argument is trying to resolve. I will kill these runs since I don't think this person intends on calling the CI 10 times for the same PR... |
Thanks Alex! |
# See: https://docs.github.com/en/actions/using-jobs/using-concurrency | ||
concurrency: | ||
group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.ref }} | ||
cancel-in-progress: true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@AlexandreSinger This cancels previous CI runs for the master branch. I think we should avoid canceling these runs on master because we don't push to master as often, and multiple pushes are usually from multiple PRs merged in a short time. If something goes wrong, having CI results for each push to the master branch helps us pinpoint issues more easily. What do you think about this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I actually did this on purpose since it is a common scenario that we would merge 3-4 PRs at one time. This causes a storm of traffic on the CI when this occurs which can cause problems. Above this section, I specifically allowed the main branch to trigger the CI on push since, I agree, that the main branch should test on merge. I personally just find running the CI on every PR merge a bit much.
If we decide to allow the CI to trigger on every push to main (and run concurrently), this resource discusses how to do that: https://docs.github.com/en/actions/using-jobs/using-concurrency#example-only-cancel-in-progress-jobs-on-specific-branches
Its very simple to do; but I am just trying to prevent too much traffic from hitting the CI when things get merged in. We can discuss in the group meeting.
Changed the workflow triggers for the test and container workflows such that they only run when there is a push to master.
Also added concurrency such that if a CI was called on a particular PR or branch that is already running the CI, it will cancel the old run. This will prevent useless runs of the CI.