presto
presto copied to clipboard
Add concurrency constraints
Test plan - Create a branch, create a PR, push another commit to the branch, verify that the previous workflows are mostly killed.
https://docs.github.com/en/actions/learn-github-actions/workflow-syntax-for-github-actions#concurrency
== NO RELEASE NOTE ==
This pull request has been automatically marked as stale because it has not had recent activity. If you'd still like this PR merged, please comment on the task, make sure you've addressed reviewer comments, and rebase on the latest master. Thank you for your contributions!
Obviously I'd prefer to have someone review my change...
@jsoref could you please rebase this PR?
@rohanpednekar done
Will take a look at it over the weekend.
@pratyakshsharma ?
Going over the github doc for workflow and concurrency as mentioned in the description of this PR. Will review post going through them.
@pratyakshsharma: this should do what you need... (again you'll want to test to confirm)
I tested and the changes look good. Here is a sample run that I triggered - https://github.com/prestodb/presto/actions/runs/3359687457.
You might want to check why all the checks got cancelled for the latest commit though.
Hmm, strange. My guess is that GitHub got confused and used the wrong version of the workflow to test. (I'd have to spend a lot of time poking and then file a bug).
Here's a PR I've created into a branch w/ the same name and sha: https://github.com/check-spelling/presto/actions?query=branch%3Ablank+event%3Apull_request
It'll take a while to clear as the workflows here use a lot of runners and I only have 20 slots.
So, in https://github.com/check-spelling/presto/actions?query=branch%3Ablank+event%3Apull_request, everything looked more or less ok:
https://github.com/check-spelling/presto/actions/runs/3361686022/jobs/5572411876 failed with a test failure:
Error: Failures:
Error: TestTimeZoneUtils.testNamedZones:77 » IllegalArgument The datetime zone id 'Europe/Kyiv' is not recognised
[INFO]
Error: Tests run: 7416, Failures: 1, Errors: 0, Skipped: 1
https://github.com/check-spelling/presto/actions/runs/3361686030
has:
product-tests-specific-environment1
The operation was canceled.
which sounded suspiciously like a problem, but in fact, it's just the job hitting the 1 hour time limit:
https://github.com/check-spelling/presto/actions/runs/3361686030/workflow#L15-L17
product-tests-specific-environment1:
runs-on: ubuntu-latest
timeout-minutes: 60
Here's a run 7 which was mostly canceled by a run 8:
I say mostly, because it looks like some new workflows were added between when this PR was originally written and now.
Fwiw, there's no meaningful distinction between a ❕ and ❌, it's just an implementation quirk of how GitHub dispatches the cancelations and reports on them. By quirk, I mean "I consider this to be a bug, but I haven't managed to convince GitHub to fix it".
Apologies for the long delay in getting this through. I tested the latest changes and found 2 different types of cancellations like you already mentioned. Anyhow your changes seem to be working and they look good to me.


@prestodb/committers This PR has been lingering for some time now. I have gone through the changes and tested them, and they look good to me.
Let us get this merged.
@rschlussel done
The first canceled here is a test that hit the 1 hour mark (which is presumably a timeout) and not related to the changes in this PR.