Skip to content

Commit

Permalink
performance: cache pages of PyPI repository in the same way as for le…
Browse files Browse the repository at this point in the history
…gacy repository to fix performance regression caused by replacing cachy
  • Loading branch information
radoering authored and neersighted committed Jan 1, 2023
1 parent 82eb934 commit a013c5c
Show file tree
Hide file tree
Showing 8 changed files with 47 additions and 40 deletions.
7 changes: 5 additions & 2 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 | None:
response = self._get_response(f"/{name}/")
if not response:
return None
return HTMLPage(response.url, response.text)
15 changes: 6 additions & 9 deletions src/poetry/repositories/legacy_repository.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 []

Expand All @@ -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 = [
Expand All @@ -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}"')

Expand All @@ -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)
10 changes: 4 additions & 6 deletions src/poetry/repositories/pypi_repository.py
Original file line number Diff line number Diff line change
Expand Up @@ -111,13 +111,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]]

Expand Down Expand Up @@ -226,7 +224,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
8 changes: 7 additions & 1 deletion src/poetry/repositories/single_page_repository.py
Original file line number Diff line number Diff line change
@@ -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.
"""
Expand Down
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
38 changes: 18 additions & 20 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 | 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]
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 | None:
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,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(
Expand All @@ -517,7 +515,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
7 changes: 6 additions & 1 deletion tests/repositories/test_single_page_repository.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,18 @@
import re

from pathlib import Path
from typing import TYPE_CHECKING

from poetry.core.packages.dependency import Dependency

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,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
Expand Down

0 comments on commit a013c5c

Please sign in to comment.