Pylint checks should be way faster now #10207
Conversation
potiuk
force-pushed
the
speedup-pylint-tests
branch
7 times, most recently
from
6ec1b71 to
7f87ca6
Compare
Instead of running separate pylint checks for tests and main source we are running a single check now. This is possible thanks to a nice hack - we have pylint plugin that injects the right "# pylint: disable=" comment for all test files while reading the file content by astroid (just before tokenization) Thanks to that we can also separate out pylint checks to a separate job in CI - this way all pylint checks will be run in parallel to all other checks effectively halfing the time needed to get the static check feedback and potentially cancelling other jobs much faster.
potiuk
force-pushed
the
speedup-pylint-tests
branch
from
7f87ca6 to
f402c40
Compare
Codecov Report
@@ Coverage Diff @@
## master #10207 +/- ##
===========================================
- Coverage 89.41% 35.11% -54.30%
===========================================
Files 1037 1037
Lines 50011 50013 +2
===========================================
- Hits 44717 17562 -27155
- Misses 5294 32451 +27157
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
| mod.file_bytes = "\n".join(decoded_lines).encode("utf-8") | ||
|
|
||
|
|
||
| MANAGER.register_transform(scoped_nodes.Module, transform) |
There was a problem hiding this comment.
I'm just wondering if we can first run pylint for main sources and then, just add additional disables to pylintrc? Not sure if this will be simpler
There was a problem hiding this comment.
This is exactly what we did before and that was a pain. It took much more time than this one.
This is a huge optimization, that's why I did it.
The problem is that when you run pylint for tests, it also pulls in all the tested code which means that basically a lot of the main airflow code was parsed and processed twice - once in the "pylint main" and once in the "pylint tests". This code was not verified by Pylint but it was parsed and analyzed so that the test code could be pylint-checked so it's not a full duplication, but I think the savings are significant
Some random stats:
Before the change:
pylint main: 6:20s
pylint tests: 3:59s
Tota: 10m 20s
After the change:
combined pylint: 8:20s
So we save 20% (2 minutes) of elapsed time by doing this.
|
I also added yet another small speedup - I realised that when we split pre-commits we should have two separate pre-comit virtualenv caches - that should give another 30-40 seconds boost overall. |
Instead of running separate pylint checks for tests and main source
we are running a single check now. This is possible thanks to a
nice hack - we have pylint plugin that injects the right
"# pylint: disable=" comment for all test files while reading
the file content by astroid (just before tokenization)
Thanks to that we can also separate out pylint checks
to a separate job in CI - this way all pylint checks will
be run in parallel to all other checks effectively halving
the time needed to get the static check feedback and potentially
canceling other jobs much faster.
^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code change, Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in UPDATING.md.