pre-commit-ci / issues

public issues for https://pre-commit.ci
16 stars 3 forks source link

Running GitHub Actions only after pre-commit.ci suceeds to save resources, built time etc. #106

Closed lorenzwalthert closed 2 years ago

lorenzwalthert commented 2 years ago

My ideal workflow

The problem

By default, all GitHub Actions and app integrations are triggered simultaneously. If pre-commit fails on first attempt, then auto-fixes and pushes, all GitHub actions are triggered twice, which is a problem from multiple perspectives:

If I get the numbers right, pre-commit.ci ran over 280k times as of Nov 2021, and is probably growing exponentially, so I think it's worth having a look at this.

The solution

I searched the web and found out that it may be possible to trigger a GitHub Action workflow only conditionally on the result of check_suite, e.g. as demonstrated in a repo that previously implemented that functionality and now leverages the native GitHub solution:

on:
  check_suite:
    type: ['completed']

name: Continue after Cirrus CI Complets Successfully
jobs:
  continue:
    name: After Cirrus CI
    if: github.event.check_suite.app.name == 'Cirrus CI' &&  github.event.check_suite.conclusion == 'success'
    runs-on: ubuntu-latest
    steps:
    - name: Continue    
      run: echo "Hello after Cirrus CI Completed"

This is very similar to the official docs and what people discussed on Github Community Forum.

However, I could not make pre-commit.ci trigger another workflow in my example repo, maybe the GitHub App (in that case pre-commit.ci) must use the GitHub API to set the status of the suite as completed. I don't know what exactly it would take to implement and if it requires further work on the pre-commit.ci side. Even if not, I think providing hints in the docs on how to set up GitHub Actions to run after pre-commit.ci successfully completes sounds like a useful thing for me to avoid redundant builds.

asottile commented 2 years ago

pre-commit.ci doesn't use check suites -- but you can probably do the same with the status action trigger: https://docs.github.com/en/actions/learn-github-actions/events-that-trigger-workflows#status

asottile commented 2 years ago

also github I believe cancels in-flight actions in the case of a push -- or at least can be configured to do so -- so the impact of this is rather minimal

asottile commented 2 years ago

also this isn't any more actions execution than previously -- previously the linting would fail and a user would push manually still triggering all of CI twice

asottile commented 2 years ago

so in conclusion:

but thanks for the issue nonetheless!

lorenzwalthert commented 2 years ago

Thanks for the links, I think the in-flight cancel indeed got officially supported recently. This requires opt-in, and I think few people will do that.

I don't think there's anything to fix here -- it's not worse than the status quo without pre-commit.ci

Well, assuming all checks that pre-commit does happen in CI before, which I think is not the case (at least some of them did not happen). Anyways, we could also see it as an opportunity to improve status quo 😄.

if you need custom post-pre-commit.ci behaviour you can leverage the status webhook above

Ok, that sounds complicated, I think the cancel on push will suffice.

Anyways, thanks for the answers, appreciate it. Still did not give up the hope that this will be revisited at some point in the future 😉