From 619b6610e30a8229d20e1ac28a762f33d6807eb7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Randy=20D=C3=B6ring?= <30527984+radoering@users.noreply.github.com> Date: Thu, 22 Dec 2022 11:55:10 +0100 Subject: [PATCH] performance: cache pages of PyPI repository in the same way as for legacy repository to fix performance regression caused by replacing cachy (cherry picked from commit a013c5c02565dc3011ae95230f699628b6abcdeb) --- src/poetry/repositories/http_repository.py | 7 +++- src/poetry/repositories/legacy_repository.py | 15 +++----- src/poetry/repositories/pypi_repository.py | 10 ++--- .../repositories/single_page_repository.py | 8 +++- tests/installation/test_chooser.py | 2 +- ...al_yank.html => futures-partial-yank.html} | 0 tests/repositories/test_legacy_repository.py | 38 +++++++++---------- .../test_single_page_repository.py | 7 +++- 8 files changed, 47 insertions(+), 40 deletions(-) rename tests/repositories/fixtures/legacy/{futures_partial_yank.html => futures-partial-yank.html} (100%) diff --git a/src/poetry/repositories/http_repository.py b/src/poetry/repositories/http_repository.py index bcb0e0fa2b6..8077f9ce57e 100644 --- a/src/poetry/repositories/http_repository.py +++ b/src/poetry/repositories/http_repository.py @@ -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 @@ -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 | None: + response = self._get_response(f"/{name}/") if not response: return None return HTMLPage(response.url, response.text) diff --git a/src/poetry/repositories/legacy_repository.py b/src/poetry/repositories/legacy_repository.py index 73bde91a664..89e5221a706 100644 --- a/src/poetry/repositories/legacy_repository.py +++ b/src/poetry/repositories/legacy_repository.py @@ -72,7 +72,7 @@ def package( return package def find_links_for_package(self, package: Package) -> list[Link]: - page = self.get_page(f"/{package.name}/") + page = self.get_page(package.name) if page is None: return [] @@ -90,12 +90,9 @@ def _find_packages( if not constraint.is_any(): key = f"{key}:{constraint!s}" - page = self.get_page(f"/{name}/") + page = self.get_page(name) if page is None: - self._log( - f"No packages found for {name}", - level="debug", - ) + self._log(f"No packages found for {name}", level="debug") return [] versions = [ @@ -119,7 +116,7 @@ def _find_packages( def _get_release_info( self, name: NormalizedName, version: Version ) -> dict[str, Any]: - page = self.get_page(f"/{name}/") + page = self.get_page(name) if page is None: raise PackageNotFound(f'No package named "{name}"') @@ -141,8 +138,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 | None: + response = self._get_response(f"/{name}/") if not response: return None return SimpleRepositoryPage(response.url, response.text) diff --git a/src/poetry/repositories/pypi_repository.py b/src/poetry/repositories/pypi_repository.py index 236dabadfda..22c8ddeb9d0 100644 --- a/src/poetry/repositories/pypi_repository.py +++ b/src/poetry/repositories/pypi_repository.py @@ -109,13 +109,11 @@ 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 [] + assert isinstance(json_page, SimpleJsonPage) versions: list[tuple[Version, str | bool]] @@ -224,7 +222,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) diff --git a/src/poetry/repositories/single_page_repository.py b/src/poetry/repositories/single_page_repository.py index e8de0b141f8..1cd77a4d3a1 100644 --- a/src/poetry/repositories/single_page_repository.py +++ b/src/poetry/repositories/single_page_repository.py @@ -1,11 +1,17 @@ from __future__ import annotations +from typing import TYPE_CHECKING + 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 | None: """ Single page repositories only have one page irrespective of endpoint. """ diff --git a/tests/installation/test_chooser.py b/tests/installation/test_chooser.py index 0c82faacf61..a50338489a4 100644 --- a/tests/installation/test_chooser.py +++ b/tests/installation/test_chooser.py @@ -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()] diff --git a/tests/repositories/fixtures/legacy/futures_partial_yank.html b/tests/repositories/fixtures/legacy/futures-partial-yank.html similarity index 100% rename from tests/repositories/fixtures/legacy/futures_partial_yank.html rename to tests/repositories/fixtures/legacy/futures-partial-yank.html diff --git a/tests/repositories/test_legacy_repository.py b/tests/repositories/test_legacy_repository.py index bde7b563fc5..d2a30bdfe7f 100644 --- a/tests/repositories/test_legacy_repository.py +++ b/tests/repositories/test_legacy_repository.py @@ -30,6 +30,7 @@ import httpretty from _pytest.monkeypatch import MonkeyPatch + from packaging.utils import NormalizedName from poetry.config.config import Config @@ -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 | None: fixture = self.FIXTURES / (name + ".html") if not fixture.exists(): return None 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] @@ -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: @@ -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: @@ -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') @@ -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) @@ -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 @@ -441,8 +439,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 | None: + return super()._get_page(canonicalize_name(f"{name}-partial-yank")) repo = MockRepository() package = repo.package("futures", Version.parse("3.2.0")) @@ -488,31 +486,31 @@ 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 + assert repo.get_page("foo") is None 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( @@ -524,7 +522,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/" diff --git a/tests/repositories/test_single_page_repository.py b/tests/repositories/test_single_page_repository.py index 54e7069feeb..081a58d6f32 100644 --- a/tests/repositories/test_single_page_repository.py +++ b/tests/repositories/test_single_page_repository.py @@ -3,6 +3,7 @@ import re from pathlib import Path +from typing import TYPE_CHECKING from poetry.core.packages.dependency import Dependency @@ -10,6 +11,10 @@ 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" @@ -20,7 +25,7 @@ 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 | None: fixture = self.FIXTURES / self.url.rsplit("/", 1)[-1] if not fixture.exists(): return