-
Notifications
You must be signed in to change notification settings - Fork 359
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Make pylint work again #5269
Make pylint work again #5269
Conversation
|
c0fd4ca
to
75e7589
Compare
Hello @VladimirSlavik! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:
Comment last updated at 2024-02-22 08:32:46 UTC |
75e7589
to
f07092b
Compare
/kickstart-test --testtype smoke |
d6a5ed0
to
02d3dd9
Compare
Looks like astroid dies on parsing subscription now. Can't reproduce locally. Lovely. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great to me. Thanks!
I wonder if you don't have different version of a container image locally? |
That's a tricky question, as rebuilds are also stuck due to new dnf breaking tests... Generally speaking, this can wait a bit. |
d2a75e5
to
a7cac8d
Compare
Still fails here and succeeds locally. I'll try filing the astroid bug implied by the results. |
a7cac8d
to
acc7a67
Compare
Rebased. The failure persists. It seems I can reproduce that locally, sporadically: When I rebuild the CI container, the first test run in that container fails, sometimes. Re-running the tests will make them succeed. Rebuilding the container does not seem to negate the success on its own. So it's unpredictable anyway. |
acc7a67
to
f025478
Compare
f025478
to
7617c26
Compare
7617c26
to
f8bd6e7
Compare
Invoking the standalone containerized pylint make target now also: - propagates error code - provides logs
f8bd6e7
to
9f88445
Compare
Closing in favor of #5486 which has cherry-picked these commits. But at least we can stop spanning Vladimir's account. |
Resolves:
TODO:
E0602(undefined-variable):ui/webui/test/check-basic:227,16: TestBasic.testJsErrorHandling: Undefined variable 'time'
E1101(no-member):ui/webui/test/helpers/storage.py:414,8: StorageMountPointMapping.select_mountpoint: Instance of 'StorageMountPointMapping' has no 'set_partitioning' member
I'm rather stumped by the last two finds listed above, there's no clear reason how these actually work.After this is done, we can make another PR where we either make pylint into another gh test alongside unit tests, or put it back with them as it was. I left out this part intentionally, so that we can have the fixes without the other decisions.