Change move_to_cache to also check should_be_cached

This simplifies the usage in file datastore.

python/lsst/daf/butler/configs/datastores/fileDatastore.yaml

@@ -18,7 +18,7 @@ datastore:
     # Instantiating a Butler from another Butler could propagate temporary
     # location. This can be left out or null used
     # to indicate a temporary directory.
-    root: null
+    root: /Volumes/MediaHD/Users/timj/work/lsstsw/build/daf_butler_test/cache
     # Use a dict over list to simplify merging logic.
     # Later could change it to be the timescale for caching:
     #   - Quantum (clear at end of quantum)

python/lsst/daf/butler/core/datastoreCacheManager.py

@@ -99,11 +99,12 @@ def should_be_cached(self, entity: Union[DatasetRef, DatasetType, StorageClass])
         raise NotImplementedError()
 
     @abstractmethod
-    def move_to_cache(self, uri: ButlerURI, ref: DatasetRef) -> ButlerURI:
+    def move_to_cache(self, uri: ButlerURI, ref: DatasetRef) -> Optional[ButlerURI]:
         """Move a file to the cache.
 
         Move the given file into the cache, using the supplied DatasetRef
-        for naming.
+        for naming. A call is made to `should_be_cached()` and if the
+        DatasetRef should not be accepted `None` will be returned.
 
         Parameters
         ----------
@@ -115,8 +116,9 @@ def move_to_cache(self, uri: ButlerURI, ref: DatasetRef) -> ButlerURI:
 
         Returns
         -------
-        new : `ButlerURI`
-            URI to the file within the cache.
+        new : `ButlerURI` or `None`
+            URI to the file within the cache, or `None` if the dataset
+            was not accepted by the cache.
         """
         raise NotImplementedError()
 
@@ -201,11 +203,14 @@ def _construct_cache_name(self, ref: DatasetRef, extension: str) -> ButlerURI:
         """
         return self.cache_directory.join(f"{ref.id}{extension}")
 
-    def move_to_cache(self, uri: ButlerURI, ref: DatasetRef) -> ButlerURI:
+    def move_to_cache(self, uri: ButlerURI, ref: DatasetRef) -> Optional[ButlerURI]:
         # Docstring inherited
         if ref.id is None:
             raise ValueError(f"Can not cache a file associated with an unresolved reference ({ref})")
 
+        if not self.should_be_cached(ref):
+            return None
+
         # Write the file using the id of the dataset ref and the file
         # extension.
         cached_location = self._construct_cache_name(ref, uri.getExtension())
@@ -250,15 +255,9 @@ def should_be_cached(self, entity: Union[DatasetRef, DatasetType, StorageClass])
         """
         return False
 
-    def move_to_cache(self, uri: ButlerURI, ref: DatasetRef) -> ButlerURI:
-        """Move a file to the cache.
-
-        Always raises to indicate that there has been a logic error if
-        this is called without first checking that it should be cached.
-        """
-        # If this is a no-op what URI would it return?
-        raise RuntimeError("Caching is disabled. Please use should_be_cached() method prior to"
-                           " trying to cache a dataset")
+    def move_to_cache(self, uri: ButlerURI, ref: DatasetRef) -> Optional[ButlerURI]:
+        """Move dataset to cache but always refuse and returns `None`."""
+        return None
 
     def find_in_cache(self, ref: DatasetRef, extension: str) -> Optional[ButlerURI]:
         """Look for a dataset in the cache and return its location.

python/lsst/daf/butler/datastores/fileDatastore.py

@@ -1009,8 +1009,7 @@ def _removeFileExists(uri: ButlerURI) -> None:
                     uri.transfer_from(tmpLocation.uri, transfer="copy", overwrite=True)
 
                     # Cache if required
-                    if self.cacheManager.should_be_cached(ref):
-                        self.cacheManager.move_to_cache(tmpLocation.uri, ref)
+                    self.cacheManager.move_to_cache(tmpLocation.uri, ref)
 
                 log.debug("Successfully wrote dataset to %s via a temporary file.", uri)
 
@@ -1096,8 +1095,9 @@ def _read_artifact_into_memory(self, getInfo: DatastoreFileGetInformation,
                     location_updated = True
 
                     # Cache the downloaded file if needed.
-                    if self.cacheManager.should_be_cached(ref):
-                        local_uri = self.cacheManager.move_to_cache(local_uri, ref)
+                    cached_uri = self.cacheManager.move_to_cache(local_uri, ref)
+                    if cached_uri is not None:
+                        local_uri = cached_uri
                         cache_msg = " and cached"
 
                     msg = f"(via download to local file{cache_msg})"