Skip to content

Commit

Permalink
Avoid the deprecated JSON API (#6081)
Browse files Browse the repository at this point in the history
Resolves #6076 

I've taken the JSON version of the simple API and converted it into a
`LinkSource` so that the package-finding logic in the `PyPiRepository`
is very similar to - but annoyingly not quite the same as! - the
`LegacyRepository`.

I've also taken the opportunity to refactor the `LegacyRepository` ever
so slightly to emphasise that similarity. I think I've probably fixed a
small bug re caching and pre-releases: previously the processing for
ignored pre-releases was skipped when reading from the cache.

I believe this change will tend to be a modest performance hit. Eg
consider a package like `cryptography`, for which there are maybe a
couple of dozen downloads available at each release: to get the
available versions we now have to iterate over each of those files and
parse their names, rather than simply reading the answer.

However if the API that poetry currently uses is truly deprecated I see
little choice but to suck that up - or risk being in an awkward spot
when it is turned off. cf #5970, but worse.

Most of the changes are in the test fixtures:
- unversioned fixtures were generated from the existing fixtures: I
didn't want to download fresh data and start getting different answers
than the tests were expecting
- new versioned fixtures were downloaded fresh
  • Loading branch information
dimbleby authored Sep 17, 2022
1 parent 714c09d commit b61a4dd
Show file tree
Hide file tree
Showing 44 changed files with 4,276 additions and 7,717 deletions.
40 changes: 40 additions & 0 deletions src/poetry/repositories/link_sources/json.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
from __future__ import annotations

from collections import defaultdict
from typing import TYPE_CHECKING
from typing import Any

from poetry.core.packages.utils.link import Link

from poetry.repositories.link_sources.base import LinkSource
from poetry.utils._compat import cached_property


if TYPE_CHECKING:
from poetry.repositories.link_sources.base import LinkCache


class SimpleJsonPage(LinkSource):
"""Links as returned by PEP 691 compatible JSON-based Simple API."""

def __init__(self, url: str, content: dict[str, Any]) -> None:
super().__init__(url=url)
self.content = content

@cached_property
def _link_cache(self) -> LinkCache:
links: LinkCache = defaultdict(lambda: defaultdict(list))
for file in self.content["files"]:
url = file["url"]
requires_python = file.get("requires-python")
yanked = file.get("yanked", False)
link = Link(url, requires_python=requires_python, yanked=yanked)

if link.ext not in self.SUPPORTED_FORMATS:
continue

pkg = self.link_package_data(link)
if pkg:
links[pkg.name][pkg.version].append(link)

return links
68 changes: 36 additions & 32 deletions src/poetry/repositories/pypi_repository.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,11 @@
from html5lib.html5parser import parse
from poetry.core.packages.package import Package
from poetry.core.packages.utils.link import Link
from poetry.core.semver.version import Version
from poetry.core.version.exceptions import InvalidVersion

from poetry.repositories.exceptions import PackageNotFound
from poetry.repositories.http import HTTPRepository
from poetry.repositories.link_sources.json import SimpleJsonPage
from poetry.utils._compat import to_str
from poetry.utils.constants import REQUESTS_TIMEOUT

Expand All @@ -27,6 +27,7 @@

if TYPE_CHECKING:
from packaging.utils import NormalizedName
from poetry.core.semver.version import Version
from poetry.core.semver.version_constraint import VersionConstraint

SUPPORTED_PACKAGE_TYPES = {"sdist", "bdist_wheel"}
Expand Down Expand Up @@ -114,50 +115,44 @@ def _find_packages(
Find packages on the remote server.
"""
try:
info = self.get_package_info(name)
json_page = self.get_json_page(name)
except PackageNotFound:
self._log(
f"No packages found for {name} {constraint!s}",
f"No packages found for {name}",
level="debug",
)
return []

packages = []
versions: list[tuple[Version, str | bool]]

for version_string, release in info["releases"].items():
if not release:
# Bad release
self._log(
f"No release information found for {name}-{version_string},"
" skipping",
level="debug",
)
continue
key: str = name
if not constraint.is_any():
key = f"{key}:{constraint!s}"

try:
version = Version.parse(version_string)
except InvalidVersion:
self._log(
f'Unable to parse version "{version_string}" for the'
f" {name} package, skipping",
level="debug",
)
continue
if self._cache.store("matches").has(key):
versions = self._cache.store("matches").get(key)
else:
versions = [
(version, json_page.yanked(name, version))
for version in json_page.versions(name)
if constraint.allows(version)
]
self._cache.store("matches").put(key, versions, 5)

if constraint.allows(version):
# PEP 592: PyPI always yanks entire releases, not individual files,
# so we just have to look for the first file
yanked = self._get_yanked(release[0])
packages.append(Package(info["info"]["name"], version, yanked=yanked))
pretty_name = json_page.content["name"]
packages = [
Package(pretty_name, version, yanked=yanked) for version, yanked in versions
]

return packages

def _get_package_info(self, name: NormalizedName) -> dict[str, Any]:
data = self._get(f"pypi/{name}/json")
if data is None:
def _get_package_info(self, name: str) -> dict[str, Any]:
headers = {"Accept": "application/vnd.pypi.simple.v1+json"}
info = self._get(f"simple/{name}/", headers=headers)
if info is None:
raise PackageNotFound(f"Package [{name}] not found.")

return data
return info

def find_links_for_package(self, package: Package) -> list[Link]:
json_data = self._get(f"pypi/{package.name}/{package.version}/json")
Expand Down Expand Up @@ -239,12 +234,20 @@ def _get_release_info(

return data.asdict()

def _get(self, endpoint: str) -> dict[str, Any] | None:
def get_json_page(self, name: NormalizedName) -> SimpleJsonPage:
source = self._base_url + f"simple/{name}/"
info = self.get_package_info(name)
return SimpleJsonPage(source, info)

def _get(
self, endpoint: str, headers: dict[str, str] | None = None
) -> dict[str, Any] | None:
try:
json_response = self.session.get(
self._base_url + endpoint,
raise_for_status=False,
timeout=REQUESTS_TIMEOUT,
headers=headers,
)
except requests.exceptions.TooManyRedirects:
# Cache control redirect loop.
Expand All @@ -254,6 +257,7 @@ def _get(self, endpoint: str) -> dict[str, Any] | None:
self._base_url + endpoint,
raise_for_status=False,
timeout=REQUESTS_TIMEOUT,
headers=headers,
)

if json_response.status_code != 200:
Expand Down
3 changes: 2 additions & 1 deletion src/poetry/utils/authenticator.py
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,8 @@ def authenticated_url(self, url: str) -> str:
def request(
self, method: str, url: str, raise_for_status: bool = True, **kwargs: Any
) -> requests.Response:
request = requests.Request(method, url)
headers = kwargs.get("headers")
request = requests.Request(method, url, headers=headers)
credential = self.get_credentials_for_url(url)

if credential.username is not None or credential.password is not None:
Expand Down
24 changes: 12 additions & 12 deletions tests/installation/test_executor.py
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ def test_execute_executes_a_batch_of_operations(

return_code = executor.execute(
[
Install(Package("pytest", "3.5.2")),
Install(Package("pytest", "3.5.1")),
Uninstall(Package("attrs", "17.4.0")),
Update(Package("requests", "2.18.3"), Package("requests", "2.18.4")),
Uninstall(Package("clikit", "0.2.3")).skip("Not currently installed"),
Expand All @@ -160,7 +160,7 @@ def test_execute_executes_a_batch_of_operations(
expected = f"""
Package operations: 4 installs, 1 update, 1 removal
• Installing pytest (3.5.2)
• Installing pytest (3.5.1)
• Removing attrs (17.4.0)
• Updating requests (2.18.3 -> 2.18.4)
• Installing demo (0.1.0 {file_package.source_url})
Expand All @@ -182,20 +182,20 @@ def test_execute_executes_a_batch_of_operations(
"operations, has_warning",
[
(
[Install(Package("black", "21.11b0")), Install(Package("pytest", "3.5.2"))],
[Install(Package("black", "21.11b0")), Install(Package("pytest", "3.5.1"))],
True,
),
(
[
Uninstall(Package("black", "21.11b0")),
Uninstall(Package("pytest", "3.5.2")),
Uninstall(Package("pytest", "3.5.1")),
],
False,
),
(
[
Update(Package("black", "19.10b0"), Package("black", "21.11b0")),
Update(Package("pytest", "3.5.1"), Package("pytest", "3.5.2")),
Update(Package("pytest", "3.5.0"), Package("pytest", "3.5.1")),
],
True,
),
Expand Down Expand Up @@ -299,18 +299,18 @@ def test_execute_works_with_ansi_output(
mocker.patch.object(env, "_run", return_value=install_output)
return_code = executor.execute(
[
Install(Package("pytest", "3.5.2")),
Install(Package("pytest", "3.5.1")),
]
)
env._run.assert_called_once()

# fmt: off
expected = [
"\x1b[39;1mPackage operations\x1b[39;22m: \x1b[34m1\x1b[39m install, \x1b[34m0\x1b[39m updates, \x1b[34m0\x1b[39m removals", # noqa: E501
"\x1b[34;1m•\x1b[39;22m \x1b[39mInstalling \x1b[39m\x1b[36mpytest\x1b[39m\x1b[39m (\x1b[39m\x1b[39;1m3.5.2\x1b[39;22m\x1b[39m)\x1b[39m: \x1b[34mPending...\x1b[39m", # noqa: E501
"\x1b[34;1m•\x1b[39;22m \x1b[39mInstalling \x1b[39m\x1b[36mpytest\x1b[39m\x1b[39m (\x1b[39m\x1b[39;1m3.5.2\x1b[39;22m\x1b[39m)\x1b[39m: \x1b[34mDownloading...\x1b[39m", # noqa: E501
"\x1b[34;1m•\x1b[39;22m \x1b[39mInstalling \x1b[39m\x1b[36mpytest\x1b[39m\x1b[39m (\x1b[39m\x1b[39;1m3.5.2\x1b[39;22m\x1b[39m)\x1b[39m: \x1b[34mInstalling...\x1b[39m", # noqa: E501
"\x1b[32;1m•\x1b[39;22m \x1b[39mInstalling \x1b[39m\x1b[36mpytest\x1b[39m\x1b[39m (\x1b[39m\x1b[32m3.5.2\x1b[39m\x1b[39m)\x1b[39m", # finished # noqa: E501
"\x1b[34;1m•\x1b[39;22m \x1b[39mInstalling \x1b[39m\x1b[36mpytest\x1b[39m\x1b[39m (\x1b[39m\x1b[39;1m3.5.1\x1b[39;22m\x1b[39m)\x1b[39m: \x1b[34mPending...\x1b[39m", # noqa: E501
"\x1b[34;1m•\x1b[39;22m \x1b[39mInstalling \x1b[39m\x1b[36mpytest\x1b[39m\x1b[39m (\x1b[39m\x1b[39;1m3.5.1\x1b[39;22m\x1b[39m)\x1b[39m: \x1b[34mDownloading...\x1b[39m", # noqa: E501
"\x1b[34;1m•\x1b[39;22m \x1b[39mInstalling \x1b[39m\x1b[36mpytest\x1b[39m\x1b[39m (\x1b[39m\x1b[39;1m3.5.1\x1b[39;22m\x1b[39m)\x1b[39m: \x1b[34mInstalling...\x1b[39m", # noqa: E501
"\x1b[32;1m•\x1b[39;22m \x1b[39mInstalling \x1b[39m\x1b[36mpytest\x1b[39m\x1b[39m (\x1b[39m\x1b[32m3.5.1\x1b[39m\x1b[39m)\x1b[39m", # finished # noqa: E501
]
# fmt: on

Expand Down Expand Up @@ -341,15 +341,15 @@ def test_execute_works_with_no_ansi_output(
mocker.patch.object(env, "_run", return_value=install_output)
return_code = executor.execute(
[
Install(Package("pytest", "3.5.2")),
Install(Package("pytest", "3.5.1")),
]
)
env._run.assert_called_once()

expected = """
Package operations: 1 install, 0 updates, 0 removals
• Installing pytest (3.5.2)
• Installing pytest (3.5.1)
"""
expected = set(expected.splitlines())
output = set(io_not_decorated.fetch_output().splitlines())
Expand Down
Loading

0 comments on commit b61a4dd

Please sign in to comment.