Skip to content

Commit

Permalink
Match RemoteButler.get() exceptions to Direct
Browse files Browse the repository at this point in the history
It turns out that DirectButler typically throws two different exceptions from get():
1. DatasetNotFoundError if the dataset can't be found in registry
2.  FileNotFoundError if the file itself cannot be found or under some other error conditions (some of which involve not being able to find the dataset in registry.)

Added test coverage for case #1, and fixed RemoteButler to comply with it.  Loosened the tests for some unusual error conditions to allow either of the two exception types, because RemoteButler doesn't have enough information available to replicate the DirectButler behavior.
  • Loading branch information
dhirving committed Mar 8, 2024
1 parent 5f502db commit 4dd2f92
Show file tree
Hide file tree
Showing 3 changed files with 11 additions and 8 deletions.
5 changes: 1 addition & 4 deletions python/lsst/daf/butler/remote_butler/_remote_butler.py
Original file line number Diff line number Diff line change
Expand Up @@ -264,10 +264,7 @@ def get(
**kwargs: Any,
) -> Any:
# Docstring inherited.
try:
model = self._get_file_info(datasetRefOrType, dataId, collections, kwargs)
except DatasetNotFoundError as e:
raise FileNotFoundError(str(e)) from e
model = self._get_file_info(datasetRefOrType, dataId, collections, kwargs)

ref = DatasetRef.from_simple(model.dataset_ref, universe=self.dimensions)
# If the caller provided a DatasetRef, they may have overridden the
Expand Down
9 changes: 7 additions & 2 deletions tests/test_butler.py
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ def mock_aws(*args: Any, **kwargs: Any) -> Any: # type: ignore[no-untyped-def]
Config,
DataCoordinate,
DatasetExistence,
DatasetNotFoundError,
DatasetRef,
DatasetType,
FileDataset,
Expand Down Expand Up @@ -287,6 +288,10 @@ def runPutGetTest(self, storageClass: StorageClass, datasetTypeName: str) -> But
metric = makeExampleMetrics()
dataId = butler.registry.expandDataId({"instrument": "DummyCamComp", "visit": 423})

# Dataset should not exist if we haven't added it
with self.assertRaises(DatasetNotFoundError):
butler.get(datasetTypeName, dataId)

# Put and remove the dataset once as a DatasetRef, once as a dataId,
# and once with a DatasetType

Expand Down Expand Up @@ -413,7 +418,7 @@ def runPutGetTest(self, storageClass: StorageClass, datasetTypeName: str) -> But
kwargs = {} # Prevent warning from being issued.
self.assertFalse(butler.exists(*args, **kwargs))
# get() should still fail.
with self.assertRaises(FileNotFoundError):
with self.assertRaises((FileNotFoundError, DatasetNotFoundError)):
butler.get(ref)
# Registry shouldn't be able to find it by dataset_id anymore.
self.assertIsNone(butler.get_dataset(ref.id))
Expand Down Expand Up @@ -496,7 +501,7 @@ def runPutGetTest(self, storageClass: StorageClass, datasetTypeName: str) -> But
with self.assertRaisesRegex(ValueError, "DatasetRef given, cannot use dataId as well"):
butler.get(ref, dataId)
# Getting with an explicit ref should fail if the id doesn't match.
with self.assertRaises(FileNotFoundError):
with self.assertRaises((FileNotFoundError, DatasetNotFoundError)):
butler.get(DatasetRef(ref.datasetType, ref.dataId, id=uuid.UUID(int=101), run=butler.run))

# Getting a dataset with unknown parameters should fail
Expand Down
5 changes: 3 additions & 2 deletions tests/test_server.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@
from lsst.daf.butler import (
Butler,
DataCoordinate,
DatasetNotFoundError,
DatasetRef,
LabeledButlerFactory,
MissingDatasetTypeError,
Expand Down Expand Up @@ -229,7 +230,7 @@ def test_get(self):
self.assertEqual(metric, kwarg_data_coordinate_metric)
# Test get() of a non-existent DataId.
invalid_data_id = {"instrument": "NotAValidlInstrument", "visit": 423}
with self.assertRaises(FileNotFoundError):
with self.assertRaises(DatasetNotFoundError):
self.butler_without_error_propagation.get(
dataset_type, dataId=invalid_data_id, collections=collections
)
Expand All @@ -245,7 +246,7 @@ def test_get(self):

# Test looking up a non-existent ref
invalid_ref = ref.replace(id=uuid.uuid4())
with self.assertRaises(FileNotFoundError):
with self.assertRaises(DatasetNotFoundError):
self.butler_without_error_propagation.get(invalid_ref)

with self.assertRaises(RuntimeError):
Expand Down

0 comments on commit 4dd2f92

Please sign in to comment.