Skip to content

Commit

Permalink
Move butler.datastore to _datastore and issue deprecation warning
Browse files Browse the repository at this point in the history
  • Loading branch information
timj committed Jul 6, 2023
1 parent 2674b5c commit c3aa0e9
Show file tree
Hide file tree
Showing 10 changed files with 128 additions and 120 deletions.
70 changes: 36 additions & 34 deletions python/lsst/daf/butler/_butler.py
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,7 @@ def __init__(
"Cannot pass 'config', 'searchPaths', or 'writeable' arguments with 'butler' argument."
)
self.registry = butler.registry.copy(defaults)
self.datastore = butler.datastore
self._datastore = butler._datastore
self.storageClasses = butler.storageClasses
self._config: ButlerConfig = butler._config
else:
Expand All @@ -225,7 +225,7 @@ def __init__(
self.registry = Registry.fromConfig(
self._config, butlerRoot=butlerRoot, writeable=writeable, defaults=defaults
)
self.datastore = Datastore.fromConfig(
self._datastore = Datastore.fromConfig(
self._config, self.registry.getDatastoreBridgeManager(), butlerRoot=butlerRoot
)
self.storageClasses = StorageClassFactory()
Expand All @@ -240,7 +240,7 @@ def __init__(
# dependency-inversion trick. This is not used by regular butler,
# but we do not have a way to distinguish regular butler from execution
# butler.
self.datastore.set_retrieve_dataset_type_method(self._retrieve_dataset_type)
self._datastore.set_retrieve_dataset_type_method(self._retrieve_dataset_type)

if "run" in self._config or "collection" in self._config:
raise ValueError("Passing a run or collection via configuration is no longer supported.")
Expand Down Expand Up @@ -524,7 +524,7 @@ def __reduce__(self) -> tuple:

def __str__(self) -> str:
return "Butler(collections={}, run={}, datastore='{}', registry='{}')".format(
self.collections, self.run, self.datastore, self.registry
self.collections, self.run, self._datastore, self.registry
)

def isWriteable(self) -> bool:
Expand All @@ -538,7 +538,7 @@ def transaction(self) -> Iterator[None]:
Transactions can be nested.
"""
with self.registry.transaction():
with self.datastore.transaction():
with self._datastore.transaction():
yield

def _standardizeArgs(
Expand Down Expand Up @@ -1163,13 +1163,13 @@ def put(
self.registry._importDatasets([datasetRefOrType], expand=True)
# Before trying to write to the datastore check that it does not
# know this dataset. This is prone to races, of course.
if self.datastore.knows(datasetRefOrType):
if self._datastore.knows(datasetRefOrType):
raise ConflictingDefinitionError(f"Datastore already contains dataset: {datasetRefOrType}")
# Try to write dataset to the datastore, if it fails due to a race
# with another write, the content of stored data may be
# unpredictable.
try:
self.datastore.put(obj, datasetRefOrType)
self._datastore.put(obj, datasetRefOrType)
except IntegrityError as e:
raise ConflictingDefinitionError(f"Datastore already contains dataset: {e}")
return datasetRefOrType
Expand All @@ -1185,7 +1185,7 @@ def put(
# Add Registry Dataset entry.
dataId = self.registry.expandDataId(dataId, graph=datasetType.dimensions, **kwargs)
(ref,) = self.registry.insertDatasets(datasetType, run=run, dataIds=[dataId])
self.datastore.put(obj, ref)
self._datastore.put(obj, ref)

return ref

Expand Down Expand Up @@ -1223,7 +1223,7 @@ def getDirect(
obj : `object`
The dataset.
"""
return self.datastore.get(ref, parameters=parameters, storageClass=storageClass)
return self._datastore.get(ref, parameters=parameters, storageClass=storageClass)

Check warning on line 1226 in python/lsst/daf/butler/_butler.py

View check run for this annotation

Codecov / codecov/patch

python/lsst/daf/butler/_butler.py#L1226

Added line #L1226 was not covered by tests

@deprecated(
reason="Butler.getDeferred() now behaves like getDirectDeferred() when given a DatasetRef. "
Expand Down Expand Up @@ -1266,7 +1266,7 @@ def getDirectDeferred(
Raised if no matching dataset exists in the `Registry`.
"""
# Check thad dataset actuall exists.
if not self.datastore.exists(ref):
if not self._datastore.exists(ref):
raise LookupError(f"Dataset reference {ref} does not exist.")
return DeferredDatasetHandle(butler=self, ref=ref, parameters=parameters, storageClass=storageClass)

Expand Down Expand Up @@ -1325,7 +1325,7 @@ def getDeferred(
TypeError
Raised if no collections were provided.
"""
if isinstance(datasetRefOrType, DatasetRef) and not self.datastore.exists(datasetRefOrType):
if isinstance(datasetRefOrType, DatasetRef) and not self._datastore.exists(datasetRefOrType):
raise LookupError(f"Dataset reference {datasetRefOrType} does not exist.")
ref = self._findDatasetRef(datasetRefOrType, dataId, collections=collections, **kwargs)
return DeferredDatasetHandle(butler=self, ref=ref, parameters=parameters, storageClass=storageClass)
Expand Down Expand Up @@ -1396,7 +1396,7 @@ def get(
"""
log.debug("Butler get: %s, dataId=%s, parameters=%s", datasetRefOrType, dataId, parameters)
ref = self._findDatasetRef(datasetRefOrType, dataId, collections=collections, **kwargs)
return self.datastore.get(ref, parameters=parameters, storageClass=storageClass)
return self._datastore.get(ref, parameters=parameters, storageClass=storageClass)

def getURIs(
self,
Expand Down Expand Up @@ -1445,7 +1445,7 @@ def getURIs(
ref = self._findDatasetRef(
datasetRefOrType, dataId, predict=predict, run=run, collections=collections, **kwargs
)
return self.datastore.getURIs(ref, predict)
return self._datastore.getURIs(ref, predict)

def getURI(
self,
Expand Down Expand Up @@ -1562,7 +1562,7 @@ def retrieveArtifacts(
a hierarchical data structure in a NoSQL database may well be stored
as a JSON file.
"""
return self.datastore.retrieveArtifacts(
return self._datastore.retrieveArtifacts(
refs,
ResourcePath(destination),
transfer=transfer,
Expand Down Expand Up @@ -1645,11 +1645,11 @@ def exists(
return existence
existence |= DatasetExistence.RECORDED

if self.datastore.knows(ref):
if self._datastore.knows(ref):
existence |= DatasetExistence.DATASTORE

if full_check:
if self.datastore.exists(ref):
if self._datastore.exists(ref):
existence |= DatasetExistence._ARTIFACT
elif existence.value != DatasetExistence.UNRECOGNIZED.value:
# Do not add this flag if we have no other idea about a dataset.
Expand Down Expand Up @@ -1706,13 +1706,13 @@ def _exists_many(
existence[ref] |= DatasetExistence.RECORDED

# Ask datastore if it knows about these refs.
knows = self.datastore.knows_these(refs)
knows = self._datastore.knows_these(refs)
for ref, known in knows.items():
if known:
existence[ref] |= DatasetExistence.DATASTORE

if full_check:
mexists = self.datastore.mexists(refs)
mexists = self._datastore.mexists(refs)
for ref, exists in mexists.items():
if exists:
existence[ref] |= DatasetExistence._ARTIFACT
Expand Down Expand Up @@ -1776,7 +1776,7 @@ def datasetExists(
)
else:
ref = self._findDatasetRef(datasetRefOrType, dataId, collections=collections, **kwargs)
return self.datastore.exists(ref)
return self._datastore.exists(ref)

Check warning on line 1779 in python/lsst/daf/butler/_butler.py

View check run for this annotation

Codecov / codecov/patch

python/lsst/daf/butler/_butler.py#L1779

Added line #L1779 was not covered by tests

def removeRuns(self, names: Iterable[str], unstore: bool = True) -> None:
"""Remove one or more `~CollectionType.RUN` collections and the
Expand Down Expand Up @@ -1808,17 +1808,17 @@ def removeRuns(self, names: Iterable[str], unstore: bool = True) -> None:
if collectionType is not CollectionType.RUN:
raise TypeError(f"The collection type of '{name}' is {collectionType.name}, not RUN.")
refs.extend(self.registry.queryDatasets(..., collections=name, findFirst=True))
with self.datastore.transaction():
with self._datastore.transaction():
with self.registry.transaction():
if unstore:
self.datastore.trash(refs)
self._datastore.trash(refs)
else:
self.datastore.forget(refs)
self._datastore.forget(refs)
for name in names:
self.registry.removeCollection(name)
if unstore:
# Point of no return for removing artifacts
self.datastore.emptyTrash()
self._datastore.emptyTrash()

def pruneDatasets(
self,
Expand Down Expand Up @@ -1863,10 +1863,10 @@ def pruneDatasets(
# mutating the Registry (it can _look_ at Datastore-specific things,
# but shouldn't change them), and hence all operations here are
# Registry operations.
with self.datastore.transaction():
with self._datastore.transaction():
with self.registry.transaction():
if unstore:
self.datastore.trash(refs)
self._datastore.trash(refs)
if purge:
self.registry.removeDatasets(refs)
elif disassociate:
Expand All @@ -1884,7 +1884,7 @@ def pruneDatasets(
# in the dataset_location_trash table.
if unstore:
# Point of no return for removing artifacts
self.datastore.emptyTrash()
self._datastore.emptyTrash()

@transactional
def ingest(
Expand Down Expand Up @@ -2056,7 +2056,9 @@ def ingest(
# (_importDatasets only complains if they exist but differ) so
# we have to catch IntegrityError explicitly.
try:
self.datastore.ingest(*datasets, transfer=transfer, record_validation_info=record_validation_info)
self._datastore.ingest(
*datasets, transfer=transfer, record_validation_info=record_validation_info
)
except IntegrityError as e:
raise ConflictingDefinitionError(f"Datastore already contains one or more datasets: {e}")

Expand Down Expand Up @@ -2136,7 +2138,7 @@ def export(
backend = BackendClass(stream, universe=self.dimensions)
try:
helper = RepoExportContext(
self.registry, self.datastore, backend=backend, directory=directory, transfer=transfer
self.registry, self._datastore, backend=backend, directory=directory, transfer=transfer
)
yield helper
except BaseException:
Expand Down Expand Up @@ -2230,7 +2232,7 @@ def doImport(importStream: TextIO | ResourceHandleProtocol) -> None:
backend.register()
with self.transaction():
backend.load(
self.datastore,
self._datastore,
directory=directory,
transfer=transfer,
skip_dimensions=skip_dimensions,
Expand Down Expand Up @@ -2315,7 +2317,7 @@ def transfer_from(
# this with no datastore records.
artifact_existence: dict[ResourcePath, bool] = {}
if skip_missing:
dataset_existence = source_butler.datastore.mexists(
dataset_existence = source_butler._datastore.mexists(
source_refs, artifact_existence=artifact_existence
)
source_refs = [ref for ref, exists in dataset_existence.items() if exists]
Expand Down Expand Up @@ -2442,8 +2444,8 @@ def transfer_from(

# Ask the datastore to transfer. The datastore has to check that
# the source datastore is compatible with the target datastore.
accepted, rejected = self.datastore.transfer_from(
source_butler.datastore,
accepted, rejected = self._datastore.transfer_from(
source_butler._datastore,
source_refs,
transfer=transfer,
artifact_existence=artifact_existence,
Expand Down Expand Up @@ -2536,13 +2538,13 @@ def validateConfiguration(

datastoreErrorStr = None
try:
self.datastore.validateConfiguration(entities, logFailures=logFailures)
self._datastore.validateConfiguration(entities, logFailures=logFailures)
except ValidationError as e:
datastoreErrorStr = str(e)

# Also check that the LookupKeys used by the datastores match
# registry and storage class definitions
keys = self.datastore.getLookupKeys()
keys = self._datastore.getLookupKeys()

failedNames = set()
failedDataId = set()
Expand Down
26 changes: 16 additions & 10 deletions python/lsst/daf/butler/_limited_butler.py
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ def get(
to use a resolved `DatasetRef`. Subclasses can support more options.
"""
log.debug("Butler get: %s, parameters=%s, storageClass: %s", ref, parameters, storageClass)
return self.datastore.get(ref, parameters=parameters, storageClass=storageClass)
return self._datastore.get(ref, parameters=parameters, storageClass=storageClass)

@deprecated(
reason="Butler.get() now behaves like Butler.getDirect() when given a DatasetRef."
Expand Down Expand Up @@ -197,7 +197,7 @@ def getDirect(
obj : `object`
The dataset.
"""
return self.datastore.get(ref, parameters=parameters, storageClass=storageClass)
return self._datastore.get(ref, parameters=parameters, storageClass=storageClass)

Check warning on line 200 in python/lsst/daf/butler/_limited_butler.py

View check run for this annotation

Codecov / codecov/patch

python/lsst/daf/butler/_limited_butler.py#L200

Added line #L200 was not covered by tests

@deprecated(
reason="Butler.getDeferred() now behaves like getDirectDeferred() when given a DatasetRef. "
Expand Down Expand Up @@ -289,7 +289,7 @@ def stored(self, ref: DatasetRef) -> bool:
Whether the dataset artifact exists in the datastore and can be
retrieved.
"""
return self.datastore.exists(ref)
return self._datastore.exists(ref)

def stored_many(
self,
Expand All @@ -309,7 +309,7 @@ def stored_many(
Mapping from given dataset refs to boolean indicating artifact
existence.
"""
return self.datastore.mexists(refs)
return self._datastore.mexists(refs)

@deprecated(
reason="Butler.datasetExistsDirect() has been replaced by Butler.stored(). "
Expand Down Expand Up @@ -410,13 +410,19 @@ def dimensions(self) -> DimensionUniverse:
"""
raise NotImplementedError()

datastore: Datastore
"""The object that manages actual dataset storage (`Datastore`).
@property
@deprecated(
reason="The Butler.datastore property is now deprecated. Butler APIs should now exist with the "
"relevant functionality. Will be removed after v27.0.",
version="v26.0",
category=FutureWarning,
)
def datastore(self) -> Datastore:
"""The object that manages actual dataset storage. (`Datastore`)"""
return self._datastore

Check warning on line 422 in python/lsst/daf/butler/_limited_butler.py

View check run for this annotation

Codecov / codecov/patch

python/lsst/daf/butler/_limited_butler.py#L422

Added line #L422 was not covered by tests

Direct user access to the datastore should rarely be necessary; the primary
exception is the case where a `Datastore` implementation provides extra
functionality beyond what the base class defines.
"""
_datastore: Datastore
"""The object that manages actual dataset storage (`Datastore`)."""

storageClasses: StorageClassFactory
"""An object that maps known storage class names to objects that fully
Expand Down
Loading

0 comments on commit c3aa0e9

Please sign in to comment.