Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Apply concurrency for GHA workflow pull_requests events only #1544

Merged
merged 1 commit into from
Jan 12, 2023

Conversation

ydah
Copy link
Member

@ydah ydah commented Jan 11, 2023

The following are included as keys to the group.
${{ github.head_ref || github.run_id }}

This allows concurrency to work only with Push to PullRequest, and cancels all but the most recent push for successive pushes; for merge commits, in other words push events, ${{ github.run_id }} is used, so The cancellation is not enforced because it is always a unique group.

This eliminates the problem of consecutive Pushes being cancelled at the time of release.

release


Before submitting the PR make sure the following are checked:

  • Feature branch is up-to-date with master (if not - rebase it).
  • Squashed related commits together.
  • [-] Added tests.
  • [-] Updated documentation.
  • [-] Added an entry to the CHANGELOG.md if the new code introduces user-observable changes.
  • The build (bundle exec rake) passes (be sure to run this locally, since it may produce updated documentation that you will need to commit).

The following are included as keys to the group.
`${{ github.head_ref || github.run_id }}`

This allows concurrency to work only with Push to PullRequest, and cancels all but the most recent push for successive pushes; for merge commits, in other words push events, `${{ github.run_id }}` is used, so The cancellation is not enforced because it is always a unique group.
@ydah ydah force-pushed the use_concurrency_only_pull_request branch from 860e358 to 306ff21 Compare January 11, 2023 03:21
@ydah ydah changed the title Change concurrency only in Pull Request events Apply concurrency for GHA workflow pull_requests envents only Jan 11, 2023
@ydah ydah changed the title Apply concurrency for GHA workflow pull_requests envents only Apply concurrency for GHA workflow pull_requests events only Jan 11, 2023
ydah added a commit to rubocop/rubocop-capybara that referenced this pull request Jan 11, 2023
Same as: rubocop/rubocop-rspec#1544 rubocop/rubocop#11414

The following are included as keys to the group.
`${{ github.head_ref || github.run_id }}`

This allows concurrency to work only with Push to PullRequest, and cancels all but the most recent push for successive pushes; for merge commits, in other words push events, `${{ github.run_id }}` is used, so The cancellation is not enforced because it is always a unique group.

This eliminates the problem of consecutive Pushes being cancelled at the time of release.

![release](https://user-images.githubusercontent.com/13041216/211700533-cb01c051-dc8d-4889-8276-327c96bf4755.png)
Copy link
Member

@pirj pirj 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 the explanation and this follow-up!

@pirj pirj requested review from bquorning and Darhazer January 11, 2023 14:26
pirj pushed a commit to rubocop/rubocop-capybara that referenced this pull request Jan 11, 2023
Same as: rubocop/rubocop-rspec#1544 rubocop/rubocop#11414

The following are included as keys to the group.
`${{ github.head_ref || github.run_id }}`

This allows concurrency to work only with Push to PullRequest, and cancels all but the most recent push for successive pushes; for merge commits, in other words push events, `${{ github.run_id }}` is used, so The cancellation is not enforced because it is always a unique group.

This eliminates the problem of consecutive Pushes being cancelled at the time of release.

![release](https://user-images.githubusercontent.com/13041216/211700533-cb01c051-dc8d-4889-8276-327c96bf4755.png)
Copy link
Collaborator

@bquorning bquorning left a comment

Choose a reason for hiding this comment

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

It does seem rather complicated. Wouldn’t it be easier to remove group and cancel-in-progress: true?

@ydah
Copy link
Member Author

ydah commented Jan 12, 2023

Perhaps, A appears to be a required parameter.
https://json.schemastore.org/github-workflow.json

imege

Also, removing cancel-in-progress: true cancels only the pending ones, but in the following case, the middle ones are cancelled, so I don't think it solves the issue this PR is trying to solve.
image

In addition, the approach implemented in this PR is the approach described in the GitHub documentation.
https://docs.github.com/en/actions/using-workflows/workflow-syntax-for-github-actions#example-using-a-fallback-value

@bquorning
Copy link
Collaborator

I don't think it solves the issue this PR is trying to solve.

Probably I don’t understand which problem this PR solves.

If concurrency is removed from the config, builds would never be canceled. Which seems optimal to me. But, in which cases do we want builds to be canceled?

@ydah
Copy link
Member Author

ydah commented Jan 12, 2023

For example, if you continuously push to PullRequest, or if you continuously take in Suggest Changes, you will probably have several jobs running.

However, since there is a limit to the number of concurrent actions that can be executed, sometimes a job will become pending in the queue, waiting for the last CI that was pushed last to be successfully executed.

For more information about the upper limit on the number of actions, please refer to the following page:
https://docs.github.com/en/actions/learn-github-actions/usage-limits-billing-and-administration#usage-limits

Therefore, my motivation is to reduce this waiting time and to cancel the execution of GHA workflows that are unnecessary in the first place.
Since this PR allows us to cancel only unnecessary GHA Workflow executions, it seems like a better with no side effects. How about it?

@pirj
Copy link
Member

pirj commented Jan 12, 2023

But, in which cases do we want builds to be canceled?

When there is more than one running build for the same PR branch.
This happens when we push often.

Our builds run quickly, it wouldn't be such a big deal for us. But the limit is per-account, i.e. per rubocop organization:

The number of concurrent jobs you can run in your account depends on your GitHub plan, as well as the type of runner used. If exceeded, any additional jobs are queued.
Total concurrent jobs 20

This change is to minimize build queuing across rubocop repos.

@bquorning
Copy link
Collaborator

This change is to minimize build queuing across RuboCop repos.

Got it. Thank you for the explanation (and patience) @ydah and @pirj ❤️

@bquorning bquorning merged commit 7795cbd into rubocop:master Jan 12, 2023
@ydah ydah deleted the use_concurrency_only_pull_request branch January 12, 2023 07:24
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants