From ede863b1b04d076d6d3c984aa94e2e9b1339552a Mon Sep 17 00:00:00 2001 From: Albert Tugushev Date: Tue, 26 Feb 2019 02:11:17 +0300 Subject: [PATCH 1/4] Show a progress of a hashes generation process in a verbose mode --- piptools/_compat/contextlib.py | 18 ++++++++++++++++ piptools/repositories/pypi.py | 39 +++++++++++++++++++++++++++++----- piptools/resolver.py | 2 ++ tests/test_cli_compile.py | 13 ++++++++++++ 4 files changed, 67 insertions(+), 5 deletions(-) create mode 100644 piptools/_compat/contextlib.py diff --git a/piptools/_compat/contextlib.py b/piptools/_compat/contextlib.py new file mode 100644 index 000000000..04039ccb0 --- /dev/null +++ b/piptools/_compat/contextlib.py @@ -0,0 +1,18 @@ +# Ported from python 3.7 contextlib.py +class nullcontext(object): + """Context manager that does no additional processing. + Used as a stand-in for a normal context manager, when a particular + block of code is only sometimes used with a normal context manager: + cm = optional_cm if condition else nullcontext() + with cm: + # Perform operation, using optional_cm if condition is True + """ + + def __init__(self, enter_result=None): + self.enter_result = enter_result + + def __enter__(self): + return self.enter_result + + def __exit__(self, *excinfo): + pass diff --git a/piptools/repositories/pypi.py b/piptools/repositories/pypi.py index 08449d808..179704f85 100644 --- a/piptools/repositories/pypi.py +++ b/piptools/repositories/pypi.py @@ -1,6 +1,7 @@ # coding: utf-8 from __future__ import absolute_import, division, print_function, unicode_literals +import collections import hashlib import os from contextlib import contextmanager @@ -16,11 +17,14 @@ RequirementSet, TemporaryDirectory, Wheel, + contextlib, is_file_url, url_to_path, ) from ..cache import CACHE_DIR +from ..click import progressbar from ..exceptions import NoCandidateFound +from ..logging import log from ..utils import ( fs_str, is_pinned_requirement, @@ -43,6 +47,9 @@ def RequirementTracker(): except ImportError: from pip.wheel import WheelCache +FILE_CHUNK_SIZE = 4096 +FileStream = collections.namedtuple("File", "stream size") + class PyPIRepository(BaseRepository): DEFAULT_INDEX_URL = PyPI.simple_url @@ -278,15 +285,30 @@ def get_hashes(self, ireq): ) matching_candidates = candidates_by_version[matching_versions[0]] + log.debug(" {}".format(ireq.name)) + return { self._get_file_hash(candidate.location) for candidate in matching_candidates } def _get_file_hash(self, location): + log.debug(" Hashing {}".format(location.url_without_fragment)) h = hashlib.new(FAVORITE_HASH) - with open_local_or_remote_file(location, self.session) as fp: - for chunk in iter(lambda: fp.read(8096), b""): - h.update(chunk) + with open_local_or_remote_file(location, self.session) as f: + # Chunks to iterate + chunks = iter(lambda: f.stream.read(FILE_CHUNK_SIZE), b"") + + # Choose a context manager depending on verbosity + if log.verbosity >= 1: + iter_length = f.size / FILE_CHUNK_SIZE if f.size else None + context_manager = progressbar(chunks, length=iter_length, label=" ") + else: + context_manager = contextlib.nullcontext(chunks) + + # Iterate over the chosen context manager + with context_manager as bar: + for chunk in bar: + h.update(chunk) return ":".join([FAVORITE_HASH, h.hexdigest()]) @contextmanager @@ -340,13 +362,20 @@ def open_local_or_remote_file(link, session): if os.path.isdir(local_path): raise ValueError("Cannot open directory for read: {}".format(url)) else: + st = os.stat(local_path) with open(local_path, "rb") as local_file: - yield local_file + yield FileStream(stream=local_file, size=st.st_size) else: # Remote URL headers = {"Accept-Encoding": "identity"} response = session.get(url, headers=headers, stream=True) + + # Content length must be int or None + content_length = response.headers.get("content-length") + if content_length is not None: + content_length = int(content_length) + try: - yield response.raw + yield FileStream(stream=response.raw, size=content_length) finally: response.close() diff --git a/piptools/resolver.py b/piptools/resolver.py index 57a4b332b..764cb6b2f 100644 --- a/piptools/resolver.py +++ b/piptools/resolver.py @@ -82,6 +82,8 @@ def resolve_hashes(self, ireqs): """ Finds acceptable hashes for all of the given InstallRequirements. """ + log.debug("") + log.debug("Generating hashes:") with self.repository.allow_all_wheels(): return {ireq: self.repository.get_hashes(ireq) for ireq in ireqs} diff --git a/tests/test_cli_compile.py b/tests/test_cli_compile.py index 9a16d80fa..eb80eaa97 100644 --- a/tests/test_cli_compile.py +++ b/tests/test_cli_compile.py @@ -343,6 +343,19 @@ def test_generate_hashes_with_editable(runner): assert expected in out.output +def test_generate_hashes_verbose(runner): + """ + The hashes generation process should show a progress. + """ + with open("requirements.in", "w") as fp: + fp.write("pytz==2017.2") + + out = runner.invoke(cli, ["--generate-hashes", "-v"]) + + expected_verbose_text = "Generating hashes:\n pytz\n" + assert expected_verbose_text in out.output + + @fail_below_pip9 def test_filter_pip_markers(runner): """ From 64171792a3c973bf9898739cada26f0f0820e9d2 Mon Sep 17 00:00:00 2001 From: Albert Tugushev Date: Sun, 7 Apr 2019 03:23:24 +0700 Subject: [PATCH 2/4] Rename File to FileStream [ci skip] --- piptools/repositories/pypi.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/piptools/repositories/pypi.py b/piptools/repositories/pypi.py index 179704f85..ddffc3904 100644 --- a/piptools/repositories/pypi.py +++ b/piptools/repositories/pypi.py @@ -48,7 +48,7 @@ def RequirementTracker(): from pip.wheel import WheelCache FILE_CHUNK_SIZE = 4096 -FileStream = collections.namedtuple("File", "stream size") +FileStream = collections.namedtuple("FileStream", "stream size") class PyPIRepository(BaseRepository): From 81bfd19a2e1e2d9a81f428362fb012696f157a01 Mon Sep 17 00:00:00 2001 From: Albert Tugushev Date: Sun, 14 Apr 2019 09:08:41 +0800 Subject: [PATCH 3/4] Add tests for open_local_or_remote_file function --- piptools/_compat/pip_compat.py | 2 ++ piptools/repositories/pypi.py | 9 ++--- tests/test_repository_pypi.py | 65 +++++++++++++++++++++++++++++++++- 3 files changed, 71 insertions(+), 5 deletions(-) diff --git a/piptools/_compat/pip_compat.py b/piptools/_compat/pip_compat.py index eb8f2a907..0259ab5bd 100644 --- a/piptools/_compat/pip_compat.py +++ b/piptools/_compat/pip_compat.py @@ -43,6 +43,8 @@ def do_import(module_path, subimport=None, old_path=None): PyPI = do_import("models.index", "PyPI") stdlib_pkgs = do_import("utils.compat", "stdlib_pkgs", old_path="compat") DEV_PKGS = do_import("commands.freeze", "DEV_PKGS") +Link = do_import("models.link", "Link", old_path="index") +Session = do_import("_vendor.requests.sessions", "Session") # pip 18.1 has refactored InstallRequirement constructors use by pip-tools. if pkg_resources.parse_version(pip.__version__) < pkg_resources.parse_version("18.1"): diff --git a/piptools/repositories/pypi.py b/piptools/repositories/pypi.py index ddffc3904..463885979 100644 --- a/piptools/repositories/pypi.py +++ b/piptools/repositories/pypi.py @@ -352,7 +352,7 @@ def open_local_or_remote_file(link, session): :type link: pip.index.Link :type session: requests.Session :raises ValueError: If link points to a local directory. - :return: a context manager to the opened file-like object + :return: a context manager to a FileStream with the opened file-like object """ url = link.url_without_fragment @@ -371,9 +371,10 @@ def open_local_or_remote_file(link, session): response = session.get(url, headers=headers, stream=True) # Content length must be int or None - content_length = response.headers.get("content-length") - if content_length is not None: - content_length = int(content_length) + try: + content_length = int(response.headers["content-length"]) + except (ValueError, KeyError, TypeError): + content_length = None try: yield FileStream(stream=response.raw, size=content_length) diff --git a/tests/test_repository_pypi.py b/tests/test_repository_pypi.py index 0a9969fd1..f79af7b4d 100644 --- a/tests/test_repository_pypi.py +++ b/tests/test_repository_pypi.py @@ -1,5 +1,9 @@ +import mock +import pytest + +from piptools._compat.pip_compat import Link, Session, path_to_url from piptools.pip import get_pip_command -from piptools.repositories.pypi import PyPIRepository +from piptools.repositories.pypi import PyPIRepository, open_local_or_remote_file def test_generate_hashes_all_platforms(from_line): @@ -78,3 +82,62 @@ def test_get_hashes_editable_empty_set(from_editable): repository = PyPIRepository(pip_options, session) ireq = from_editable("git+https://github.com/django/django.git#egg=django") assert repository.get_hashes(ireq) == set() + + +@pytest.mark.parametrize("content, content_length", [(b"foo", 3), (b"foobar", 6)]) +def test_open_local_or_remote_file__local_file(tmp_path, content, content_length): + """ + Test the `open_local_or_remote_file` returns a context manager to a FileStream + for a given `Link` to a local file. + """ + local_file_path = tmp_path / "foo.txt" + local_file_path.write_bytes(content) + + link = Link(local_file_path.as_uri()) + session = Session() + + with open_local_or_remote_file(link, session) as file_stream: + assert file_stream.stream.read() == content + assert file_stream.size == content_length + + +def test_open_local_or_remote_file__directory(tmpdir): + """ + Test the `open_local_or_remote_file` raises a ValueError for a given `Link` + to a directory. + """ + link = Link(path_to_url(tmpdir.strpath)) + session = Session() + + with pytest.raises(ValueError, match="Cannot open directory for read"): + with open_local_or_remote_file(link, session): + pass + + +@pytest.mark.parametrize( + "content, content_length, expected_content_length", + [(b"foo", 3, 3), (b"bar", None, None), (b"kek", "invalid-content-length", None)], +) +def test_open_local_or_remote_file__remote_file( + tmp_path, content, content_length, expected_content_length +): + """ + Test the `open_local_or_remote_file` returns a context manager to a FileStream + for a given `Link` to a remote file. + """ + link = Link("https://example.com/foo.txt") + session = Session() + + response_file_path = tmp_path / "foo.txt" + response_file_path.write_bytes(content) + + mock_response = mock.Mock() + mock_response.raw = response_file_path.open("rb") + mock_response.headers = {"content-length": content_length} + + with mock.patch.object(session, "get", return_value=mock_response): + with open_local_or_remote_file(link, session) as file_stream: + assert file_stream.stream.read() == content + assert file_stream.size == expected_content_length + + mock_response.close.assert_called_once() From 0d7cbfd4eb72c8d9f03809a30e0ed9279e2e963a Mon Sep 17 00:00:00 2001 From: Albert Tugushev Date: Sun, 14 Apr 2019 12:48:21 +0700 Subject: [PATCH 4/4] Exclude line from coverage --- tests/test_repository_pypi.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_repository_pypi.py b/tests/test_repository_pypi.py index f79af7b4d..7dc1f92a2 100644 --- a/tests/test_repository_pypi.py +++ b/tests/test_repository_pypi.py @@ -111,7 +111,7 @@ def test_open_local_or_remote_file__directory(tmpdir): with pytest.raises(ValueError, match="Cannot open directory for read"): with open_local_or_remote_file(link, session): - pass + pass # pragma: no cover @pytest.mark.parametrize(