search

ddev / ddev-addon-template

Template for DDEV add-ons.
Apache License 2.0
17 stars 17 forks source link

Prevent concurrent workflows #49

Closed tyler36 closed 3 months ago

tyler36 commented 4 months ago

The Issue

Recently I was updating the tyler36/ddev-laravel-queue addon.

The tests generally take 5-6 minutes. When I push a new commit, it triggers another workflow process.

At one point, I click a couple of "Commit suggestion"s and I had 6 workflows running on the same branch!

How This PR Solves The Issue

This PR configures "concurrency" in the workflow. This could be set on a per-job level, but I opted for the entire workflow with this PR. Feel free to change this is desired.

Concurrency is set to cancel in-progress workflows on a branch/tag before starting a new workflow on the same branch/tag.

See https://docs.github.com/en/actions/using-jobs/using-concurrency

Manual Testing Instructions

Automated Testing Overview

Related Issue Link(s)

Release/Deployment Notes

rfay commented 4 months ago

Wow, this seems really appropriate. With DDEV tests we've had the same problem and we use styfle/cancel-workflow-action@0.12.1, https://github.com/ddev/ddev/blob/master/.github/workflows/cancel-previous-workflow.yml

I wonder if your solution here is much more elegant...

rfay commented 4 months ago

Wouldn't it be better to put this into ddev/github-action-add-on-test ? /cc @julienloizelet

tyler36 commented 4 months ago

Wouldn't it be better to put this into ddev/github-action-add-on-test ? /cc

From my understanding, putting it in github-action-add-on-test means it applys to workflows for github-action-add-on-test. Having it here, means that applies to addons that use this template. It's a higher level.

It would be beneficial to have it there too, for that repos tests.

tyler36 commented 4 months ago

I've used this "trick" for the last few years and it works well. My private repos have a "monthly limit" so this was a good solution to reduce wasted cycles.

I took a quick look at https://github.com/styfle/cancel-workflow-action and it says at the top.

Warning You probably don't need to install this custom action. Instead, use the native concurrency property to cancel workflows, for example:

Seems like it was the solution to the problem before Github implemented it natively (2021-04)

rfay commented 4 months ago

Wow, thanks! We have some cleanup to do!

rfay commented 4 months ago

From my understanding, putting it in github-action-add-on-test means it applys to workflows for github-action-add-on-test. Having it here, means that applies to addons that use this template. It's a higher level.

I'm pretty sure that github-action-add-on-test could implement this for everything it's used in. Not sure. It could also implement it for itself, of course.

julienloizelet commented 4 months ago

This "concurrency" directive is very interesting, thanks @tyler36 !

I've done some quick tests and it seems that the "concurrency" directive is not allowed inside a composite action (the github-action-add-on-test action is a composite action).

Did not find any explicit documentation about it but all my tests failed with some "Unexpected value 'concurrency'" message.

This may not be an argument, as we can't always trust it, but chatGPT told me the same thing:

Can I use the concurrency directive in a GitHub composite action ?

No, as of now, you cannot use the concurrency directive directly within the steps of a GitHub composite action. If you need to manage concurrency within a GitHub composite action, you would typically handle it at the workflow level that calls the composite action

In the end, I think this PR is the best we can do.

Thanks

rfay commented 4 months ago

Well. at least we can add it to the ddev-addon-template. Thanks!

tyler36 commented 3 months ago

Thank you for update.

Did not find any explicit documentation about it but all my tests failed with some "Unexpected value 'concurrency'" message.

Yes, that sounds like it is unsupported composite actions (github-action-add-on-test). Should be fine here though.

rfay commented 3 months ago

Thanks @tyler36 , we now have this in DDEV core.

tyler36 commented 3 months ago

Thank you.

Just documenting the last 2 scheduled tests are ran fine.