Separate CacheMetadata class for CachingFileSystem

[ianthomas23]

This PR moves handling of cache metadata out of the CachingFileSystem and into a new class CacheMetadata. The idea is for the metadata handling occur in one place so that we can easily modify and/or extend it in future PRs, e.g. to change the storage format or location.

Essentially cached_files used to be a property of CacheMetadata but is now a property of the new class. Both CachingFileSystem and CacheMetadata need access to the storage locations, so this is shared between them.

There is no functional change here. The only changes to the tests are due to the change in where the private cached_files are stored. There is one failing test that I haven't yet fixed, it is test_metadata_save_blocked and is something to do with the hot-swapping of the open function that I don't fully understand.

[martindurant]

Looks good.

I commented on the use of os.* functions, but maybe thoughts about allowing generic fsspec URLs should be deferred.

[martindurant]

Why only of the writable store?

[ianthomas23]

We only need to know if the writable store is empty following a call to CacheMetadata.clear_cache() as if it is clear then the caller (CachingFileSystem.clear_expired_cache()) deletes all of the writable cache and resets the cache access time, etc.

It is clumsy. I could just make the name better. But probably better still is to return a boolean from CacheMetadata.clear_cache() to tell CachingFileSystem.clear_expired_cache() whether the writable cache is empty. Then we wouldn't have an empty() method hanging around waiting to be used incorrectly.

[martindurant]

I like the plan you describe

[ianthomas23]

Done.

[martindurant]

The one failing test is indeed related to caching, but I can't tell if it's really failing or perhaps just the text of the error changed so the pyrest match got broken.

[ianthomas23]

The failure is a real one in fsspec/implementations/tests/test_cached.py::test_metadata_save_blocked. Somehow I have contrived to break the dynamic swapping of the open function which raises a NameError to simulate an interpreter shutdown. I am still trying to understand the cunning CachingFileSystem.__getattribute__ to work out whether I have fundamentally broken something important or I just need a slightly different test.

[ianthomas23]

I solved the problem with test_metadata_save_blocked, it was quite simple in the end.