Skip to content

Add timeouts for CI jobs#5752

Merged
cdce8p merged 1 commit intopylint-dev:mainfrom
cdce8p:ci-timeouts
Jan 31, 2022
Merged

Add timeouts for CI jobs#5752
cdce8p merged 1 commit intopylint-dev:mainfrom
cdce8p:ci-timeouts

Conversation

@cdce8p
Copy link
Copy Markdown
Member

@cdce8p cdce8p commented Jan 31, 2022

Description

I recently had a job run for 6h just to fail. Usually they all complete in <10min (CI) / <30min (Primer). So anything that is taking longer than that is probably going to fail anyway.

This PR adds timeout-minutes to the individual jobs to abort them after a reasonable amount of time. Usually 2x what they actually need. https://docs.github.com/en/actions/using-workflows/workflow-syntax-for-github-actions#jobsjob_idtimeout-minutes

/CC: @DanielNoord Is 60min enough for Primer jobs? From what I've seen an individual one usually takes 20min.

@cdce8p cdce8p added the Maintenance Discussion or action around maintaining pylint or the dev workflow label Jan 31, 2022
@DanielNoord
Copy link
Copy Markdown
Collaborator

60 minutes is more than enough! I would even consider setting it to 45 or 50, that's more than twice they usually take.

@cdce8p
Copy link
Copy Markdown
Member Author

cdce8p commented Jan 31, 2022

60 minutes is more than enough! I would even consider setting it to 45 or 50, that's more than twice they usually take.

👍🏻 We can leave some headroom. It's still much better than the default 6h timeout. Usually those are issues with Github Actions anyway.

@coveralls
Copy link
Copy Markdown

coveralls commented Jan 31, 2022

Pull Request Test Coverage Report for Build 1774619958

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 93.878%

Totals Coverage Status
Change from base Build 1772737787: 0.0%
Covered Lines: 14767
Relevant Lines: 15730

💛 - Coveralls

@Pierre-Sassoulas Pierre-Sassoulas added this to the 2.13.0 milestone Jan 31, 2022
@Pierre-Sassoulas
Copy link
Copy Markdown
Member

Great catch @cdce8p ! Should we change to 45mn before merging ?

@cdce8p
Copy link
Copy Markdown
Member Author

cdce8p commented Jan 31, 2022

Should we change to 45mn before merging ?

Don't think that's necessary. The primer tests are somewhat separate from the rest already. 1h also isn't that long. Still an improvement over 6h though.

@cdce8p cdce8p merged commit 1d2c51c into pylint-dev:main Jan 31, 2022
@cdce8p cdce8p deleted the ci-timeouts branch January 31, 2022 20:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Maintenance Discussion or action around maintaining pylint or the dev workflow

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants