-
-
Notifications
You must be signed in to change notification settings - Fork 3.3k
Support automatic worker selection #21983
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
Changes from all commits
f38d10b
b72b92b
d343293
53e4150
9076099
cedfd22
7aa5729
5a7aaba
dabc51b
03ca7ff
bf91acb
d73e581
f88e779
2158d1c
13be468
8a7fcd6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -8,15 +8,139 @@ | |
| from __future__ import annotations | ||
|
|
||
| import argparse | ||
| import contextlib | ||
| import os | ||
| import sys | ||
| import tempfile | ||
| from collections.abc import Iterator | ||
| from io import StringIO | ||
| from pathlib import Path | ||
| from typing import Any, cast | ||
| from unittest import mock | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. in general, use
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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)
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @KevinRK29 to be clear
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i see, i've updated it to use the context manager approach |
||
|
|
||
| from mypy.main import infer_python_executable, process_options | ||
| from mypy import util | ||
| from mypy.config_parser import parse_num_workers | ||
| from mypy.main import infer_python_executable, main, process_options | ||
| from mypy.options import Options | ||
| from mypy.test.helpers import Suite | ||
|
|
||
|
|
||
| @contextlib.contextmanager | ||
| def _available_threads(value: int) -> Iterator[None]: | ||
| previous = util._AVAILABLE_THREADS | ||
| util._AVAILABLE_THREADS = value | ||
| try: | ||
| yield | ||
| finally: | ||
| util._AVAILABLE_THREADS = previous | ||
|
|
||
|
|
||
| class ArgSuite(Suite): | ||
| def test_parse_num_workers(self) -> None: | ||
| with _available_threads(4): | ||
| assert parse_num_workers("auto") == 4 | ||
| with _available_threads(32): | ||
| assert parse_num_workers("auto") == 8 | ||
|
|
||
| assert parse_num_workers(12) == 12 | ||
| assert parse_num_workers("12") == 12 | ||
|
|
||
| assert parse_num_workers("0") == 0 | ||
| assert parse_num_workers("1") == 1 | ||
|
|
||
| assert parse_num_workers("-1") == -1 | ||
| with self.assertRaises(argparse.ArgumentTypeError): | ||
| parse_num_workers("automatic") | ||
|
|
||
| def test_num_workers_auto_from_command_line_and_environment(self) -> None: | ||
| with _available_threads(6): | ||
| with mock.patch.dict(os.environ, {"MYPY_NUM_WORKERS": ""}): | ||
| _, cli_options = process_options( | ||
| ["--config-file=", "--num-workers=auto"], require_targets=False | ||
| ) | ||
| with mock.patch.dict(os.environ, {"MYPY_NUM_WORKERS": "auto"}): | ||
| _, env_options = process_options(["--config-file="], require_targets=False) | ||
|
|
||
| assert cli_options.num_workers == 6 | ||
| assert env_options.num_workers == 6 | ||
|
|
||
| def test_invalid_num_workers_environment(self) -> None: | ||
| stderr = StringIO() | ||
| with mock.patch.dict(os.environ, {"MYPY_NUM_WORKERS": "automatic"}): | ||
| with self.assertRaises(SystemExit) as context: | ||
| process_options( | ||
| ["--config-file="], require_targets=False, stdout=StringIO(), stderr=stderr | ||
| ) | ||
|
|
||
| assert context.exception.code == 2 | ||
| assert "MYPY_NUM_WORKERS: Invalid number of workers 'automatic'" in stderr.getvalue() | ||
|
|
||
| def test_num_workers_auto_from_config(self) -> None: | ||
| configs = ( | ||
| ("mypy.ini", "[mypy]\nnum_workers = auto\n"), | ||
| ("pyproject.toml", '[tool.mypy]\nnum_workers = "auto"\n'), | ||
| ) | ||
| with tempfile.TemporaryDirectory() as temp_dir: | ||
| with _available_threads(6): | ||
| for filename, contents in configs: | ||
| config = Path(temp_dir) / filename | ||
| config.write_text(contents, encoding="utf-8") | ||
| with mock.patch.dict(os.environ, {"MYPY_NUM_WORKERS": ""}): | ||
| _, options = process_options( | ||
| ["--config-file", str(config)], require_targets=False | ||
| ) | ||
| assert options.num_workers == 6, filename | ||
|
|
||
| def test_num_workers_precedence(self) -> None: | ||
| with tempfile.TemporaryDirectory() as temp_dir: | ||
| config = Path(temp_dir) / "mypy.ini" | ||
| config.write_text("[mypy]\nnum_workers = 2\n", encoding="utf-8") | ||
|
|
||
| with _available_threads(4): | ||
| with mock.patch.dict(os.environ, {"MYPY_NUM_WORKERS": ""}): | ||
| _, config_options = process_options( | ||
| ["--config-file", str(config)], require_targets=False | ||
| ) | ||
|
|
||
| with mock.patch.dict(os.environ, {"MYPY_NUM_WORKERS": "7"}): | ||
| _, env_options = process_options( | ||
| ["--config-file", str(config)], require_targets=False | ||
| ) | ||
|
|
||
| _, cli_options = process_options( | ||
| ["--config-file", str(config), "--num-workers=auto"], require_targets=False | ||
| ) | ||
|
|
||
| assert config_options.num_workers == 2 | ||
| assert env_options.num_workers == 7 | ||
| assert cli_options.num_workers == 4 | ||
|
|
||
| def test_parallel_mode_warns_when_incremental_disabled(self) -> None: | ||
| with tempfile.TemporaryDirectory() as cache_dir: | ||
| stdout = StringIO() | ||
| stderr = StringIO() | ||
| main( | ||
| args=[ | ||
| "--config-file=", | ||
| "--num-workers=1", | ||
| "--no-incremental", | ||
| "--no-site-packages", | ||
| "--no-error-summary", | ||
| f"--cache-dir={cache_dir}", | ||
| "-c", | ||
| "pass", | ||
| ], | ||
| stdout=stdout, | ||
| stderr=stderr, | ||
| clean_exit=True, | ||
| ) | ||
|
|
||
| assert stdout.getvalue() == ( | ||
| "Warning: disabling incremental mode may severely reduce performance\n" | ||
| f"If this is intentional, delete '{cache_dir}' to suppress this warning\n" | ||
| ) | ||
| assert stderr.getvalue() == "" | ||
|
|
||
| def test_coherence(self) -> None: | ||
| options = Options() | ||
| _, parsed_options = process_options([], require_targets=False) | ||
|
|
||
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.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I think this is the standard order for mypy.
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.
Yea, it follows the
cache_dirbehaviour