Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix 1.3 performance regression when locking with PyPI repository #7232

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 6 additions & 3 deletions src/poetry/repositories/http_repository.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,11 @@


if TYPE_CHECKING:
from packaging.utils import NormalizedName

from poetry.config.config import Config
from poetry.inspection.info import PackageInfo
from poetry.repositories.link_sources.base import LinkSource
from poetry.utils.authenticator import RepositoryCertificateConfig


Expand Down Expand Up @@ -293,8 +296,8 @@ def _get_response(self, endpoint: str) -> requests.Response | None:
)
return response

def _get_page(self, endpoint: str) -> HTMLPage | None:
response = self._get_response(endpoint)
def _get_page(self, name: NormalizedName) -> LinkSource:
response = self._get_response(f"/{name}/")
if not response:
return None
raise PackageNotFound(f"Package [{name}] not found.")
return HTMLPage(response.url, response.text)
31 changes: 11 additions & 20 deletions src/poetry/repositories/legacy_repository.py
Original file line number Diff line number Diff line change
Expand Up @@ -72,8 +72,9 @@ def package(
return package

def find_links_for_package(self, package: Package) -> list[Link]:
page = self.get_page(f"/{package.name}/")
if page is None:
try:
page = self.get_page(package.name)
except PackageNotFound:
return []

return list(page.links_for_version(package.name, package.version))
Expand All @@ -84,18 +85,10 @@ def _find_packages(
"""
Find packages on the remote server.
"""
versions: list[tuple[Version, str | bool]]

key: str = name
if not constraint.is_any():
key = f"{key}:{constraint!s}"

page = self.get_page(f"/{name}/")
if page is None:
self._log(
f"No packages found for {name}",
level="debug",
)
try:
page = self.get_page(name)
except PackageNotFound:
self._log(f"No packages found for {name}", level="debug")
return []

versions = [
Expand All @@ -119,9 +112,7 @@ def _find_packages(
def _get_release_info(
self, name: NormalizedName, version: Version
) -> dict[str, Any]:
page = self.get_page(f"/{name}/")
if page is None:
raise PackageNotFound(f'No package named "{name}"')
page = self.get_page(name)

links = list(page.links_for_version(name, version))
yanked = page.yanked(name, version)
Expand All @@ -141,8 +132,8 @@ def _get_release_info(
),
)

def _get_page(self, endpoint: str) -> SimpleRepositoryPage | None:
response = self._get_response(endpoint)
def _get_page(self, name: NormalizedName) -> SimpleRepositoryPage:
response = self._get_response(f"/{name}/")
if not response:
return None
raise PackageNotFound(f"Package [{name}] not found.")
return SimpleRepositoryPage(response.url, response.text)
22 changes: 4 additions & 18 deletions src/poetry/repositories/pypi_repository.py
Original file line number Diff line number Diff line change
Expand Up @@ -111,32 +111,18 @@ def _find_packages(
Find packages on the remote server.
"""
try:
json_page = self.get_json_page(name)
json_page = self.get_page(name)
except PackageNotFound:
self._log(
f"No packages found for {name}",
level="debug",
)
self._log(f"No packages found for {name}", level="debug")
return []

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

key: str = name
if not constraint.is_any():
key = f"{key}:{constraint!s}"

versions = [
(version, json_page.yanked(name, version))
for version in json_page.versions(name)
if constraint.allows(version)
]

pretty_name = json_page.content["name"]
packages = [
Package(pretty_name, version, yanked=yanked) for version, yanked in versions
]

return packages
return [Package(name, version, yanked=yanked) for version, yanked in versions]

def _get_package_info(self, name: str) -> dict[str, Any]:
headers = {"Accept": "application/vnd.pypi.simple.v1+json"}
Expand Down Expand Up @@ -226,7 +212,7 @@ def _get_release_info(

return data.asdict()

def get_json_page(self, name: NormalizedName) -> SimpleJsonPage:
def _get_page(self, name: NormalizedName) -> SimpleJsonPage:
source = self._base_url + f"simple/{name}/"
info = self.get_package_info(name)
return SimpleJsonPage(source, info)
Expand Down
11 changes: 9 additions & 2 deletions src/poetry/repositories/single_page_repository.py
Original file line number Diff line number Diff line change
@@ -1,15 +1,22 @@
from __future__ import annotations

from typing import TYPE_CHECKING

from poetry.repositories.exceptions import PackageNotFound
from poetry.repositories.legacy_repository import LegacyRepository
from poetry.repositories.link_sources.html import SimpleRepositoryPage


if TYPE_CHECKING:
from packaging.utils import NormalizedName


class SinglePageRepository(LegacyRepository):
def _get_page(self, endpoint: str | None = None) -> SimpleRepositoryPage | None:
def _get_page(self, name: NormalizedName) -> SimpleRepositoryPage:
"""
Single page repositories only have one page irrespective of endpoint.
"""
response = self._get_response("")
if not response:
return None
raise PackageNotFound(f"Package [{name}] not found.")
return SimpleRepositoryPage(response.url, response.text)
2 changes: 1 addition & 1 deletion tests/installation/test_chooser.py
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ def callback(
parts = uri.rsplit("/")
name = parts[-2]

fixture = LEGACY_FIXTURES / (name + "_partial_yank" + ".html")
fixture = LEGACY_FIXTURES / (name + "-partial-yank" + ".html")

with fixture.open(encoding="utf-8") as f:
return [200, headers, f.read()]
Expand Down
41 changes: 20 additions & 21 deletions tests/repositories/test_legacy_repository.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
import httpretty

from _pytest.monkeypatch import MonkeyPatch
from packaging.utils import NormalizedName

from poetry.config.config import Config

Expand All @@ -45,16 +46,13 @@ class MockRepository(LegacyRepository):
def __init__(self) -> None:
super().__init__("legacy", url="http://legacy.foo.bar", disable_cache=True)

def _get_page(self, endpoint: str) -> SimpleRepositoryPage | None:
parts = endpoint.split("/")
name = parts[1]

def _get_page(self, name: NormalizedName) -> SimpleRepositoryPage:
fixture = self.FIXTURES / (name + ".html")
if not fixture.exists():
return None
raise PackageNotFound(f"Package [{name}] not found.")

with fixture.open(encoding="utf-8") as f:
return SimpleRepositoryPage(self._url + endpoint, f.read())
return SimpleRepositoryPage(self._url + f"/{name}/", f.read())

def _download(self, url: str, dest: Path) -> None:
filename = urlparse.urlparse(url).path.rsplit("/")[-1]
Expand All @@ -73,7 +71,7 @@ def test_packages_property_returns_empty_list() -> None:
def test_page_relative_links_path_are_correct() -> None:
repo = MockRepository()

page = repo.get_page("/relative")
page = repo.get_page("relative")
assert page is not None

for link in page.links:
Expand All @@ -84,7 +82,7 @@ def test_page_relative_links_path_are_correct() -> None:
def test_page_absolute_links_path_are_correct() -> None:
repo = MockRepository()

page = repo.get_page("/absolute")
page = repo.get_page("absolute")
assert page is not None

for link in page.links:
Expand All @@ -95,7 +93,7 @@ def test_page_absolute_links_path_are_correct() -> None:
def test_page_clean_link() -> None:
repo = MockRepository()

page = repo.get_page("/relative")
page = repo.get_page("relative")
assert page is not None

cleaned = page.clean_link('https://legacy.foo.bar/test /the"/cleaning\0')
Expand All @@ -105,7 +103,7 @@ def test_page_clean_link() -> None:
def test_page_invalid_version_link() -> None:
repo = MockRepository()

page = repo.get_page("/invalid-version")
page = repo.get_page("invalid-version")
assert page is not None

links = list(page.links)
Expand All @@ -123,7 +121,7 @@ def test_page_invalid_version_link() -> None:

def test_sdist_format_support() -> None:
repo = MockRepository()
page = repo.get_page("/relative")
page = repo.get_page("relative")
assert page is not None
bz2_links = list(filter(lambda link: link.ext == ".tar.bz2", page.links))
assert len(bz2_links) == 1
Expand Down Expand Up @@ -434,8 +432,8 @@ def test_package_yanked(

def test_package_partial_yank():
class SpecialMockRepository(MockRepository):
def _get_page(self, endpoint: str) -> SimpleRepositoryPage | None:
return super()._get_page(f"/{endpoint.strip('/')}_partial_yank/")
def _get_page(self, name: NormalizedName) -> SimpleRepositoryPage:
return super()._get_page(canonicalize_name(f"{name}-partial-yank"))

repo = MockRepository()
package = repo.package("futures", Version.parse("3.2.0"))
Expand Down Expand Up @@ -481,31 +479,32 @@ def __init__(


def test_get_200_returns_page(http: type[httpretty.httpretty]) -> None:
repo = MockHttpRepository({"/foo": 200}, http)
repo = MockHttpRepository({"/foo/": 200}, http)

assert repo.get_page("/foo")
assert repo.get_page("foo")


@pytest.mark.parametrize("status_code", [401, 403, 404])
def test_get_40x_and_returns_none(
http: type[httpretty.httpretty], status_code: int
) -> None:
repo = MockHttpRepository({"/foo": status_code}, http)
repo = MockHttpRepository({"/foo/": status_code}, http)

assert repo.get_page("/foo") is None
with pytest.raises(PackageNotFound):
repo.get_page("foo")


def test_get_5xx_raises(http: type[httpretty.httpretty]) -> None:
repo = MockHttpRepository({"/foo": 500}, http)
repo = MockHttpRepository({"/foo/": 500}, http)

with pytest.raises(RepositoryError):
repo.get_page("/foo")
repo.get_page("foo")


def test_get_redirected_response_url(
http: type[httpretty.httpretty], monkeypatch: MonkeyPatch
) -> None:
repo = MockHttpRepository({"/foo": 200}, http)
repo = MockHttpRepository({"/foo/": 200}, http)
redirect_url = "http://legacy.redirect.bar"

def get_mock(
Expand All @@ -517,7 +516,7 @@ def get_mock(
return response

monkeypatch.setattr(repo.session, "get", get_mock)
page = repo.get_page("/foo")
page = repo.get_page("foo")
assert page is not None
assert page._url == "http://legacy.redirect.bar/foo/"

Expand Down
8 changes: 0 additions & 8 deletions tests/repositories/test_pypi_repository.py
Original file line number Diff line number Diff line change
Expand Up @@ -332,14 +332,6 @@ def test_urls() -> None:
assert repository.authenticated_url == "https://pypi.org/simple/"


def test_use_pypi_pretty_name() -> None:
repo = MockRepository(fallback=True)

package = repo.find_packages(Factory.create_dependency("twisted", "*"))
assert len(package) == 1
assert package[0].pretty_name == "Twisted"


def test_find_links_for_package_of_supported_types():
repo = MockRepository()
package = repo.find_packages(Factory.create_dependency("hbmqtt", "0.9.6"))
Expand Down
10 changes: 8 additions & 2 deletions tests/repositories/test_single_page_repository.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,19 @@
import re

from pathlib import Path
from typing import TYPE_CHECKING

from poetry.core.packages.dependency import Dependency

from poetry.repositories.exceptions import PackageNotFound
from poetry.repositories.link_sources.html import SimpleRepositoryPage
from poetry.repositories.single_page_repository import SinglePageRepository


if TYPE_CHECKING:
from packaging.utils import NormalizedName


class MockSinglePageRepository(SinglePageRepository):
FIXTURES = Path(__file__).parent / "fixtures" / "single-page"

Expand All @@ -20,10 +26,10 @@ def __init__(self, page: str) -> None:
disable_cache=True,
)

def _get_page(self, endpoint: str = None) -> SimpleRepositoryPage | None:
def _get_page(self, name: NormalizedName) -> SimpleRepositoryPage:
fixture = self.FIXTURES / self.url.rsplit("/", 1)[-1]
if not fixture.exists():
return
raise PackageNotFound(f"Package [{name}] not found.")

with fixture.open(encoding="utf-8") as f:
return SimpleRepositoryPage(self._url, f.read())
Expand Down