diff --git a/lib/pbench/cli/server/tree_manage.py b/lib/pbench/cli/server/tree_manage.py index 7eb08f4ed1..e70fb93b97 100644 --- a/lib/pbench/cli/server/tree_manage.py +++ b/lib/pbench/cli/server/tree_manage.py @@ -30,10 +30,6 @@ def reclaim_cache(tree: CacheManager, logger: Logger, lifetime: float = CACHE_LI has_cache = 0 reclaimed = 0 reclaim_failed = 0 - total_count = 0 - has_cache = 0 - reclaimed = 0 - reclaim_failed = 0 for tarball in tree.datasets.values(): total_count += 1 if tarball.unpacked: diff --git a/lib/pbench/server/api/resources/datasets_contents.py b/lib/pbench/server/api/resources/datasets_contents.py index 5ba337c315..5f89f7e7fa 100644 --- a/lib/pbench/server/api/resources/datasets_contents.py +++ b/lib/pbench/server/api/resources/datasets_contents.py @@ -127,9 +127,13 @@ def _get( # Normalize because we want the "root" directory to be reported as # "" rather than as Path's favored "." - loc = "" if str(details.location) == "." else str(details.location) + loc = str(details.location) + name = details.name + if loc == ".": + loc = "" + name = "" val = { - "name": details.name, + "name": name, "type": details.type.name, "directories": dir_list, "files": file_list, diff --git a/lib/pbench/server/cache_manager.py b/lib/pbench/server/cache_manager.py index b55bf674ce..642a3235d2 100644 --- a/lib/pbench/server/cache_manager.py +++ b/lib/pbench/server/cache_manager.py @@ -21,7 +21,6 @@ class CacheManagerError(Exception): """Base class for exceptions raised from this module.""" pass - pass class BadDirpath(CacheManagerError): @@ -344,8 +343,6 @@ def __init__( def close(self): """Close the byte stream and clean up the Popen object""" - exception = None - exception = None if self.subproc: try: @@ -369,7 +366,6 @@ def close(self): # object can be reclaimed. self.subproc = None exception = e - exception = e if self.lock: try: self.lock.release() @@ -384,13 +380,6 @@ def close(self): if exception: raise exception - # NOTE: if both subprocess cleanup and unlock fail with exceptions, we - # raise the latter, and the former will be ignored. In practice, that's - # not a problem as we only construct an Inventory with a subprocess - # reference for extract, which doesn't need to lock a cache directory. - if exception: - raise exception - def getbuffer(self): """Return the underlying byte buffer (used by send_file)""" return self.stream.getbuffer() @@ -494,16 +483,6 @@ def __init__(self, path: Path, resource_id: str, controller: "Controller"): # Record the path of the companion MD5 file self.md5_path: Path = path.with_suffix(".xz.md5") - # Record the lockf file path used to control cache access - self.lock: Path = self.cache / "lock" - - # Record a marker file path used to record the last cache access - # timestamp - self.last_ref: Path = self.cache / "last_ref" - - # Record the path of the companion MD5 file - self.md5_path: Path = path.with_suffix(".xz.md5") - # Record the name of the containing controller self.controller_name: str = controller.name @@ -685,7 +664,7 @@ def find_entry(self, path: Path) -> CacheMapEntry: with LockManager(self.lock) as lock: self.get_results(lock) - if str(path) in (".", ""): + if str(path) == ".": return self.cachemap path_parts = path.parts[:-1] diff --git a/lib/pbench/server/indexing_tarballs.py b/lib/pbench/server/indexing_tarballs.py index 26dec8ec84..9f33d1c104 100644 --- a/lib/pbench/server/indexing_tarballs.py +++ b/lib/pbench/server/indexing_tarballs.py @@ -363,7 +363,6 @@ def sighup_handler(*args): ptb = None tarobj: Optional[Tarball] = None tb_res = error_code["OK"] - lock = None try: # We need the fully unpacked cache tree to index it try: @@ -375,8 +374,6 @@ def sighup_handler(*args): dataset, f"Unable to find dataset: {e!s}", ) - if lock: - lock.release() continue with LockManager(tarobj.lock) as lock: @@ -512,8 +509,6 @@ def sighup_handler(*args): Audit.create( root=audit, status=doneness, attributes=attributes ) - if lock: - lock.release() try: ie_len = ie_filepath.stat().st_size except FileNotFoundError: diff --git a/lib/pbench/test/unit/server/test_cache_manager.py b/lib/pbench/test/unit/server/test_cache_manager.py index 331b219183..25958b34ca 100644 --- a/lib/pbench/test/unit/server/test_cache_manager.py +++ b/lib/pbench/test/unit/server/test_cache_manager.py @@ -560,8 +560,6 @@ def __init__(self, path: Path, resource_id: str, controller: Controller): self.isolator = controller.path / resource_id self.lock = self.cache / "lock" self.last_ref = self.cache / "last_ref" - self.lock = self.cache / "lock" - self.last_ref = self.cache / "last_ref" self.unpacked = None self.cachemap = None self.controller = controller @@ -1888,10 +1886,8 @@ def test_lockmanager(self, monkeypatch): assert not lock.exclusive assert lock.wait assert FakeLockRef.operations == [("acquire", False, True)] - assert FakeLockRef.operations == [("acquire", False, True)] lock.upgrade() assert FakeLockRef.operations == [("acquire", False, True), ("upgrade")] - assert FakeLockRef.operations == [("acquire", False, True), ("upgrade")] lock.downgrade() assert FakeLockRef.operations == [ ("acquire", False, True), diff --git a/lib/pbench/test/unit/server/test_datasets_contents.py b/lib/pbench/test/unit/server/test_datasets_contents.py index 71deba059f..ff967f3fdf 100644 --- a/lib/pbench/test/unit/server/test_datasets_contents.py +++ b/lib/pbench/test/unit/server/test_datasets_contents.py @@ -60,10 +60,10 @@ def test_unauthorized_access(self, query_get_as): "message": "User drb is not authorized to READ a resource owned by test with private access" } - @pytest.mark.parametrize("key", (None, "", "subdir1")) + @pytest.mark.parametrize("key", ("", ".", "subdir1")) def test_path_is_directory(self, query_get_as, monkeypatch, key): def mock_find_entry(_s, _d: str, path: Optional[Path]): - file = path if path else Path("") + file = path if path else Path(".") return { "children": {}, "details": CacheObject( @@ -77,16 +77,14 @@ def mock_find_entry(_s, _d: str, path: Optional[Path]): } monkeypatch.setattr(CacheManager, "find_entry", mock_find_entry) - monkeypatch.setattr(Path, "is_file", lambda self: False) - monkeypatch.setattr(Path, "exists", lambda self: True) - - response = query_get_as("fio_2", key if key else "", HTTPStatus.OK) + response = query_get_as("fio_2", key, HTTPStatus.OK) + name = "" if key == "." else key assert response.json == { "directories": [], "files": [], - "name": key if key else "", + "name": name, "type": "DIRECTORY", - "uri": f"https://localhost/api/v1/datasets/random_md5_string4/contents/{key if key else ''}", + "uri": f"https://localhost/api/v1/datasets/random_md5_string4/contents/{name}", } def test_not_a_file(self, query_get_as, monkeypatch): @@ -94,9 +92,6 @@ def mock_find_entry(_s, _d: str, path: Optional[Path]): raise BadDirpath("Nobody home") monkeypatch.setattr(CacheManager, "find_entry", mock_find_entry) - monkeypatch.setattr(Path, "is_file", lambda self: False) - monkeypatch.setattr(Path, "exists", lambda self: False) - response = query_get_as("fio_2", "subdir1/f1_sym", HTTPStatus.NOT_FOUND) assert response.json == {"message": "Nobody home"}