search

pre-commit-ci / issues

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

Queries around pre-commit.ci timing out #86

Closed peterjc closed 2 years ago

peterjc commented 2 years ago

After successful deployment on several personal smaller repositories (which was very easy - thank you!), I've been trying pre-commit.ci on the larger Biopython repository. Here running flake8 etc via pre-commit on all files can take a few minutes (about 2.5 mins on my desktop best of three; can be 4 mins on CircleCI). This hits the current pre-commit.ci 3 mins timeout:

pre-commit.ci status

See https://github.com/biopython/biopython/pull/3717 with timings. In local testing (where black has access to its cache from previous runs and is thus very quick), the total runtime is dominated by flake8 (and its plugins).

I have several related questions here, some of which might deserve separate issues:

  1. Can the timeout be increased (e.g. paid service)?
  2. Could the badge and GitHub status distinguish "failure" from "time out"?
  3. Does pre-commit.ci retain the black cache between runs, or will black always have to check all files?
  4. Does pre-commit.ci run with access to multiple cores? Should we try that explicitly for flake8?
  5. When run on a pull request, could pre-commit.ci restrict checks to changed files only? Perhaps as a configurable option.
asottile commented 2 years ago
  1. Can the timeout be increased yes as a paid service
  2. Can the badge distinguish time out a timeout isn't distinguishable from a failure
  3. Is the black cache kept? no, and doing so is not safe
  4. Does pre-commit.ci have access to multiple cores? I don't want to give away too much about how it works but yes, and flake8 will use them automatically
  5. When run on a pull request, restrict to changed files? It's certainly possible, but sacrifices correctness and increases complexity
peterjc commented 2 years ago

Thank you, that's helpful. No trivial options I've missed then.

We're skipping flake8 under pre-commit.ci for now, but perhaps that can be sped up enough for the 3min limit (e.g. dropping some of the flake8 plugins we're using).

asottile commented 2 years ago

there's a few files in biopython that flake8 has a hard time with (large generated files especially) -- and a few of your plugins may be made superfluous with pyupgrade

peterjc commented 2 years ago

Adopting pyupgrade in pre-commit should make flake8-comprehensions redundant, but any time change is marginal (may be slower). However, running pyupgrade again was a good idea: https://github.com/biopython/biopython/pull/3722

I think you are right, we may want to exclude a few problematic files: https://github.com/biopython/biopython/issues/3721

Thanks.