More cleanup again

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:

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,

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]

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:

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),

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,26 +77,21 @@ 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):
         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"}