Support automatic worker selection - #21983
Conversation
This comment has been minimized.
This comment has been minimized.
for more information, see https://pre-commit.ci
…into add-auto-workers
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
hmm, seems like because the CI doesn't like that im using these mocks |
| The ``MYPY_NUM_WORKERS`` environment variable accepts the same values and | ||
| overrides this setting. |
There was a problem hiding this comment.
The order of precedence is: command line flag > environment variable > configuration file; yes? Does that match the other options?
There was a problem hiding this comment.
Yes, I think this is the standard order for mypy.
There was a problem hiding this comment.
Yea, it follows the cache_dir behaviour
| from io import StringIO | ||
| from pathlib import Path | ||
| from typing import Any, cast | ||
| from unittest import mock |
There was a problem hiding this comment.
in general, use pytest.monkeypatch instead of unittest; though neither can mock mypyc compiled code
There was a problem hiding this comment.
Yes, @KevinRK29 mypyc doesn't support monkey patching. Instead, you can try setting (private) cached value like this:
mypy.util._AVAILABLE_THREADS = 32
# <...testing...>
mypy.util._AVAILABLE_THREADS = NoneYou can even write a simple context manager to do this (but do not expose it, keep it private to this test file)
There was a problem hiding this comment.
@KevinRK29 to be clear mock.patch.dict is fine, the problem is only with trying to patch a function.
There was a problem hiding this comment.
i see, i've updated it to use the context manager approach
ilevkivskyi
left a comment
ilevkivskyi
left a comment
There was a problem hiding this comment.
LG, thanks! I have few comments.
| * Parallel mode requires and automatically enables :option:`--native-parser`. | ||
| * Parallel mode requires and automatically enables :option:`--native-parser` | ||
| and :ref:`incremental mode <incremental>`. Specifying | ||
| :option:`--no-incremental` has no effect in parallel mode. |
There was a problem hiding this comment.
I was thinking more about this, and silently overriding it may be not the best thing. People may need it for some niche things like running benchmarks. So we should respect it, but show a warning instead. More on this below.
There was a problem hiding this comment.
yea that makes sense to me
| if options.num_workers: | ||
| # Supporting both parsers would be really tricky, so just support the new one. | ||
| options.native_parser = True | ||
| options.incremental = True |
There was a problem hiding this comment.
I think we should do something like this here instead:
if not options.incremental and os.path.isdir(options.cache_dir):
print("Warning: disabling incremental mode may severely reduce performance")
print(f"If this is intentional, delete '{options.cache_dir}' to suppress this warning")There was a problem hiding this comment.
used this approach
| metavar="VALUE", | ||
| default=0, | ||
| help="Number of separate mypy worker processes (experimental)", | ||
| help="Number of separate mypy worker processes, or 'auto' (experimental)", |
There was a problem hiding this comment.
I don't think we need to say that parallel type checking is experimental anymore.
There was a problem hiding this comment.
removed the experimental wording
Co-authored-by: Ivan Levkivskyi <levkivskyi@gmail.com>
|
According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅ |
ilevkivskyi
left a comment
ilevkivskyi
left a comment
There was a problem hiding this comment.
Thanks! This is ready now.
3 participants
Fixes #21477
Allow
autoanywhere the number of parallel workers can be configured.The auto selection reuses
get_available_threadsand is capped at 8 workers. You can still exceed the cap by specifying the number of workers.Also
0still disables parallel checking,1selects one worker, and the default remains0.Also this displays a warning when incremental mode is disabled (when using parallel checking)