Repository: fsspec/filesystem_spec

Issue:
File cache not cleared on program end
I am using fsspec in a Jupyter notebook to remotely access files. I've been using a `CachingFileSystem` to cache files in a local directory. However, the files I'm accessing are very large. Between runs of my notebook, it doesn't appear that the cache gets cleared. This fills up my machine's local storage _very_ quickly, requiring me to manually delete the cache. Is there a mechanism for this in fsspec, or something I'm missing in my usage of the filesystem? If not, I propose there should be.

Discussion:
martindurant: > I was surprised this was necessary as I assumed that del fs followed by gc.collect() would ensure the filesystem was deleted

FS instances are cached in fsspec.spec._Cached. You can prevent this by using skip_instance_cache=True or instance.clear_instance_cache() . The config value weakref_instance_cache can also be used so that instances are not linked, but this is meant only for debugging. Note that implementation classes also have a class attribute `cachable` which will skip the cache if False.
ianthomas23: > FS instances are cached in fsspec.spec._Cached. You can prevent this by using skip_instance_cache=True or instance.clear_instance_cache() . The config value weakref_instance_cache can also be used so that instances are not linked, but this is meant only for debugging. Note that implementation classes also have a class attribute `cachable` which will skip the cache if False.

Thanks, that explains it. I should have learnt about this by now!

It is probably a truism that even if you know about `n` ways in which things are cached in fsspec, it turns out there are really `n+1` :smile:

I have simplified the test now.


Patch:
diff --git a/fsspec/core.py b/fsspec/core.py
index 890ca81..6f3652c 100644
--- a/fsspec/core.py
+++ b/fsspec/core.py
@@ -210,7 +210,6 @@ def open_files(
     num=1,
     protocol=None,
     newline=None,
-    auto_mkdir=True,
     expand=True,
     **kwargs,
 ):
@@ -249,9 +248,6 @@ def open_files(
     newline: bytes or None
         Used for line terminator in text mode. If None, uses system default;
         if blank, uses no translation.
-    auto_mkdir: bool (True)
-        If in write mode, this will ensure the target directory exists before
-        writing, by calling ``fs.mkdirs(exist_ok=True)``.
     expand: bool
     **kwargs: dict
         Extra options that make sense to a particular storage connection, e.g.
@@ -288,9 +284,6 @@ def open_files(
         protocol=protocol,
         expand=expand,
     )
-    if "r" not in mode and auto_mkdir:
-        parents = {fs._parent(path) for path in paths}
-        [fs.makedirs(parent, exist_ok=True) for parent in parents]
     return OpenFiles(
         [
             OpenFile(
@@ -363,7 +356,6 @@ def url_to_fs(url, **kwargs):
     # non-FS arguments that appear in fsspec.open()
     # inspect could keep this in sync with open()'s signature
     known_kwargs = {
-        "auto_mkdir",
         "compression",
         "encoding",
         "errors",
diff --git a/fsspec/gui.py b/fsspec/gui.py
index 2e47fda..bbc450c 100644
--- a/fsspec/gui.py
+++ b/fsspec/gui.py
@@ -262,7 +262,9 @@ class FileSelector(SigSlot):
             name="protocol",
             align="center",
         )
-        self.kwargs = pn.widgets.TextInput(name="kwargs", value="{}", align="center")
+        self.kwargs = pn.widgets.TextInput(
+            name="kwargs", value=self.init_kwargs, align="center"
+        )
         self.go = pn.widgets.Button(name="⇨", align="end", width=45)
         self.main = SingleSelect(size=10)
         self.home = pn.widgets.Button(name="🏠", width=40, height=30, align="end")
diff --git a/fsspec/implementations/cached.py b/fsspec/implementations/cached.py
index d16e227..b679cce 100644
--- a/fsspec/implementations/cached.py
+++ b/fsspec/implementations/cached.py
@@ -5,6 +5,7 @@ import logging
 import os
 import tempfile
 import time
+import weakref
 from shutil import rmtree
 from typing import TYPE_CHECKING, Any, Callable, ClassVar
 
@@ -111,7 +112,9 @@ class CachingFileSystem(AbstractFileSystem):
                 "Both filesystems (fs) and target_protocol may not be both given."
             )
         if cache_storage == "TMP":
-            storage = [tempfile.mkdtemp()]
+            tempdir = tempfile.mkdtemp()
+            storage = [tempdir]
+            weakref.finalize(self, self._remove_tempdir, tempdir)
         else:
             if isinstance(cache_storage, str):
                 storage = [cache_storage]
@@ -152,6 +155,13 @@ class CachingFileSystem(AbstractFileSystem):
 
         self._strip_protocol: Callable = _strip_protocol
 
+    @staticmethod
+    def _remove_tempdir(tempdir):
+        try:
+            rmtree(tempdir)
+        except Exception:
+            pass
+
     def _mkcache(self):
         os.makedirs(self.storage[-1], exist_ok=True)
 


Test patch:
diff --git a/fsspec/implementations/tests/test_cached.py b/fsspec/implementations/tests/test_cached.py
index 3307495..ee69045 100644
--- a/fsspec/implementations/tests/test_cached.py
+++ b/fsspec/implementations/tests/test_cached.py
@@ -1115,3 +1115,40 @@ def test_getitems_errors(tmpdir):
     assert m.getitems(["afile", "bfile"], on_error="omit") == {"afile": b"test"}
     with pytest.raises(FileNotFoundError):
         m.getitems(["afile", "bfile"])
+
+
+@pytest.mark.parametrize("temp_cache", [False, True])
+def test_cache_dir_auto_deleted(temp_cache, tmpdir):
+    import gc
+
+    source = os.path.join(tmpdir, "source")
+    afile = os.path.join(source, "afile")
+    os.mkdir(source)
+    open(afile, "w").write("test")
+
+    fs = fsspec.filesystem(
+        "filecache",
+        target_protocol="file",
+        cache_storage="TMP" if temp_cache else os.path.join(tmpdir, "cache"),
+        skip_instance_cache=True,  # Important to avoid fs itself being cached
+    )
+
+    cache_dir = fs.storage[-1]
+
+    # Force cache to be created
+    with fs.open(afile, "rb") as f:
+        assert f.read(5) == b"test"
+
+    # Confirm cache exists
+    local = fsspec.filesystem("file")
+    assert local.exists(cache_dir)
+
+    # Delete file system
+    del fs
+    gc.collect()
+
+    # Ensure cache has been deleted, if it is temporary
+    if temp_cache:
+        assert not local.exists(cache_dir)
+    else:
+        assert local.exists(cache_dir)
diff --git a/fsspec/tests/test_core.py b/fsspec/tests/test_core.py
index c786643..e2d4fb9 100644
--- a/fsspec/tests/test_core.py
+++ b/fsspec/tests/test_core.py
@@ -150,10 +150,10 @@ def test_pathobject(tmpdir):
 
 def test_automkdir(tmpdir):
     dir = os.path.join(str(tmpdir), "a")
-    of = fsspec.open(os.path.join(dir, "afile"), "w")
-    with of:
-        pass
-    assert "afile" in os.listdir(dir)
+    of = fsspec.open(os.path.join(dir, "afile"), "w", auto_mkdir=False)
+    with pytest.raises(IOError):
+        with of:
+            pass
 
     dir = os.path.join(str(tmpdir), "b")
     of = fsspec.open(os.path.join(dir, "bfile"), "w", auto_mkdir=True)
@@ -306,3 +306,8 @@ def test_chained_url(ftp_writable):
         url += f"::ftp://{username}:{password}@{host}:{port}/archive.zip"
         with fsspec.open(url, "rb") as f:
             assert f.read() == data["afile"]
+
+
+def test_automkdir_local():
+    fs, _ = fsspec.core.url_to_fs("file://", auto_mkdir=True)
+    assert fs.auto_mkdir is True
diff --git a/fsspec/tests/test_gui.py b/fsspec/tests/test_gui.py
index f944333..0058638 100644
--- a/fsspec/tests/test_gui.py
+++ b/fsspec/tests/test_gui.py
@@ -8,3 +8,12 @@ def test_basic():
 
     gui = fsspec.gui.FileSelector()
     assert "url" in str(gui.panel)
+
+
+def test_kwargs(tmpdir):
+    """confirm kwargs are passed to the filesystem instance"""
+    import fsspec.gui
+
+    gui = fsspec.gui.FileSelector(f"file://{tmpdir}", kwargs="{'auto_mkdir': True}")
+
+    assert gui.fs.auto_mkdir