Repository: fsspec/filesystem_spec
Pull request #1006 (2022-07-29 15:02:53)

Be sure to remove temp files when writing with simplecache


Patch:
diff --git a/fsspec/core.py b/fsspec/core.py
index 6fe462e..c60b924 100644
--- a/fsspec/core.py
+++ b/fsspec/core.py
@@ -94,10 +94,6 @@ class OpenFile(object):
     def __repr__(self):
         return "<OpenFile '{}'>".format(self.path)
 
-    def __fspath__(self):
-        # may raise if cannot be resolved to local file
-        return self.open().__fspath__()
-
     def __enter__(self):
         mode = self.mode.replace("t", "").replace("b", "") + "b"
 
@@ -196,18 +192,17 @@ class OpenFiles(list):
 
     def __exit__(self, *args):
         fs = self.fs
+        [s.__exit__(*args) for s in self]
         if "r" not in self.mode:
             while True:
                 if hasattr(fs, "open_many"):
                     # check for concurrent cache upload
                     fs.commit_many(self.files)
-                    self.files.clear()
                     return
                 if hasattr(fs, "fs") and fs.fs is not None:
                     fs = fs.fs
                 else:
                     break
-        [s.__exit__(*args) for s in self]
 
     def __getitem__(self, item):
         out = super().__getitem__(item)
diff --git a/fsspec/implementations/cached.py b/fsspec/implementations/cached.py
index 216fb34..f751f08 100644
--- a/fsspec/implementations/cached.py
+++ b/fsspec/implementations/cached.py
@@ -497,8 +497,7 @@ class WholeFileCacheFileSystem(CachingFileSystem):
             self._mkcache()
         else:
             return [
-                LocalTempFile(self.fs, path, mode=open_files.mode, autocommit=False)
-                for path in paths
+                LocalTempFile(self.fs, path, mode=open_files.mode) for path in paths
             ]
 
         if self.compression:
@@ -541,6 +540,13 @@ class WholeFileCacheFileSystem(CachingFileSystem):
 
     def commit_many(self, open_files):
         self.fs.put([f.fn for f in open_files], [f.path for f in open_files])
+        [f.close() for f in open_files]
+        for f in open_files:
+            # in case autocommit is off, and so close did not already delete
+            try:
+                os.remove(f.name)
+            except FileNotFoundError:
+                pass
 
     def _make_local_details(self, path):
         hash = self.hash_name(path, self.same_names)
@@ -595,7 +601,7 @@ class WholeFileCacheFileSystem(CachingFileSystem):
     def _open(self, path, mode="rb", **kwargs):
         path = self._strip_protocol(path)
         if "r" not in mode:
-            return self.fs._open(path, mode=mode, **kwargs)
+            return LocalTempFile(self, path, mode=mode)
         detail = self._check_file(path)
         if detail:
             detail, fn = detail
@@ -755,6 +761,8 @@ class LocalTempFile:
         self.close()
 
     def close(self):
+        if self.closed:
+            return
         self.fh.close()
         self.closed = True
         if self.autocommit:
@@ -766,13 +774,15 @@ class LocalTempFile:
 
     def commit(self):
         self.fs.put(self.fn, self.path)
+        try:
+            os.remove(self.fn)
+        except (PermissionError, FileNotFoundError):
+            # file path may be held by new version of the file on windows
+            pass
 
     @property
     def name(self):
-        if isinstance(self.fh.name, str):
-            return self.fh.name  # initialized by open()
-        else:
-            return self.fn  # initialized by tempfile.mkstemp()
+        return self.fn
 
     def __getattr__(self, item):
         return getattr(self.fh, item)


Test patch:
diff --git a/fsspec/implementations/tests/test_cached.py b/fsspec/implementations/tests/test_cached.py
index ea5bd4d..c49140b 100644
--- a/fsspec/implementations/tests/test_cached.py
+++ b/fsspec/implementations/tests/test_cached.py
@@ -8,7 +8,7 @@ import pytest
 import fsspec
 from fsspec.compression import compr
 from fsspec.exceptions import BlocksizeMismatchError
-from fsspec.implementations.cached import CachingFileSystem
+from fsspec.implementations.cached import CachingFileSystem, LocalTempFile
 
 from .test_ftp import FTPFileSystem
 
@@ -724,11 +724,15 @@ def test_strip(protocol):
 @pytest.mark.parametrize("protocol", ["simplecache", "filecache"])
 def test_cached_write(protocol):
     d = tempfile.mkdtemp()
-    with fsspec.open_files(f"{protocol}::file://{d}/*.out", mode="wb", num=2) as files:
+    ofs = fsspec.open_files(f"{protocol}::file://{d}/*.out", mode="wb", num=2)
+    with ofs as files:
         for f in files:
+            assert isinstance(f, LocalTempFile)
             f.write(b"data")
+            fn = f.name
 
     assert sorted(os.listdir(d)) == ["0.out", "1.out"]
+    assert not os.path.exists(fn)
 
 
 def test_expiry():
@@ -806,18 +810,10 @@ def test_equality():
     assert hash(cfs2) == hash(cfs3)
 
 
-@pytest.mark.xfail
-def test_json():
-    """Test that the JSON representation refers to correct class.
-
-    Make sure that the JSON representation of a CachingFileSystem refers to the
-    CachingFileSystem, not to the underlying filesystem.
-    """
-    import json
-
+def test_str():
+    """Test that the str representation refers to correct class."""
     from fsspec.implementations.local import LocalFileSystem
 
     lfs = LocalFileSystem()
-    cfs = CachingFileSystem(fs=lfs, cache_storage="raspberry")
-    D = json.loads(cfs.to_json())
-    assert D["cls"].endswith("CachingFileSystem")
+    cfs = CachingFileSystem(fs=lfs)
+    assert "CachingFileSystem" in str(cfs)