checkstyle / test-configs

gsoc 24 project where will store configs for diff.groovy , https://github.com/orgs/checkstyle/projects/8
GNU Lesser General Public License v2.1
2 stars 3 forks source link

Concurrency model for new workflow should cancel previous execution #121

Closed romani closed 1 month ago

romani commented 1 month ago

for now we use: https://github.com/checkstyle/checkstyle/blob/6fff4fa9b16a3bb03a92de004663096815227b29/.github/workflows/diff-report.yml#L30-L33

but may be we should cancel previous executions and concurrency should be by PR number. But it use to be line this: https://github.com/checkstyle/checkstyle/commit/22c0361c6efafbce9972ebbe005a36090d1724e6 https://github.com/checkstyle/checkstyle/pull/14069

romani commented 1 month ago

@relentless-pursuit ,

I think we should cancel previous executions if one more is triggered. Please investigate how to archive this. We already have this model in some workflows in main repo.

Example: https://github.com/checkstyle/checkstyle/blob/981c0cb87eb58cafef5c84014f94ba1f05080a46/.github/workflows/checker-framework.yml#L10-L13

relentless-pursuit commented 1 month ago
  1. Purpose of concurrency in GitHub Actions:

    • Controls how multiple workflow runs are handled when triggered at the same time or close together.
  2. Current setup:

    • Each workflow run gets a unique group (using github.run_id).
    • No cancellation of in-progress runs.
  3. Basic options:

    a. Allow all runs (current setup):

      concurrency:
        group: ${{ github.run_id }}
        cancel-in-progress: false
    • Every run executes, no matter how many are triggered.

    b. One run per PR at a time:

      concurrency:
        group: ${{ github.workflow }}-${{ github.event.issue.number }}
        cancel-in-progress: false
    • Only one run per PR executes at a time.
    • New runs wait for the current one to finish.

    c. Always run latest, cancel others:

      concurrency:
        group: ${{ github.workflow }}-${{ github.event.issue.number }}
        cancel-in-progress: true
    • Only the latest run for a PR executes.
    • Cancels any in-progress run when a new one is triggered.
  4. @romani if we wish to go with your suggestion, i believe option c can help.

The choice depends on your specific needs for the Diff-Report action. Consider factors like resource usage, how often reports are needed, and the importance of always having the very latest changes in the report.

relentless-pursuit commented 1 month ago
Screenshot 2024-07-24 at 1 10 24 AM

As you can see with changes in workflow to option c, i can only run the latest. Others will be cancelled in the same PR.

image
romani commented 1 month ago

We need One run per PR at a time:

Please do it in your org and test it

romani commented 1 month ago

Ok, I see it. Please keep diff , share here a branch , we will merge it after initial merge.

relentless-pursuit commented 1 month ago

We can close this issue.

romani commented 1 month ago

Merged at https://github.com/checkstyle/checkstyle/pull/15235#issuecomment-2253466000