add a concurrency check to gh workflows

[mahf708]

And maybe I would also add a concurrency check. If a commit is pushed while testing, we should kill any other existing PR. Something like

concurrency:
  group: ${{ github.workflow }}-${{ github.event_name }}-${{ github.event.pull_request.number || github.run_id }}
  cancel-in-progress: true

Btw, I'm not sure why we would want a workflow_dispath trigger here. And if you removed "workflow_dispatch", the group could simply be ${{ github.workflow }}-${{ github.event.pull_request.number}}. But I don't feel strongly about this, so I'm ok keeping workflow_dispatch.

Originally posted by @bartgol in #6637 (comment)

[mahf708]

If someone picks this up, pls keep the workflow_dispatch, and at any rate, pls request review from or at least tag both luca (@bartgol) and me (@mahf708) to review the PR

The workflow_dispatch part is useful because it allows us to trigger the workflow on any arbitrary branch (i.e., outside of PRs) and it also allows us to trigger the workflows at-will.

[bartgol]

I thought that in your action, as it was written (or, maybe, as I read it) there was no real need for dispatch trigger. That said, there could be a use for dispatch trigger: to generate baselines. As such, we would need a check in the job section that, depending on the trigger, decides what flags to add to test-all. In my fork, I was experimenting with multiple dispatch types in the same action, each of which needs to handle different test-all flags:

• if pr trigger: no "extra" flags
• if schedule trigger (nightly): add -s (to submit to cdash)
• if manual trigger (bless): add -g (to generate baselines)

This would however require to store baselines somewhere. I don't think we can store inside the container (unless we rebuild it). Perhaps we could store eamxx baselines for gh actions (or even all machines) in a separate repo, divided by machine? They are not that big anyways. Our whole mappy baselines folder (for eamxx standalone) is ~650M, and our v1 folders are prob ladding up to ~250M. We can make our nightly job copy the baselines from this helper repo into the container image, so that PR testing is always "fast" (just one download: the image).

[mahf708]

I thought that in your action, as it was written (or, maybe, as I read it) there was no real need for dispatch trigger.

The only reason for that line is that it allows us to trigger it manually. Without it, I don't think we can trigger the action manually... do you think I am mistaken? Is there a way to trigger the action manually without it?

[bartgol]

You can't without, that's true. That said, I don't know if we need that for nightlies. If the action fails, we can always re-run the job (even without the dispatch trigger). You can't run for a different sha though, that's true.

So I guess the question is: why do you think we'd need to manually trigger it? Since this action does not take an input, it would only run for current master. Do you think we'd ever need to run master manually (with a sha different from last night's run)?

[mahf708]

The ability to manually trigger action runs is important in my experience. I personally made use of it quite a bit. For example when Oksana and Andrew were fixing the homme issue last week, I ran those tests manually to verify. The tests also help in the event of an unexplained fail in a PR, we can quickly trigger a run on master (or any other branch that is not part of a PR). It's just convenient. In the future, we can extend that to making baselines, etc., but that (baselines-related stuff) is not part of my plans for the foreseeable future (though I think it is pretty easy to impl)

[bartgol]

Re: triggering run on master. Unless something was merged since the prev night run, you can retrigger a master run even without workflow dispatch. And if you want to re-run master tests if another PR was pushed since last night's testing, you can also add a push: master trigger to the action.

I don't see much value adding the ability of workflow_dispatch, unless you can provide inputs to the manual dispatch. The nightly run (via schedule), or the push: master run would already give you the same coverage.

This is all very tomato tomato, maybe. So I don't really care that much.

[mahf708]

These are valid points :) I think I agree with you that in the long term (i.e., once we move a lot more of our testing to the gh/ci infra), we shouldn't want to keep the manual dispatch around.

For the time being, we don't run on a push to master at all (only in pull requests) and we don't run on a nightly schedule. If we have one of these two, as you rightly point out, then having the manual dispatch becomes useless, I agree