Skip to content

Commit

Permalink
Merge pull request #12566 from ichard26/lazy-imports
Browse files Browse the repository at this point in the history
Avoid network/index related imports for commands that won't need 'em
  • Loading branch information
pradyunsg committed Apr 9, 2024
2 parents 06d21db + 23f4ad5 commit 550388e
Show file tree
Hide file tree
Showing 8 changed files with 82 additions and 22 deletions.
1 change: 1 addition & 0 deletions news/4768.feature.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Reduce startup time of commands (e.g. show, freeze) that do not access the network by 15-30%.
2 changes: 1 addition & 1 deletion src/pip/_internal/commands/inspect.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

from pip import __version__
from pip._internal.cli import cmdoptions
from pip._internal.cli.req_command import Command
from pip._internal.cli.base_command import Command
from pip._internal.cli.status_codes import SUCCESS
from pip._internal.metadata import BaseDistribution, get_environment
from pip._internal.utils.compat import stdlib_pkgs
Expand Down
8 changes: 5 additions & 3 deletions src/pip/_internal/distributions/base.py
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
import abc
from typing import Optional
from typing import TYPE_CHECKING, Optional

from pip._internal.index.package_finder import PackageFinder
from pip._internal.metadata.base import BaseDistribution
from pip._internal.req import InstallRequirement

if TYPE_CHECKING:
from pip._internal.index.package_finder import PackageFinder


class AbstractDistribution(metaclass=abc.ABCMeta):
"""A base class for handling installable artifacts.
Expand Down Expand Up @@ -44,7 +46,7 @@ def get_metadata_distribution(self) -> BaseDistribution:
@abc.abstractmethod
def prepare_distribution_metadata(
self,
finder: PackageFinder,
finder: "PackageFinder",
build_isolation: bool,
check_build_deps: bool,
) -> None:
Expand Down
12 changes: 7 additions & 5 deletions src/pip/_internal/distributions/sdist.py
Original file line number Diff line number Diff line change
@@ -1,13 +1,15 @@
import logging
from typing import Iterable, Optional, Set, Tuple
from typing import TYPE_CHECKING, Iterable, Optional, Set, Tuple

from pip._internal.build_env import BuildEnvironment
from pip._internal.distributions.base import AbstractDistribution
from pip._internal.exceptions import InstallationError
from pip._internal.index.package_finder import PackageFinder
from pip._internal.metadata import BaseDistribution
from pip._internal.utils.subprocess import runner_with_spinner_message

if TYPE_CHECKING:
from pip._internal.index.package_finder import PackageFinder

logger = logging.getLogger(__name__)


Expand All @@ -29,7 +31,7 @@ def get_metadata_distribution(self) -> BaseDistribution:

def prepare_distribution_metadata(
self,
finder: PackageFinder,
finder: "PackageFinder",
build_isolation: bool,
check_build_deps: bool,
) -> None:
Expand Down Expand Up @@ -66,7 +68,7 @@ def prepare_distribution_metadata(
self._raise_missing_reqs(missing)
self.req.prepare_metadata()

def _prepare_build_backend(self, finder: PackageFinder) -> None:
def _prepare_build_backend(self, finder: "PackageFinder") -> None:
# Isolate in a BuildEnvironment and install the build-time
# requirements.
pyproject_requires = self.req.pyproject_requires
Expand Down Expand Up @@ -110,7 +112,7 @@ def _get_build_requires_editable(self) -> Iterable[str]:
with backend.subprocess_runner(runner):
return backend.get_requires_for_build_editable()

def _install_build_reqs(self, finder: PackageFinder) -> None:
def _install_build_reqs(self, finder: "PackageFinder") -> None:
# Install any extra build dependencies that the backend requests.
# This must be done in a second pass, as the pyproject.toml
# dependencies must be installed before we can call the backend.
Expand Down
8 changes: 5 additions & 3 deletions src/pip/_internal/distributions/wheel.py
Original file line number Diff line number Diff line change
@@ -1,15 +1,17 @@
from typing import Optional
from typing import TYPE_CHECKING, Optional

from pip._vendor.packaging.utils import canonicalize_name

from pip._internal.distributions.base import AbstractDistribution
from pip._internal.index.package_finder import PackageFinder
from pip._internal.metadata import (
BaseDistribution,
FilesystemWheel,
get_wheel_distribution,
)

if TYPE_CHECKING:
from pip._internal.index.package_finder import PackageFinder


class WheelDistribution(AbstractDistribution):
"""Represents a wheel distribution.
Expand All @@ -33,7 +35,7 @@ def get_metadata_distribution(self) -> BaseDistribution:

def prepare_distribution_metadata(
self,
finder: PackageFinder,
finder: "PackageFinder",
build_isolation: bool,
check_build_deps: bool,
) -> None:
Expand Down
7 changes: 4 additions & 3 deletions src/pip/_internal/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,15 @@
from itertools import chain, groupby, repeat
from typing import TYPE_CHECKING, Dict, Iterator, List, Literal, Optional, Union

from pip._vendor.requests.models import Request, Response
from pip._vendor.rich.console import Console, ConsoleOptions, RenderResult
from pip._vendor.rich.markup import escape
from pip._vendor.rich.text import Text

if TYPE_CHECKING:
from hashlib import _Hash

from pip._vendor.requests.models import Request, Response

from pip._internal.metadata import BaseDistribution
from pip._internal.req.req_install import InstallRequirement

Expand Down Expand Up @@ -293,8 +294,8 @@ class NetworkConnectionError(PipError):
def __init__(
self,
error_msg: str,
response: Optional[Response] = None,
request: Optional[Request] = None,
response: Optional["Response"] = None,
request: Optional["Request"] = None,
) -> None:
"""
Initialize NetworkConnectionError with `request` and `response`
Expand Down
16 changes: 9 additions & 7 deletions src/pip/_internal/req/req_file.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,13 +25,12 @@
from pip._internal.cli import cmdoptions
from pip._internal.exceptions import InstallationError, RequirementsFileParseError
from pip._internal.models.search_scope import SearchScope
from pip._internal.network.session import PipSession
from pip._internal.network.utils import raise_for_status
from pip._internal.utils.encoding import auto_decode
from pip._internal.utils.urls import get_url_scheme

if TYPE_CHECKING:
from pip._internal.index.package_finder import PackageFinder
from pip._internal.network.session import PipSession

__all__ = ["parse_requirements"]

Expand Down Expand Up @@ -133,7 +132,7 @@ def __init__(

def parse_requirements(
filename: str,
session: PipSession,
session: "PipSession",
finder: Optional["PackageFinder"] = None,
options: Optional[optparse.Values] = None,
constraint: bool = False,
Expand Down Expand Up @@ -210,7 +209,7 @@ def handle_option_line(
lineno: int,
finder: Optional["PackageFinder"] = None,
options: Optional[optparse.Values] = None,
session: Optional[PipSession] = None,
session: Optional["PipSession"] = None,
) -> None:
if opts.hashes:
logger.warning(
Expand Down Expand Up @@ -278,7 +277,7 @@ def handle_line(
line: ParsedLine,
options: Optional[optparse.Values] = None,
finder: Optional["PackageFinder"] = None,
session: Optional[PipSession] = None,
session: Optional["PipSession"] = None,
) -> Optional[ParsedRequirement]:
"""Handle a single parsed requirements line; This can result in
creating/yielding requirements, or updating the finder.
Expand Down Expand Up @@ -321,7 +320,7 @@ def handle_line(
class RequirementsFileParser:
def __init__(
self,
session: PipSession,
session: "PipSession",
line_parser: LineParser,
) -> None:
self._session = session
Expand Down Expand Up @@ -526,7 +525,7 @@ def expand_env_variables(lines_enum: ReqFileLines) -> ReqFileLines:
yield line_number, line


def get_file_content(url: str, session: PipSession) -> Tuple[str, str]:
def get_file_content(url: str, session: "PipSession") -> Tuple[str, str]:
"""Gets the content of a file; it may be a filename, file: URL, or
http: URL. Returns (location, content). Content is unicode.
Respects # -*- coding: declarations on the retrieved files.
Expand All @@ -538,6 +537,9 @@ def get_file_content(url: str, session: PipSession) -> Tuple[str, str]:

# Pip has special support for file:// URLs (LocalFSAdapter).
if scheme in ["http", "https", "file"]:
# Delay importing heavy network modules until absolutely necessary.
from pip._internal.network.utils import raise_for_status

resp = session.get(url)
raise_for_status(resp)
return resp.url, resp.text
Expand Down
50 changes: 50 additions & 0 deletions tests/functional/test_cli.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,13 @@
"""Basic CLI functionality checks.
"""
import subprocess
import sys
from pathlib import Path
from textwrap import dedent

import pytest

from pip._internal.commands import commands_dict
from tests.lib import PipTestEnvironment


Expand Down Expand Up @@ -45,3 +49,49 @@ def test_entrypoints_work(entrypoint: str, script: PipTestEnvironment) -> None:
result2 = script.run("fake_pip", "-V", allow_stderr_warning=True)
assert result.stdout == result2.stdout
assert "old script wrapper" in result2.stderr


@pytest.mark.parametrize(
"command",
sorted(
set(commands_dict).symmetric_difference(
# Exclude commands that are expected to use the network.
# TODO: uninstall and list should only import network modules as needed
{"install", "uninstall", "download", "search", "index", "wheel", "list"}
)
),
)
def test_no_network_imports(command: str, tmp_path: Path) -> None:
"""
Verify that commands that don't access the network do NOT import network code.
This helps to reduce the startup time of these commands.
Note: This won't catch lazy network imports, but it'll catch top-level
network imports which were accidently added (which is the most likely way
to regress anyway).
"""
file = tmp_path / f"imported_modules_for_{command}.txt"
code = f"""
import runpy
import sys
sys.argv[1:] = [{command!r}, "--help"]
try:
runpy.run_module("pip", alter_sys=True, run_name="__main__")
finally:
with open({str(file)!r}, "w") as f:
print(*sys.modules.keys(), sep="\\n", file=f)
"""
subprocess.run(
[sys.executable],
input=code,
encoding="utf-8",
check=True,
)
imported = file.read_text().splitlines()
assert not any("pip._internal.index" in mod for mod in imported)
assert not any("pip._internal.network" in mod for mod in imported)
assert not any("requests" in mod for mod in imported)
assert not any("urllib3" in mod for mod in imported)

0 comments on commit 550388e

Please sign in to comment.