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

Convert TOC to use cache map #3555

Merged
merged 21 commits into from
Oct 16, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
7 changes: 2 additions & 5 deletions lib/pbench/server/api/resources/datasets_contents.py
Original file line number Diff line number Diff line change
Expand Up @@ -145,13 +145,10 @@ def _get(
access = "inventory"
link = str(details.location)
if details.type is CacheType.SYMLINK:
link = str(details.resolve_path)
if details.resolve_type is CacheType.DIRECTORY:
access = "contents"
elif details.resolve_type is not CacheType.FILE:
link = str(details.location)
else:
link = str(details.location)
if details.resolve_type in (CacheType.FILE, CacheType.DIRECTORY):
link = str(details.resolve_path)
val = {
"name": details.name,
"type": details.type.name,
Expand Down
26 changes: 12 additions & 14 deletions lib/pbench/server/cache_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -98,9 +98,9 @@ def __init__(self, tarball: Path, error: str):
class CacheType(Enum):
"""The type of a file or symlink destination"""

BROKEN = auto() # An invalid symlink (absolute or outside tarball)
DIRECTORY = auto() # A directory
FILE = auto() # A regular file
BROKEN = auto() # An invalid symlink (absolute or outside tarball)
OTHER = auto() # An unsupported file type (mount point, etc.)
SYMLINK = auto() # A symbolic link

Expand Down Expand Up @@ -143,23 +143,21 @@ def create(cls, root: Path, path: Path) -> "CacheObject":
if path.is_symlink():
ftype = CacheType.SYMLINK
link_path = path.readlink()
if link_path.is_absolute():
try:
if link_path.is_absolute():
raise ValueError("symlink target can't be absolute")
r_path = path.resolve(strict=True)
resolve_path = r_path.relative_to(root)
except (FileNotFoundError, ValueError):
resolve_path = link_path
resolve_type = CacheType.BROKEN
else:
try:
r_path = path.resolve(strict=True)
resolve_path = r_path.relative_to(root)
except (FileNotFoundError, ValueError):
resolve_path = link_path
resolve_type = CacheType.BROKEN
if r_path.is_dir():
resolve_type = CacheType.DIRECTORY
elif r_path.is_file():
resolve_type = CacheType.FILE
else:
if r_path.is_dir():
resolve_type = CacheType.DIRECTORY
elif r_path.is_file():
resolve_type = CacheType.FILE
else:
resolve_type = CacheType.OTHER
resolve_type = CacheType.OTHER
elif path.is_file():
ftype = CacheType.FILE
size = path.stat().st_size
Expand Down
16 changes: 6 additions & 10 deletions lib/pbench/test/unit/server/test_cache_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -503,10 +503,6 @@ def generate_test_result_tree(tmp_path: Path, dir_name: str) -> Path:
(sub_dir / "metadata.log").write_text(
"[pbench]\nkey = value\n", encoding="utf-8"
)
(sub_dir / "f1.json").write_text("{'json': 'value'}", encoding="utf-8")
(sub_dir / "metadata.log").write_text(
"[pbench]\nkey = value\n", encoding="utf-8"
)
for i in range(1, 4):
(sub_dir / "subdir1" / f"subdir1{i}").mkdir(parents=True, exist_ok=True)
(sub_dir / "subdir1" / "f11.txt").write_text(
Expand All @@ -518,11 +514,11 @@ def generate_test_result_tree(tmp_path: Path, dir_name: str) -> Path:
(sub_dir / "subdir1" / "subdir14" / "subdir141" / "f1411.txt").touch()
os.symlink(Path("../../.."), sub_dir / "subdir1" / "f12_sym")
os.symlink(
Path("../..") / "subdir1" / "subdir15",
Path("../../subdir1/subdir15"),
sub_dir / "subdir1" / "subdir12" / "f121_sym",
)
os.symlink(
Path(".") / "bad_subdir" / "nonexistent_file.txt",
Path("./bad_subdir/nonexistent_file.txt"),
sub_dir / "subdir1" / "subdir12" / "f122_sym",
)
os.symlink(
Expand All @@ -533,19 +529,19 @@ def generate_test_result_tree(tmp_path: Path, dir_name: str) -> Path:
sub_dir / "subdir1" / "subdir14" / "subdir141" / "f1412_sym",
)
os.symlink(
Path("..") / "subdir141",
Path("../subdir141"),
sub_dir / "subdir1" / "subdir14" / "subdir141" / "f1413_sym",
)
os.symlink(
Path(".") / "f1411.txt",
webbnh marked this conversation as resolved.
Show resolved Hide resolved
sub_dir / "subdir1" / "subdir14" / "subdir141" / "f1414_sym",
)
os.symlink(
Path(".") / "f1412_sym",
Path("./f1412_sym"),
webbnh marked this conversation as resolved.
Show resolved Hide resolved
sub_dir / "subdir1" / "subdir14" / "subdir141" / "f1415_sym",
)
os.symlink(
Path("../..") / "subdir12" / "f122_sym",
Path("../../subdir12/f122_sym"),
sub_dir / "subdir1" / "subdir14" / "subdir141" / "f1416_sym",
)

Expand Down Expand Up @@ -652,7 +648,7 @@ def mock_run(args, **_kwargs):
args, returncode=0, stdout="Successfully Unpacked!", stderr=None
)

def mock_resolve(_path, strict: bool = False):
def mock_resolve(_path, **_kwargs):
"""In this scenario, there are no symlinks,
so resolve() should never be called."""
raise AssertionError("Unexpected call to Path.resolve()")
Expand Down
15 changes: 12 additions & 3 deletions lib/pbench/test/unit/server/test_datasets_contents.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,31 +43,36 @@ def query_api(
return query_api

def test_get_no_dataset(self, query_get_as):
"""This fails in Dataset SQL lookup"""
response = query_get_as(
"nonexistent-dataset", "metadata.log", HTTPStatus.NOT_FOUND
)
assert response.json == {"message": "Dataset 'nonexistent-dataset' not found"}

def test_dataset_not_present(self, query_get_as):
"""This fails in the cache manager find_dataset as there are none"""
response = query_get_as("fio_2", "metadata.log", HTTPStatus.NOT_FOUND)
assert response.json == {
"message": "The dataset tarball named 'random_md5_string4' is not found"
}

def test_unauthorized_access(self, query_get_as):
"""This fails because our default user can't read the 'test' dataset"""
response = query_get_as("test", "metadata.log", HTTPStatus.FORBIDDEN)
assert response.json == {
"message": "User drb is not authorized to READ a resource owned by test with private access"
}

@pytest.mark.parametrize("key", ("", ".", "subdir1"))
def test_path_is_directory(self, query_get_as, monkeypatch, key):
"""Mock a directory cache node to check the returned data"""

def mock_find_entry(_s, _d: str, path: Optional[Path]):
file = path if path else Path(".")
return {
"children": {},
"details": CacheObject(
file.name if key else "",
"" if key == "." else key,
webbnh marked this conversation as resolved.
Show resolved Hide resolved
file,
None,
None,
Expand All @@ -88,6 +93,8 @@ def mock_find_entry(_s, _d: str, path: Optional[Path]):
}

def test_not_a_file(self, query_get_as, monkeypatch):
"""When 'find_entry' fails with an exception, we report the text"""

def mock_find_entry(_s, _d: str, path: Optional[Path]):
raise BadDirpath("Nobody home")

Expand All @@ -96,6 +103,7 @@ def mock_find_entry(_s, _d: str, path: Optional[Path]):
assert response.json == {"message": "Nobody home"}

def test_file_info(self, query_get_as, monkeypatch):
"""Mock a file cache node to check the returned data"""
name = "f1.json"

def mock_find_entry(_s, _d: str, path: Optional[Path]):
Expand All @@ -114,6 +122,7 @@ def mock_find_entry(_s, _d: str, path: Optional[Path]):
}

def test_link_info(self, query_get_as, monkeypatch):
"""Mock a symlink cache node to check the returned data"""
name = "test/slink"

def mock_find_entry(_s, _d: str, path: Optional[Path]):
Expand All @@ -130,7 +139,6 @@ def mock_find_entry(_s, _d: str, path: Optional[Path]):

monkeypatch.setattr(CacheManager, "find_entry", mock_find_entry)
response = query_get_as("fio_2", name, HTTPStatus.OK)
assert response.status_code == HTTPStatus.OK
assert response.json == {
"name": "slink",
"type": "SYMLINK",
Expand All @@ -140,6 +148,8 @@ def mock_find_entry(_s, _d: str, path: Optional[Path]):
}

def test_dir_info(self, query_get_as, monkeypatch):
"""Confirm the returned data for a directory with various children"""

def mock_find_entry(_s, _d: str, path: Optional[Path]):
base = path if path else Path("")
return {
Expand Down Expand Up @@ -222,7 +232,6 @@ def mock_find_entry(_s, _d: str, path: Optional[Path]):

monkeypatch.setattr(CacheManager, "find_entry", mock_find_entry)
response = query_get_as("fio_2", "sample1", HTTPStatus.OK)
assert response.status_code == HTTPStatus.OK
assert response.json == {
"directories": [
{
Expand Down