Skip to content

Conversation

@DanielNoord
Copy link
Collaborator

  • Add yourself to CONTRIBUTORS if you are a new contributor.
  • Write a good description on what the PR does.

Type of Changes

Type
🔨 Refactoring

Description

As discussed in #5816.

I'm confident that these times are more than enough. If we exceed them it is not due to tests really taking that long but rather because something else is broken. Actually helps to see that sooner rather than later.

@DanielNoord DanielNoord added the Maintenance Discussion or action around maintaining pylint or the dev workflow label Feb 22, 2022
@DanielNoord DanielNoord added this to the 2.13.0 milestone Feb 22, 2022
Copy link
Member

@Pierre-Sassoulas Pierre-Sassoulas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if the issues is on github's cloud side ? We're going to wonder what's the issue every time for nothing (in fact we already did for the upgrade of type-toml which is a minor package that should not make the execution time longer, and for pytest where it was legitimate to check. I don't think time is a reliable benchmark for performance when we don't control or even know what machine the code is run on.

@DanielNoord
Copy link
Collaborator Author

@Pierre-Sassoulas I think (but I said that before) that #5827 should really solve these issues.

Nevertheless, these timeouts did actually show a real issue with our tests (wrongly assuming the number of available cores). If there is something actually broken with GitHub cloud services we might want to remove them, but considering that I have not seen any news or comments within the larger python community about problems with python, pytest, Github actions and dependency upgrades this was/is more than likely an issue on our side.

If everything is good then these timeouts should never be reached and can thus be used to spot problems earlier (instead of after the default 6 hours). If the identified problem is with Github Cloud we can still merge as we just need them to update it. But if the identified problem is with us these timeouts will probably help debugging such issues as you're not unnecessarily waiting for something to time out after 30 minutes.

@Pierre-Sassoulas Pierre-Sassoulas merged commit 6622d10 into main Feb 22, 2022
@Pierre-Sassoulas Pierre-Sassoulas deleted the revert-5755-ci-timeouts-2 branch February 22, 2022 16:53
@coveralls
Copy link

coveralls commented Feb 22, 2022

Pull Request Test Coverage Report for Build 1882370291

  • 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.994%

Totals Coverage Status
Change from base Build 1878236493: 0.0%
Covered Lines: 14931
Relevant Lines: 15885

💛 - Coveralls

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