Avoid network/index related imports for commands that won't need 'em #12566
Conversation
Running `pip show packaging` now results in 420 entries in sys.modules instead of 607.
This notably helps pip freeze which doesn't need the network unless a URL is given as a requirement file.
So freeze doesn't need to import the index/network modules.
... to avoid importing all of the network/index related modules.
tests/unit/test_commands.py
Outdated
| @pytest.mark.parametrize( | ||
| "command", ["cache", "check", "config", "freeze", "hash", "help", "inspect", "show"] | ||
| ) | ||
| def test_no_network_imports(command: str) -> None: |
There was a problem hiding this comment.
This test is admittedly janky, I would appreciate ideas for preventing regressions that are cleaner :)
There was a problem hiding this comment.
A more robust approach that doesn't require using the internals would be to use runpy. You could do something like:
import runpy
import sys
sys.argv[1:] = [{command}, "--help"]
try:
runpy.run_module("pip", alter_sys=True, run_name="__main__")
finally:
with open({file!r}, "w") as f:
print(list(sys.modules.keys()), file=f)and then have file and command templated in as f-strings (file being under tmp_path fixture's folder would be cleanest, I think).
Also this test creates a bunch of subprocesses, and should live under integration tests for that reason.
There was a problem hiding this comment.
Ah, that's much cleaner. Thanks a ton!
tests/unit/test_commands.py
Outdated
| @pytest.mark.parametrize( | ||
| "command", ["cache", "check", "config", "freeze", "hash", "help", "inspect", "show"] | ||
| ) | ||
| def test_no_network_imports(command: str) -> None: |
There was a problem hiding this comment.
A more robust approach that doesn't require using the internals would be to use runpy. You could do something like:
import runpy
import sys
sys.argv[1:] = [{command}, "--help"]
try:
runpy.run_module("pip", alter_sys=True, run_name="__main__")
finally:
with open({file!r}, "w") as f:
print(list(sys.modules.keys()), file=f)and then have file and command templated in as f-strings (file being under tmp_path fixture's folder would be cleanest, I think).
Also this test creates a bunch of subprocesses, and should live under integration tests for that reason.
|
Thanks @pradyunsg for the review! You'll also like to hear that I filed a PR updating tenacity to lazy import Rewriting the test to take the difference from the command registry has revealed that |
Co-authored-by: Pradyun Gedam <[email protected]>
Towards #4768.
These imports are slow and should be avoided as much as possible. For example, this patch brought down the execution time of
pip cache dirfrom 200ms to 160ms locally.pip listis a bit special as it's currently an IndexGroupCommand and performs a pip version self-check. Speedingpip listcan be done when #11677 is addressed.