Skip to content

Commit

Permalink
performance: cache validation result to avoid unnecessary file system…
Browse files Browse the repository at this point in the history
… access
  • Loading branch information
radoering committed Dec 10, 2022
1 parent c0828fd commit aed056b
Show file tree
Hide file tree
Showing 5 changed files with 57 additions and 40 deletions.
25 changes: 8 additions & 17 deletions src/poetry/core/packages/directory_dependency.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
from __future__ import annotations

import functools
import logging

from typing import TYPE_CHECKING
from typing import Iterable
Expand All @@ -14,8 +13,6 @@
if TYPE_CHECKING:
from pathlib import Path

logger = logging.getLogger(__name__)


class DirectoryDependency(PathDependency):
def __init__(
Expand Down Expand Up @@ -46,28 +43,22 @@ def __init__(
def develop(self) -> bool:
return self._develop

def validate(self, *, raise_error: bool) -> bool:
if not super().validate(raise_error=raise_error):
return False
def _validate(self) -> str:
message = super()._validate()
if message:
return message

message = ""
if self._full_path.is_file():
message = (
return (
f"{self._full_path} for {self.pretty_name} is a file,"
" expected a directory"
)
elif not is_python_project(self._full_path):
message = (
if not is_python_project(self._full_path):
return (
f"Directory {self._full_path} for {self.pretty_name} does not seem"
" to be a Python package"
)

if message:
if raise_error:
raise ValueError(message)
logger.warning(message)
return False
return True
return ""

def _supports_poetry(self) -> bool:
return PyProjectTOML(self._full_path / "pyproject.toml").is_poetry_project()
18 changes: 6 additions & 12 deletions src/poetry/core/packages/file_dependency.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

import hashlib
import io
import logging
import warnings

from typing import TYPE_CHECKING
Expand All @@ -14,8 +13,6 @@
if TYPE_CHECKING:
from pathlib import Path

logger = logging.getLogger(__name__)


class FileDependency(PathDependency):
def __init__(
Expand All @@ -37,20 +34,17 @@ def __init__(
extras=extras,
)

def validate(self, *, raise_error: bool) -> bool:
if not super().validate(raise_error=raise_error):
return False
def _validate(self) -> str:
message = super()._validate()
if message:
return message

if self._full_path.is_dir():
message = (
return (
f"{self._full_path} for {self.pretty_name} is a directory,"
" expected a file"
)
if raise_error:
raise ValueError(message)
logger.warning(message)
return False
return True
return ""

def hash(self, hash_name: str = "sha256") -> str:
warnings.warn(
Expand Down
20 changes: 13 additions & 7 deletions src/poetry/core/packages/path_dependency.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,8 @@ def __init__(
source_url=self._full_path.as_posix(),
extras=extras,
)
# cache validation result to avoid unnecessary file system access
self._validation_error = self._validate()
self.validate(raise_error=False)

@property
Expand All @@ -66,13 +68,12 @@ def is_directory(self) -> bool:
return self._source_type == "directory"

def validate(self, *, raise_error: bool) -> bool:
if not self._full_path.exists():
message = f"Path {self._full_path} for {self.pretty_name} does not exist"
if raise_error:
raise ValueError(message)
logger.warning(message)
return False
return True
if not self._validation_error:
return True
if raise_error:
raise ValueError(self._validation_error)
logger.warning(self._validation_error)
return False

@property
def base_pep_508_name(self) -> str:
Expand All @@ -86,3 +87,8 @@ def base_pep_508_name(self) -> str:
requirement += f" @ {path}"

return requirement

def _validate(self) -> str:
if not self._full_path.exists():
return f"Path {self._full_path} for {self.pretty_name} does not exist"
return ""
18 changes: 16 additions & 2 deletions tests/packages/test_directory_dependency.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,18 @@

if TYPE_CHECKING:
from _pytest.logging import LogCaptureFixture
from pytest_mock import MockerFixture


DIST_PATH = Path(__file__).parent.parent / "fixtures" / "distributions"
SAMPLE_PROJECT = Path(__file__).parent.parent / "fixtures" / "sample_project"


def test_directory_dependency_does_not_exist(caplog: LogCaptureFixture) -> None:
def test_directory_dependency_does_not_exist(
caplog: LogCaptureFixture, mocker: MockerFixture
) -> None:
mock_exists = mocker.patch.object(Path, "exists")
mock_exists.return_value = False
dep = DirectoryDependency("demo", DIST_PATH / "invalid")
assert len(caplog.records) == 1
record = caplog.records[0]
Expand All @@ -27,8 +33,14 @@ def test_directory_dependency_does_not_exist(caplog: LogCaptureFixture) -> None:
with pytest.raises(ValueError, match="does not exist"):
dep.validate(raise_error=True)

mock_exists.assert_called_once()


def test_directory_dependency_is_file(caplog: LogCaptureFixture) -> None:
def test_directory_dependency_is_file(
caplog: LogCaptureFixture, mocker: MockerFixture
) -> None:
mock_is_file = mocker.patch.object(Path, "is_file")
mock_is_file.return_value = True
dep = DirectoryDependency("demo", DIST_PATH / "demo-0.1.0.tar.gz")
assert len(caplog.records) == 1
record = caplog.records[0]
Expand All @@ -38,6 +50,8 @@ def test_directory_dependency_is_file(caplog: LogCaptureFixture) -> None:
with pytest.raises(ValueError, match="is a file"):
dep.validate(raise_error=True)

mock_is_file.assert_called_once()


def test_directory_dependency_is_not_a_python_project(
caplog: LogCaptureFixture,
Expand Down
16 changes: 14 additions & 2 deletions tests/packages/test_file_dependency.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,11 @@
TEST_FILE = "demo-0.1.0.tar.gz"


def test_file_dependency_does_not_exist(caplog: LogCaptureFixture) -> None:
def test_file_dependency_does_not_exist(
caplog: LogCaptureFixture, mocker: MockerFixture
) -> None:
mock_exists = mocker.patch.object(Path, "exists")
mock_exists.return_value = False
dep = FileDependency("demo", DIST_PATH / "demo-0.2.0.tar.gz")
assert len(caplog.records) == 1
record = caplog.records[0]
Expand All @@ -31,8 +35,14 @@ def test_file_dependency_does_not_exist(caplog: LogCaptureFixture) -> None:
with pytest.raises(ValueError, match="does not exist"):
dep.validate(raise_error=True)

mock_exists.assert_called_once()


def test_file_dependency_is_directory(caplog: LogCaptureFixture) -> None:
def test_file_dependency_is_directory(
caplog: LogCaptureFixture, mocker: MockerFixture
) -> None:
mock_is_directory = mocker.patch.object(Path, "is_dir")
mock_is_directory.return_value = True
dep = FileDependency("demo", DIST_PATH)
assert len(caplog.records) == 1
record = caplog.records[0]
Expand All @@ -42,6 +52,8 @@ def test_file_dependency_is_directory(caplog: LogCaptureFixture) -> None:
with pytest.raises(ValueError, match="is a directory"):
dep.validate(raise_error=True)

mock_is_directory.assert_called_once()


def test_default_hash() -> None:
with pytest.warns(DeprecationWarning):
Expand Down

0 comments on commit aed056b

Please sign in to comment.