From e62f16e96938ee24e7a57168b829942526be56e2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nguy=E1=BB=85n=20Gia=20Phong?= Date: Sun, 9 Aug 2020 15:55:33 +0700 Subject: [PATCH 1/6] Make Downloader perform the download --- src/pip/_internal/network/download.py | 30 +++++++----------------- src/pip/_internal/operations/prepare.py | 31 ++++--------------------- tests/unit/test_operations_prepare.py | 13 ++--------- 3 files changed, 15 insertions(+), 59 deletions(-) diff --git a/src/pip/_internal/network/download.py b/src/pip/_internal/network/download.py index 44f9985a3..a4d4bd2a5 100644 --- a/src/pip/_internal/network/download.py +++ b/src/pip/_internal/network/download.py @@ -24,7 +24,7 @@ from pip._internal.utils.misc import ( from pip._internal.utils.typing import MYPY_CHECK_RUNNING if MYPY_CHECK_RUNNING: - from typing import Iterable, Optional + from typing import Iterable, Optional, Tuple from pip._vendor.requests.models import Response @@ -141,19 +141,6 @@ def _http_get_download(session, link): return resp -class Download(object): - def __init__( - self, - response, # type: Response - filename, # type: str - chunks, # type: Iterable[bytes] - ): - # type: (...) -> None - self.response = response - self.filename = filename - self.chunks = chunks - - class Downloader(object): def __init__( self, @@ -164,8 +151,8 @@ class Downloader(object): self._session = session self._progress_bar = progress_bar - def __call__(self, link): - # type: (Link) -> Download + def __call__(self, link, location): + # type: (Link, str) -> Tuple[str, str] try: resp = _http_get_download(self._session, link) except NetworkConnectionError as e: @@ -175,8 +162,9 @@ class Downloader(object): ) raise - return Download( - resp, - _get_http_response_filename(resp, link), - _prepare_download(resp, link, self._progress_bar), - ) + filename = _get_http_response_filename(resp, link) + chunks = _prepare_download(resp, link, self._progress_bar) + with open(os.path.join(location, filename), 'wb') as content_file: + for chunk in chunks: + content_file.write(chunk) + return content_file.name, resp.headers.get('Content-Type', '') diff --git a/src/pip/_internal/operations/prepare.py b/src/pip/_internal/operations/prepare.py index e9e534e32..e4de1be4a 100644 --- a/src/pip/_internal/operations/prepare.py +++ b/src/pip/_internal/operations/prepare.py @@ -45,9 +45,7 @@ from pip._internal.utils.unpacking import unpack_file from pip._internal.vcs import vcs if MYPY_CHECK_RUNNING: - from typing import ( - Callable, List, Optional, Tuple, - ) + from typing import Callable, List, Optional from mypy_extensions import TypedDict from pip._vendor.pkg_resources import Distribution @@ -132,9 +130,9 @@ def get_http_url( content_type = mimetypes.guess_type(from_path)[0] else: # let's download to a tmp dir - from_path, content_type = _download_http_url( - link, downloader, temp_dir.path, hashes - ) + from_path, content_type = downloader(link, temp_dir.path) + if hashes: + hashes.check_against_path(from_path) return File(from_path, content_type) @@ -273,27 +271,6 @@ def unpack_url( return file -def _download_http_url( - link, # type: Link - downloader, # type: Downloader - temp_dir, # type: str - hashes, # type: Optional[Hashes] -): - # type: (...) -> Tuple[str, str] - """Download link url into temp_dir using provided session""" - download = downloader(link) - - file_path = os.path.join(temp_dir, download.filename) - with open(file_path, 'wb') as content_file: - for chunk in download.chunks: - content_file.write(chunk) - - if hashes: - hashes.check_against_path(file_path) - - return file_path, download.response.headers.get('content-type', '') - - def _check_download_dir(link, download_dir, hashes): # type: (Link, str, Optional[Hashes]) -> Optional[str] """ Check download_dir for previously downloaded file with correct hash diff --git a/tests/unit/test_operations_prepare.py b/tests/unit/test_operations_prepare.py index 41d8be260..d2e4d6091 100644 --- a/tests/unit/test_operations_prepare.py +++ b/tests/unit/test_operations_prepare.py @@ -10,11 +10,7 @@ from pip._internal.exceptions import HashMismatch from pip._internal.models.link import Link from pip._internal.network.download import Downloader from pip._internal.network.session import PipSession -from pip._internal.operations.prepare import ( - _copy_source_tree, - _download_http_url, - unpack_url, -) +from pip._internal.operations.prepare import _copy_source_tree, unpack_url from pip._internal.utils.hashes import Hashes from pip._internal.utils.urls import path_to_url from tests.lib.filesystem import ( @@ -83,12 +79,7 @@ def test_download_http_url__no_directory_traversal(mock_raise_for_status, download_dir = tmpdir.joinpath('download') os.mkdir(download_dir) - file_path, content_type = _download_http_url( - link, - downloader, - download_dir, - hashes=None, - ) + file_path, content_type = downloader(link, download_dir) # The file should be downloaded to download_dir. actual = os.listdir(download_dir) assert actual == ['out_dir_file'] From 078e0effb72b1078bab3d268aa5b4e374505e18a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nguy=E1=BB=85n=20Gia=20Phong?= Date: Sun, 9 Aug 2020 22:44:20 +0700 Subject: [PATCH 2/6] Add memoization mechanism for file download This is intentionally dependent from caching, which relies on cache dir. --- ...a2b1b7-744e-4533-b3ff-6e7a1843d573.trivial | 0 src/pip/_internal/network/download.py | 9 +++- src/pip/_internal/operations/prepare.py | 43 +++++++++++-------- .../resolution/resolvelib/resolver.py | 6 ++- tests/unit/test_operations_prepare.py | 2 +- 5 files changed, 39 insertions(+), 21 deletions(-) create mode 100644 news/a3a2b1b7-744e-4533-b3ff-6e7a1843d573.trivial diff --git a/news/a3a2b1b7-744e-4533-b3ff-6e7a1843d573.trivial b/news/a3a2b1b7-744e-4533-b3ff-6e7a1843d573.trivial new file mode 100644 index 000000000..e69de29bb diff --git a/src/pip/_internal/network/download.py b/src/pip/_internal/network/download.py index a4d4bd2a5..0eb4fd9ce 100644 --- a/src/pip/_internal/network/download.py +++ b/src/pip/_internal/network/download.py @@ -151,8 +151,9 @@ class Downloader(object): self._session = session self._progress_bar = progress_bar - def __call__(self, link, location): + def download_one(self, link, location): # type: (Link, str) -> Tuple[str, str] + """Download the file given by link into location.""" try: resp = _http_get_download(self._session, link) except NetworkConnectionError as e: @@ -168,3 +169,9 @@ class Downloader(object): for chunk in chunks: content_file.write(chunk) return content_file.name, resp.headers.get('Content-Type', '') + + def download_many(self, links, location): + # type: (Iterable[Link], str) -> Iterable[Tuple[str, Tuple[str, str]]] + """Download the files given by links into location.""" + for link in links: + yield link.url, self.download_one(link, location) diff --git a/src/pip/_internal/operations/prepare.py b/src/pip/_internal/operations/prepare.py index e4de1be4a..5fdbd674b 100644 --- a/src/pip/_internal/operations/prepare.py +++ b/src/pip/_internal/operations/prepare.py @@ -45,7 +45,7 @@ from pip._internal.utils.unpacking import unpack_file from pip._internal.vcs import vcs if MYPY_CHECK_RUNNING: - from typing import Callable, List, Optional + from typing import Callable, Dict, List, Optional, Tuple from mypy_extensions import TypedDict from pip._vendor.pkg_resources import Distribution @@ -130,7 +130,7 @@ def get_http_url( content_type = mimetypes.guess_type(from_path)[0] else: # let's download to a tmp dir - from_path, content_type = downloader(link, temp_dir.path) + from_path, content_type = downloader.download_one(link, temp_dir.path) if hashes: hashes.check_against_path(from_path) @@ -352,6 +352,9 @@ class RequirementPreparer(object): # Should wheels be downloaded lazily? self.use_lazy_wheel = lazy_wheel + # Memoized downloaded files, as mapping of url: (path, mime type) + self._downloaded = {} # type: Dict[str, Tuple[str, str]] + @property def _download_should_save(self): # type: () -> bool @@ -480,12 +483,15 @@ class RequirementPreparer(object): return wheel_dist return self._prepare_linked_requirement(req, parallel_builds) - def prepare_linked_requirement_more(self, req, parallel_builds=False): - # type: (InstallRequirement, bool) -> None + def prepare_linked_requirements_more(self, reqs, parallel_builds=False): + # type: (List[InstallRequirement], bool) -> None """Prepare a linked requirement more, if needed.""" - if not req.needs_more_preparation: - return - self._prepare_linked_requirement(req, parallel_builds) + # Let's download to a temporary directory. + tmpdir = TempDirectory(kind="unpack", globally_managed=True).path + links = (req.link for req in reqs) + self._downloaded.update(self.downloader.download_many(links, tmpdir)) + for req in reqs: + self._prepare_linked_requirement(req, parallel_builds) def _prepare_linked_requirement(self, req, parallel_builds): # type: (InstallRequirement, bool) -> Distribution @@ -499,16 +505,19 @@ class RequirementPreparer(object): with indent_log(): self._ensure_link_req_src_dir(req, download_dir, parallel_builds) - try: - local_file = unpack_url( - link, req.source_dir, self.downloader, download_dir, - hashes=self._get_linked_req_hashes(req) - ) - except NetworkConnectionError as exc: - raise InstallationError( - 'Could not install requirement {} because of HTTP ' - 'error {} for URL {}'.format(req, exc, link) - ) + if link.url in self._downloaded: + local_file = File(*self._downloaded[link.url]) + else: + try: + local_file = unpack_url( + link, req.source_dir, self.downloader, download_dir, + hashes=self._get_linked_req_hashes(req) + ) + except NetworkConnectionError as exc: + raise InstallationError( + 'Could not install requirement {} because of HTTP ' + 'error {} for URL {}'.format(req, exc, link) + ) # For use in later processing, preserve the file path on the # requirement. diff --git a/src/pip/_internal/resolution/resolvelib/resolver.py b/src/pip/_internal/resolution/resolvelib/resolver.py index fde86413d..1cabe236d 100644 --- a/src/pip/_internal/resolution/resolvelib/resolver.py +++ b/src/pip/_internal/resolution/resolvelib/resolver.py @@ -160,8 +160,10 @@ class Resolver(BaseResolver): req_set.add_named_requirement(ireq) - for actual_req in req_set.all_requirements: - self.factory.preparer.prepare_linked_requirement_more(actual_req) + self.factory.preparer.prepare_linked_requirements_more([ + req for req in req_set.all_requirements + if req.needs_more_preparation + ]) return req_set diff --git a/tests/unit/test_operations_prepare.py b/tests/unit/test_operations_prepare.py index d2e4d6091..e90eab8d7 100644 --- a/tests/unit/test_operations_prepare.py +++ b/tests/unit/test_operations_prepare.py @@ -79,7 +79,7 @@ def test_download_http_url__no_directory_traversal(mock_raise_for_status, download_dir = tmpdir.joinpath('download') os.mkdir(download_dir) - file_path, content_type = downloader(link, download_dir) + file_path, content_type = downloader.download_one(link, download_dir) # The file should be downloaded to download_dir. actual = os.listdir(download_dir) assert actual == ['out_dir_file'] From 39d296eeb8cb8b0f8e09493bc8f1cb6eb5940a46 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nguy=E1=BB=85n=20Gia=20Phong?= Date: Mon, 10 Aug 2020 22:24:00 +0700 Subject: [PATCH 3/6] Clean up code style and internal interface Co-Authored-By: Pradyun Gedam Co-Authored-By: Chris Hunt --- src/pip/_internal/network/download.py | 7 +++++-- src/pip/_internal/operations/prepare.py | 12 +++++++----- src/pip/_internal/resolution/resolvelib/resolver.py | 7 ++----- 3 files changed, 14 insertions(+), 12 deletions(-) diff --git a/src/pip/_internal/network/download.py b/src/pip/_internal/network/download.py index 0eb4fd9ce..5fd277330 100644 --- a/src/pip/_internal/network/download.py +++ b/src/pip/_internal/network/download.py @@ -164,11 +164,14 @@ class Downloader(object): raise filename = _get_http_response_filename(resp, link) + filepath = os.path.join(location, filename) + chunks = _prepare_download(resp, link, self._progress_bar) - with open(os.path.join(location, filename), 'wb') as content_file: + with open(filepath, 'wb') as content_file: for chunk in chunks: content_file.write(chunk) - return content_file.name, resp.headers.get('Content-Type', '') + content_type = resp.headers.get('Content-Type', '') + return filepath, content_type def download_many(self, links, location): # type: (Iterable[Link], str) -> Iterable[Tuple[str, Tuple[str, str]]] diff --git a/src/pip/_internal/operations/prepare.py b/src/pip/_internal/operations/prepare.py index 5fdbd674b..5f2a0c74f 100644 --- a/src/pip/_internal/operations/prepare.py +++ b/src/pip/_internal/operations/prepare.py @@ -45,7 +45,7 @@ from pip._internal.utils.unpacking import unpack_file from pip._internal.vcs import vcs if MYPY_CHECK_RUNNING: - from typing import Callable, Dict, List, Optional, Tuple + from typing import Callable, Dict, Iterable, List, Optional, Tuple from mypy_extensions import TypedDict from pip._vendor.pkg_resources import Distribution @@ -484,8 +484,10 @@ class RequirementPreparer(object): return self._prepare_linked_requirement(req, parallel_builds) def prepare_linked_requirements_more(self, reqs, parallel_builds=False): - # type: (List[InstallRequirement], bool) -> None + # type: (Iterable[InstallRequirement], bool) -> None """Prepare a linked requirement more, if needed.""" + reqs = [req for req in reqs if req.needs_more_preparation] + # Let's download to a temporary directory. tmpdir = TempDirectory(kind="unpack", globally_managed=True).path links = (req.link for req in reqs) @@ -505,9 +507,7 @@ class RequirementPreparer(object): with indent_log(): self._ensure_link_req_src_dir(req, download_dir, parallel_builds) - if link.url in self._downloaded: - local_file = File(*self._downloaded[link.url]) - else: + if link.url not in self._downloaded: try: local_file = unpack_url( link, req.source_dir, self.downloader, download_dir, @@ -518,6 +518,8 @@ class RequirementPreparer(object): 'Could not install requirement {} because of HTTP ' 'error {} for URL {}'.format(req, exc, link) ) + else: + local_file = File(*self._downloaded[link.url]) # For use in later processing, preserve the file path on the # requirement. diff --git a/src/pip/_internal/resolution/resolvelib/resolver.py b/src/pip/_internal/resolution/resolvelib/resolver.py index 1cabe236d..031d2f107 100644 --- a/src/pip/_internal/resolution/resolvelib/resolver.py +++ b/src/pip/_internal/resolution/resolvelib/resolver.py @@ -160,11 +160,8 @@ class Resolver(BaseResolver): req_set.add_named_requirement(ireq) - self.factory.preparer.prepare_linked_requirements_more([ - req for req in req_set.all_requirements - if req.needs_more_preparation - ]) - + reqs = req_set.all_requirements + self.factory.preparer.prepare_linked_requirements_more(reqs) return req_set def get_installation_order(self, req_set): From 18c803a41363612569a57b26376693f8b7d72eea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nguy=E1=BB=85n=20Gia=20Phong?= Date: Mon, 10 Aug 2020 22:54:09 +0700 Subject: [PATCH 4/6] Check hashes of memoized downloads --- src/pip/_internal/operations/prepare.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/pip/_internal/operations/prepare.py b/src/pip/_internal/operations/prepare.py index 5f2a0c74f..82362a2cf 100644 --- a/src/pip/_internal/operations/prepare.py +++ b/src/pip/_internal/operations/prepare.py @@ -507,11 +507,12 @@ class RequirementPreparer(object): with indent_log(): self._ensure_link_req_src_dir(req, download_dir, parallel_builds) + hashes = self._get_linked_req_hashes(req) if link.url not in self._downloaded: try: local_file = unpack_url( - link, req.source_dir, self.downloader, download_dir, - hashes=self._get_linked_req_hashes(req) + link, req.source_dir, self.downloader, + download_dir, hashes, ) except NetworkConnectionError as exc: raise InstallationError( @@ -519,7 +520,10 @@ class RequirementPreparer(object): 'error {} for URL {}'.format(req, exc, link) ) else: - local_file = File(*self._downloaded[link.url]) + file_path, content_type = self._downloaded[link.url] + if hashes: + hashes.check_against_path(file_path) + local_file = File(file_path, content_type) # For use in later processing, preserve the file path on the # requirement. From a1aeb4ce01cf8753bcca89ea3f83eb30abf51ae3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nguy=E1=BB=85n=20Gia=20Phong?= Date: Tue, 11 Aug 2020 17:43:13 +0700 Subject: [PATCH 5/6] Check download folder for files to be downloaded in batch --- src/pip/_internal/operations/prepare.py | 39 ++++++++++++++++--------- 1 file changed, 26 insertions(+), 13 deletions(-) diff --git a/src/pip/_internal/operations/prepare.py b/src/pip/_internal/operations/prepare.py index 82362a2cf..13a7df65a 100644 --- a/src/pip/_internal/operations/prepare.py +++ b/src/pip/_internal/operations/prepare.py @@ -104,10 +104,14 @@ def unpack_vcs_link(link, location): class File(object): + def __init__(self, path, content_type): - # type: (str, str) -> None + # type: (str, Optional[str]) -> None self.path = path - self.content_type = content_type + if content_type is None: + self.content_type = mimetypes.guess_type(path)[0] + else: + self.content_type = content_type def get_http_url( @@ -127,7 +131,7 @@ def get_http_url( if already_downloaded_path: from_path = already_downloaded_path - content_type = mimetypes.guess_type(from_path)[0] + content_type = None else: # let's download to a tmp dir from_path, content_type = downloader.download_one(link, temp_dir.path) @@ -217,10 +221,7 @@ def get_file_url( # one; no internet-sourced hash will be in `hashes`. if hashes: hashes.check_against_path(from_path) - - content_type = mimetypes.guess_type(from_path)[0] - - return File(from_path, content_type) + return File(from_path, None) def unpack_url( @@ -378,6 +379,13 @@ class RequirementPreparer(object): else: logger.info('Collecting %s', req.req or req) + def _get_download_dir(self, link): + # type: (Link) -> Optional[str] + if link.is_wheel and self.wheel_download_dir: + # Download wheels to a dedicated dir when doing `pip wheel`. + return self.wheel_download_dir + return self.download_dir + def _ensure_link_req_src_dir(self, req, download_dir, parallel_builds): # type: (InstallRequirement, Optional[str], bool) -> None """Ensure source_dir of a linked InstallRequirement.""" @@ -487,10 +495,19 @@ class RequirementPreparer(object): # type: (Iterable[InstallRequirement], bool) -> None """Prepare a linked requirement more, if needed.""" reqs = [req for req in reqs if req.needs_more_preparation] + links = [] # type: List[Link] + for req in reqs: + download_dir = self._get_download_dir(req.link) + if download_dir is not None: + hashes = self._get_linked_req_hashes(req) + file_path = _check_download_dir(req.link, download_dir, hashes) + if download_dir is None or file_path is None: + links.append(req.link) + else: + self._downloaded[req.link.url] = file_path, None # Let's download to a temporary directory. tmpdir = TempDirectory(kind="unpack", globally_managed=True).path - links = (req.link for req in reqs) self._downloaded.update(self.downloader.download_many(links, tmpdir)) for req in reqs: self._prepare_linked_requirement(req, parallel_builds) @@ -499,11 +516,7 @@ class RequirementPreparer(object): # type: (InstallRequirement, bool) -> Distribution assert req.link link = req.link - if link.is_wheel and self.wheel_download_dir: - # Download wheels to a dedicated dir when doing `pip wheel`. - download_dir = self.wheel_download_dir - else: - download_dir = self.download_dir + download_dir = self._get_download_dir(link) with indent_log(): self._ensure_link_req_src_dir(req, download_dir, parallel_builds) From b46576d9336ba2c081b486adb49ec773847c16ee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nguy=E1=BB=85n=20Gia=20Phong?= Date: Tue, 11 Aug 2020 22:56:37 +0700 Subject: [PATCH 6/6] Give batch downloader a separate class --- src/pip/_internal/cli/req_command.py | 5 +--- src/pip/_internal/network/download.py | 36 ++++++++++++++++++++++--- src/pip/_internal/operations/prepare.py | 19 ++++++------- tests/unit/test_operations_prepare.py | 16 +++++------ tests/unit/test_req.py | 3 +-- 5 files changed, 53 insertions(+), 26 deletions(-) diff --git a/src/pip/_internal/cli/req_command.py b/src/pip/_internal/cli/req_command.py index 6562fe918..76abce5ac 100644 --- a/src/pip/_internal/cli/req_command.py +++ b/src/pip/_internal/cli/req_command.py @@ -16,7 +16,6 @@ from pip._internal.exceptions import CommandError, PreviousBuildDirError from pip._internal.index.collector import LinkCollector from pip._internal.index.package_finder import PackageFinder from pip._internal.models.selection_prefs import SelectionPreferences -from pip._internal.network.download import Downloader from pip._internal.network.session import PipSession from pip._internal.operations.prepare import RequirementPreparer from pip._internal.req.constructors import ( @@ -213,8 +212,6 @@ class RequirementCommand(IndexGroupCommand): """ Create a RequirementPreparer instance for the given parameters. """ - downloader = Downloader(session, progress_bar=options.progress_bar) - temp_build_dir_path = temp_build_dir.path assert temp_build_dir_path is not None @@ -239,7 +236,7 @@ class RequirementCommand(IndexGroupCommand): build_isolation=options.build_isolation, req_tracker=req_tracker, session=session, - downloader=downloader, + progress_bar=options.progress_bar, finder=finder, require_hashes=options.require_hashes, use_user_site=use_user_site, diff --git a/src/pip/_internal/network/download.py b/src/pip/_internal/network/download.py index 5fd277330..56feaabac 100644 --- a/src/pip/_internal/network/download.py +++ b/src/pip/_internal/network/download.py @@ -151,7 +151,7 @@ class Downloader(object): self._session = session self._progress_bar = progress_bar - def download_one(self, link, location): + def __call__(self, link, location): # type: (Link, str) -> Tuple[str, str] """Download the file given by link into location.""" try: @@ -173,8 +173,38 @@ class Downloader(object): content_type = resp.headers.get('Content-Type', '') return filepath, content_type - def download_many(self, links, location): + +class BatchDownloader(object): + + def __init__( + self, + session, # type: PipSession + progress_bar, # type: str + ): + # type: (...) -> None + self._session = session + self._progress_bar = progress_bar + + def __call__(self, links, location): # type: (Iterable[Link], str) -> Iterable[Tuple[str, Tuple[str, str]]] """Download the files given by links into location.""" for link in links: - yield link.url, self.download_one(link, location) + try: + resp = _http_get_download(self._session, link) + except NetworkConnectionError as e: + assert e.response is not None + logger.critical( + "HTTP error %s while getting %s", + e.response.status_code, link, + ) + raise + + filename = _get_http_response_filename(resp, link) + filepath = os.path.join(location, filename) + + chunks = _prepare_download(resp, link, self._progress_bar) + with open(filepath, 'wb') as content_file: + for chunk in chunks: + content_file.write(chunk) + content_type = resp.headers.get('Content-Type', '') + yield link.url, (filepath, content_type) diff --git a/src/pip/_internal/operations/prepare.py b/src/pip/_internal/operations/prepare.py index 13a7df65a..5eb71ce07 100644 --- a/src/pip/_internal/operations/prepare.py +++ b/src/pip/_internal/operations/prepare.py @@ -26,6 +26,7 @@ from pip._internal.exceptions import ( VcsHashUnsupported, ) from pip._internal.models.wheel import Wheel +from pip._internal.network.download import BatchDownloader, Downloader from pip._internal.network.lazy_wheel import ( HTTPRangeRequestUnsupported, dist_from_wheel_url, @@ -52,7 +53,6 @@ if MYPY_CHECK_RUNNING: from pip._internal.index.package_finder import PackageFinder from pip._internal.models.link import Link - from pip._internal.network.download import Downloader from pip._internal.network.session import PipSession from pip._internal.req.req_install import InstallRequirement from pip._internal.req.req_tracker import RequirementTracker @@ -116,7 +116,7 @@ class File(object): def get_http_url( link, # type: Link - downloader, # type: Downloader + download, # type: Downloader download_dir=None, # type: Optional[str] hashes=None, # type: Optional[Hashes] ): @@ -134,7 +134,7 @@ def get_http_url( content_type = None else: # let's download to a tmp dir - from_path, content_type = downloader.download_one(link, temp_dir.path) + from_path, content_type = download(link, temp_dir.path) if hashes: hashes.check_against_path(from_path) @@ -227,7 +227,7 @@ def get_file_url( def unpack_url( link, # type: Link location, # type: str - downloader, # type: Downloader + download, # type: Downloader download_dir=None, # type: Optional[str] hashes=None, # type: Optional[Hashes] ): @@ -259,7 +259,7 @@ def unpack_url( else: file = get_http_url( link, - downloader, + download, download_dir, hashes=hashes, ) @@ -311,7 +311,7 @@ class RequirementPreparer(object): build_isolation, # type: bool req_tracker, # type: RequirementTracker session, # type: PipSession - downloader, # type: Downloader + progress_bar, # type: str finder, # type: PackageFinder require_hashes, # type: bool use_user_site, # type: bool @@ -324,7 +324,8 @@ class RequirementPreparer(object): self.build_dir = build_dir self.req_tracker = req_tracker self._session = session - self.downloader = downloader + self._download = Downloader(session, progress_bar) + self._batch_download = BatchDownloader(session, progress_bar) self.finder = finder # Where still-packed archives should be written to. If None, they are @@ -508,7 +509,7 @@ class RequirementPreparer(object): # Let's download to a temporary directory. tmpdir = TempDirectory(kind="unpack", globally_managed=True).path - self._downloaded.update(self.downloader.download_many(links, tmpdir)) + self._downloaded.update(self._batch_download(links, tmpdir)) for req in reqs: self._prepare_linked_requirement(req, parallel_builds) @@ -524,7 +525,7 @@ class RequirementPreparer(object): if link.url not in self._downloaded: try: local_file = unpack_url( - link, req.source_dir, self.downloader, + link, req.source_dir, self._download, download_dir, hashes, ) except NetworkConnectionError as exc: diff --git a/tests/unit/test_operations_prepare.py b/tests/unit/test_operations_prepare.py index e90eab8d7..ab6aaf6aa 100644 --- a/tests/unit/test_operations_prepare.py +++ b/tests/unit/test_operations_prepare.py @@ -35,7 +35,7 @@ def test_unpack_url_with_urllib_response_without_content_type(data): session = Mock() session.get = _fake_session_get - downloader = Downloader(session, progress_bar="on") + download = Downloader(session, progress_bar="on") uri = path_to_url(data.packages.joinpath("simple-1.0.tar.gz")) link = Link(uri) @@ -44,7 +44,7 @@ def test_unpack_url_with_urllib_response_without_content_type(data): unpack_url( link, temp_dir, - downloader=downloader, + download=download, download_dir=None, ) assert set(os.listdir(temp_dir)) == { @@ -75,11 +75,11 @@ def test_download_http_url__no_directory_traversal(mock_raise_for_status, 'content-disposition': 'attachment;filename="../out_dir_file"' } session.get.return_value = resp - downloader = Downloader(session, progress_bar="on") + download = Downloader(session, progress_bar="on") download_dir = tmpdir.joinpath('download') os.mkdir(download_dir) - file_path, content_type = downloader.download_one(link, download_dir) + file_path, content_type = download(link, download_dir) # The file should be downloaded to download_dir. actual = os.listdir(download_dir) assert actual == ['out_dir_file'] @@ -178,11 +178,11 @@ class Test_unpack_url(object): self.dist_path2 = data.packages.joinpath(self.dist_file2) self.dist_url = Link(path_to_url(self.dist_path)) self.dist_url2 = Link(path_to_url(self.dist_path2)) - self.no_downloader = Mock(side_effect=AssertionError) + self.no_download = Mock(side_effect=AssertionError) def test_unpack_url_no_download(self, tmpdir, data): self.prep(tmpdir, data) - unpack_url(self.dist_url, self.build_dir, self.no_downloader) + unpack_url(self.dist_url, self.build_dir, self.no_download) assert os.path.isdir(os.path.join(self.build_dir, 'simple')) assert not os.path.isfile( os.path.join(self.download_dir, self.dist_file)) @@ -198,7 +198,7 @@ class Test_unpack_url(object): with pytest.raises(HashMismatch): unpack_url(dist_url, self.build_dir, - downloader=self.no_downloader, + download=self.no_download, hashes=Hashes({'md5': ['bogus']})) def test_unpack_url_thats_a_dir(self, tmpdir, data): @@ -206,7 +206,7 @@ class Test_unpack_url(object): dist_path = data.packages.joinpath("FSPkg") dist_url = Link(path_to_url(dist_path)) unpack_url(dist_url, self.build_dir, - downloader=self.no_downloader, + download=self.no_download, download_dir=self.download_dir) assert os.path.isdir(os.path.join(self.build_dir, 'fspkg')) diff --git a/tests/unit/test_req.py b/tests/unit/test_req.py index ff1b51ae4..a5a9d4bae 100644 --- a/tests/unit/test_req.py +++ b/tests/unit/test_req.py @@ -18,7 +18,6 @@ from pip._internal.exceptions import ( InvalidWheelFilename, PreviousBuildDirError, ) -from pip._internal.network.download import Downloader from pip._internal.network.session import PipSession from pip._internal.operations.prepare import RequirementPreparer from pip._internal.req import InstallRequirement, RequirementSet @@ -87,7 +86,7 @@ class TestRequirementSet(object): build_isolation=True, req_tracker=tracker, session=session, - downloader=Downloader(session, progress_bar="on"), + progress_bar='on', finder=finder, require_hashes=require_hashes, use_user_site=False,