[🐛 CI]: Jobs are not getting cancelled properly

[titusfortner]

What happened?

Putting this in an issue since there is a lot of info and I need help. 😄

The original CI code was:

name: CI

on:
  pull_request:
  push:
    branches:
      - trunk
  schedule:
    - cron: "0 */12 * * *"
  workflow_dispatch:

  group: ${{ github.workflow }}-${{ github.ref }}
  cancel-in-progress: ${{ github.event_name == 'pull_request' }}

For some reason, this job that was manually triggered (with "workflow_dispatch") got the message: Canceling since a higher priority waiting request for 'CI-refs/heads/trunk' exists based on this job which was triggered by a push.

Since I wanted to run everything on trunk at that time, and the new job only ran JS tests based on our Bazel check job, I tried to redo the concurrency section.

The idea was to differentiate the groups between something that tested everything ("workflow_dispatch" and "schedule") from the ones that do not necessarily do that ("pull_request" and "push") by appending the string "-all" for the first two. Doing this required using a "fake ternary". The current code is:

concurrency:
  group: ${{ github.workflow }}-${{ github.ref }}${{ github.event_name == 'workflow_dispatch' && '-all' || '' }}${{ github.event_name == 'schedule' && '-all' || '' }}
  cancel-in-progress: true

except now I'm running into the issue where this job that tests just the Python tests gets canceled because of this job that tests just the Ruby tests.

Looking at this again, I realize that the cancel-in-progress: ${{ github.event_name == 'pull_request' }} should have prevented the original issue I had, so now I'm a little stuck on what this code should be.

The goal:

• Cancel jobs that re-run tests that have changed in the same branch with a subsequent job
• Do not cancel jobs that are running tests that might not get run in a subsequent job

If we can trust bazel to cache properly, then maybe we don't need to worry about running more jobs and we shouldn't cancel things in progress? Or is caching not good enough? (there have been periods of active development where the CI was backed up about 4 hours which led me to believe we were redoing a lot of tests)

@p0deje / @diemol Any ideas?

[p0deje]

It should be fairly straightforward if we use the following group:

group: ${{ github.workflow }}-${{ github.ref }}-${{ github.actor }}-${{ github.event_name }}

Now if you run a job, it should not affect the builds that are started manually by other people (github.action is responsible for that) and different events (github.event_name).

[titusfortner]

Except if I push a Ruby update to trunk then push a JS update to trunk, the Ruby update will get stopped

And I'd be ok with a test run that tests everything stopping something else testing everything (hence combining dispatch & scheduled). But I guess I'd rather run more tests than fewer tests if it comes down to it.

[p0deje]

Except if I push a Ruby update to trunk then push a JS update to trunk, the Ruby update will get stopped

That should be handled by cancel-in-progress so in this case the Ruby update will have to finish first.

[titusfortner]

If it matches groups and cancel-in-progress is false, it does one at a time. If cancel-in-progress is true, it will stop the previous running one and start the new one. At least that's how I understand it, which is why I no longer understand why it wasn't working the first time.

[diemol]

Except if I push a Ruby update to trunk then push a JS update to trunk, the Ruby update will get stopped

That should be handled by cancel-in-progress so in this case the Ruby update will have to finish first.

This.

So, we should disable it, and run a build at a time. Which should be quick enough given bazel caching.

[p0deje]

At least that's how I understand it, which is why I no longer understand why it wasn't working the first time.

That's right and I am curious why it didn't work too.

[titusfortner]

I just removed everything until we know what we want — cc85374

I deleted everything because why should we only run one build at a time? Let's run whatever GitHub lets us run.

[titusfortner]

Or does that get in the way of how we are doing caching?

If 2 jobs need to use the same cache... I'm not sure how bazel + github manages that.

[titusfortner]

I'm closing this, we'll know if we want to make changes later, don't need this to track it

[github-actions]

This issue has been automatically locked since there has not been any recent activity since it was closed. Please open a new issue for related bugs.