Skip to content

Commit

Permalink
feat: cache simple repository pages (#6932)
Browse files Browse the repository at this point in the history
At #5868, I
[suggested](#5868 (comment))
that some of the caching that was being reworked could be removed
altogether, because cachecontrol was already taking care of it just
fine.

But now I find myself using an Azure artifacts repository, and it is
returning headers that insist that the client does not do any caching:
```
cache-control: no-cache
pragma: no-cache           
x-cache: CONFIG_NOCACHE 
```
(pypi, by contrast, sets max-age to 10 minutes here).

So I was wrong! And now I am seeing a big performance hit in some
projects where the solve involves overrides and backtracking: and
therefore hitting the legacy simple API repeatedly.

However, we don't need all the mechanism of cachy and its like for this,
a well-placed `@lru_cache()` seems more than sufficient.

This makes me wonder whether it wouldn't be better to do similar for
pypi anyway, and rip out cachecontrol altogether. But let's keep it
simple for now: this is an easy fix to a performance regression.
  • Loading branch information
dimbleby committed Nov 3, 2022
1 parent 4896c4b commit 0dc3d54
Show file tree
Hide file tree
Showing 4 changed files with 16 additions and 14 deletions.
2 changes: 2 additions & 0 deletions src/poetry/repositories/http_repository.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
from __future__ import annotations

import functools
import hashlib
import os
import urllib
Expand Down Expand Up @@ -50,6 +51,7 @@ def __init__(
disable_cache=disable_cache,
)
self._authenticator.add_repository(name, url)
self.get_page = functools.lru_cache(maxsize=None)(self._get_page)

@property
def session(self) -> Authenticator:
Expand Down
6 changes: 3 additions & 3 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(f"/{package.name}/")
if page is None:
return []

Expand All @@ -90,7 +90,7 @@ def _find_packages(
if not constraint.is_any():
key = f"{key}:{constraint!s}"

page = self._get_page(f"/{name}/")
page = self.get_page(f"/{name}/")
if page is None:
self._log(
f"No packages found for {name}",
Expand Down Expand Up @@ -119,7 +119,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(f"/{name}/")
if page is None:
raise PackageNotFound(f'No package named "{name}"')

Expand Down
20 changes: 10 additions & 10 deletions tests/repositories/test_legacy_repository.py
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,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 +84,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 +95,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 +105,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 +123,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 @@ -490,7 +490,7 @@ def __init__(
def test_get_200_returns_page(http: type[httpretty.httpretty]) -> None:
repo = MockHttpRepository({"/foo": 200}, http)

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


@pytest.mark.parametrize("status_code", [401, 403, 404])
Expand All @@ -499,14 +499,14 @@ def test_get_40x_and_returns_none(
) -> None:
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)

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


def test_get_redirected_response_url(
Expand All @@ -524,7 +524,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 Expand Up @@ -560,7 +560,7 @@ def test_authenticator_with_implicit_repository_configuration(
)

repo = LegacyRepository(name="source", url="https://foo.bar/simple", config=config)
repo._get_page("/foo")
repo.get_page("/foo")

request = http.last_request()

Expand Down
2 changes: 1 addition & 1 deletion tests/repositories/test_single_page_repository.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ def _download(self, url: str, dest: Path) -> None:
def test_single_page_repository_get_page():
repo = MockSinglePageRepository("jax_releases")

page = repo._get_page("/ignored")
page = repo.get_page("/ignored")
links = list(page.links)

assert len(links) == 21
Expand Down

0 comments on commit 0dc3d54

Please sign in to comment.