Skip to content

Commit

Permalink
Move registry factory methods to a separate class.
Browse files Browse the repository at this point in the history
Trying to further separate concerns into independent classes. Re-introduced
`Registry.createFromConfig` to avoid updating tests in many other packages.
  • Loading branch information
andy-slac committed Jul 12, 2023
1 parent 96adaec commit 429e7af
Show file tree
Hide file tree
Showing 12 changed files with 256 additions and 57 deletions.
7 changes: 5 additions & 2 deletions python/lsst/daf/butler/_butler.py
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@
Registry,
RegistryConfig,
RegistryDefaults,
RegistryFactory,
)
from .transfers import RepoExportContext

Expand Down Expand Up @@ -224,7 +225,7 @@ def __init__(
butlerRoot = self._config.configDir
if writeable is None:
writeable = run is not None
self._registry = ButlerRegistry.fromConfig(
self._registry = RegistryFactory.from_config(
self._config, butlerRoot=butlerRoot, writeable=writeable, defaults=defaults
)
self._datastore = Datastore.fromConfig(
Expand Down Expand Up @@ -462,7 +463,9 @@ def makeRepo(
# Create Registry and populate tables
registryConfig = RegistryConfig(config.get("registry"))
dimensionConfig = DimensionConfig(dimensionConfig)
ButlerRegistry.createFromConfig(registryConfig, dimensionConfig=dimensionConfig, butlerRoot=root_uri)
RegistryFactory.create_from_config(
registryConfig, dimensionConfig=dimensionConfig, butlerRoot=root_uri
)

log.verbose("Wrote new Butler configuration file to %s", configURI)

Expand Down
1 change: 1 addition & 0 deletions python/lsst/daf/butler/registry/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
from ._defaults import *
from ._exceptions import *
from ._registry import *
from ._registry_factory import *
from .wildcards import CollectionSearch

# Some modules intentionally not imported, either because they are purely
Expand Down
41 changes: 4 additions & 37 deletions python/lsst/daf/butler/registry/_butler_registry.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@
from typing import TYPE_CHECKING

from lsst.resources import ResourcePathExpression
from lsst.utils import doImportType

from ..core import Config, DimensionConfig
from ._config import RegistryConfig
Expand Down Expand Up @@ -79,38 +78,7 @@ def forceRegistryConfig(
return config

@classmethod
def determineTrampoline(
cls, config: ButlerConfig | RegistryConfig | Config | str | None
) -> tuple[type[ButlerRegistry], RegistryConfig]:
"""Return class to use to instantiate real registry.
Parameters
----------
config : `RegistryConfig` or `str`, optional
Registry configuration, if missing then default configuration will
be loaded from registry.yaml.
Returns
-------
requested_cls : `type` of `ButlerRegistry`
The real registry class to use.
registry_config : `RegistryConfig`
The `RegistryConfig` to use.
"""
config = cls.forceRegistryConfig(config)

# Default to the standard registry
registry_cls_name = config.get("cls", "lsst.daf.butler.registries.sql.SqlRegistry")
registry_cls = doImportType(registry_cls_name)
if registry_cls is cls:
raise ValueError("Can not instantiate the abstract base Registry from config")
if not issubclass(registry_cls, ButlerRegistry):
raise TypeError(
f"Registry class obtained from config {registry_cls_name} is not a ButlerRegistry class."
)
return registry_cls, config

@classmethod
@abstractmethod
def createFromConfig(
cls,
config: RegistryConfig | str | None = None,
Expand Down Expand Up @@ -144,10 +112,10 @@ def createFromConfig(
use from configuration. Each subclass should implement this method
even if it can not create a registry.
"""
registry_cls, registry_config = cls.determineTrampoline(config)
return registry_cls.createFromConfig(registry_config, dimensionConfig, butlerRoot)
raise NotImplementedError()

@classmethod
@abstractmethod
def fromConfig(
cls,
config: ButlerConfig | RegistryConfig | Config | str,
Expand Down Expand Up @@ -185,8 +153,7 @@ def fromConfig(
# subclass. No implementation should ever use this implementation
# directly. If no class is specified, default to the standard
# registry.
registry_cls, registry_config = cls.determineTrampoline(config)
return registry_cls.fromConfig(config, butlerRoot, writeable, defaults)
raise NotImplementedError()

@abstractmethod
def copy(self, defaults: RegistryDefaults | None = None) -> ButlerRegistry:
Expand Down
43 changes: 43 additions & 0 deletions python/lsst/daf/butler/registry/_registry.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@
from types import EllipsisType
from typing import TYPE_CHECKING, Any

from lsst.resources import ResourcePathExpression

from ..core import (
DataCoordinate,
DataId,
Expand All @@ -40,6 +42,7 @@
DatasetRef,
DatasetType,
Dimension,
DimensionConfig,
DimensionElement,
DimensionGraph,
DimensionRecord,
Expand All @@ -50,6 +53,7 @@
)
from ._collection_summary import CollectionSummary
from ._collectionType import CollectionType
from ._config import RegistryConfig
from ._defaults import RegistryDefaults
from .queries import DataCoordinateQueryResults, DatasetQueryResults, DimensionRecordQueryResults
from .wildcards import CollectionWildcard
Expand All @@ -71,6 +75,45 @@ class Registry(ABC):
implementations.
"""

@classmethod
def createFromConfig(
cls,
config: RegistryConfig | str | None = None,
dimensionConfig: DimensionConfig | str | None = None,
butlerRoot: ResourcePathExpression | None = None,
) -> Registry:
"""Create registry database and return `Registry` instance.
This method initializes database contents, database must be empty
prior to calling this method.
Parameters
----------
config : `RegistryConfig` or `str`, optional
Registry configuration, if missing then default configuration will
be loaded from registry.yaml.
dimensionConfig : `DimensionConfig` or `str`, optional
Dimensions configuration, if missing then default configuration
will be loaded from dimensions.yaml.
butlerRoot : convertible to `lsst.resources.ResourcePath`, optional
Path to the repository root this `Registry` will manage.
Returns
-------
registry : `Registry`
A new `Registry` instance.
Notes
-----
This method is for backward compatibility only, until all clients
migrate to use new `~lsst.daf.butler.registry.RegistryFactory` factory
class. Regular clients of registry class do not use this method, it is
only used by tests in multiple packages.
"""
from ._registry_factory import RegistryFactory

Check warning on line 113 in python/lsst/daf/butler/registry/_registry.py

View check run for this annotation

Codecov / codecov/patch

python/lsst/daf/butler/registry/_registry.py#L113

Added line #L113 was not covered by tests

return RegistryFactory.create_from_config(config, dimensionConfig, butlerRoot)

Check warning on line 115 in python/lsst/daf/butler/registry/_registry.py

View check run for this annotation

Codecov / codecov/patch

python/lsst/daf/butler/registry/_registry.py#L115

Added line #L115 was not covered by tests

@abstractmethod
def isWriteable(self) -> bool:
"""Return `True` if this registry allows write operations, and `False`
Expand Down
185 changes: 185 additions & 0 deletions python/lsst/daf/butler/registry/_registry_factory.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,185 @@
# This file is part of daf_butler.
#
# Developed for the LSST Data Management System.
# This product includes software developed by the LSST Project
# (http://www.lsst.org).
# See the COPYRIGHT file at the top-level directory of this distribution
# for details of code ownership.
#
# This program is free software: you can redistribute it and/or modify
# it under the terms of the GNU General Public License as published by
# the Free Software Foundation, either version 3 of the License, or
# (at your option) any later version.
#
# This program is distributed in the hope that it will be useful,
# but WITHOUT ANY WARRANTY; without even the implied warranty of
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
# GNU General Public License for more details.
#
# You should have received a copy of the GNU General Public License
# along with this program. If not, see <http://www.gnu.org/licenses/>.

from __future__ import annotations

__all__ = ("RegistryFactory",)

from typing import TYPE_CHECKING

from lsst.resources import ResourcePathExpression
from lsst.utils import doImportType

from ..core import Config, DimensionConfig
from ._butler_registry import ButlerRegistry
from ._config import RegistryConfig
from ._defaults import RegistryDefaults

if TYPE_CHECKING:
from .._butlerConfig import ButlerConfig


class RegistryFactory:
"""Interface for creating and initializing Registry instances.
Each registry implementation can have its own constructor parameters.
The assumption is that an instance of a specific subclass will be
constructed from configuration using ``RegistryClass.fromConfig()`` or
``RegistryClass.createFromConfig()``.
This class will look for a ``cls`` entry in registry configuration object
(defaulting to ``SqlRegistry``), import that class, and call one of the
above methods on the imported class.
"""

@classmethod
def force_registry_config(
cls, config: ButlerConfig | RegistryConfig | Config | str | None
) -> RegistryConfig:
"""Force the supplied config to a `RegistryConfig`.
Parameters
----------
config : `RegistryConfig`, `Config` or `str` or `None`
Registry configuration, if missing then default configuration will
be loaded from registry.yaml.
Returns
-------
registry_config : `RegistryConfig`
A registry config.
"""
if not isinstance(config, RegistryConfig):
if isinstance(config, (str, Config)) or config is None:
config = RegistryConfig(config)
else:
raise ValueError(f"Incompatible Registry configuration: {config}")

Check warning on line 74 in python/lsst/daf/butler/registry/_registry_factory.py

View check run for this annotation

Codecov / codecov/patch

python/lsst/daf/butler/registry/_registry_factory.py#L74

Added line #L74 was not covered by tests
return config

@classmethod
def determine_trampoline(
cls, config: ButlerConfig | RegistryConfig | Config | str | None
) -> tuple[type[ButlerRegistry], RegistryConfig]:
"""Return class to use to instantiate real registry.
Parameters
----------
config : `RegistryConfig` or `str`, optional
Registry configuration, if missing then default configuration will
be loaded from registry.yaml.
Returns
-------
requested_cls : `type` of `ButlerRegistry`
The real registry class to use.
registry_config : `RegistryConfig`
The `RegistryConfig` to use.
"""
config = cls.force_registry_config(config)

# Default to the standard registry
registry_cls_name = config.get("cls", "lsst.daf.butler.registries.sql.SqlRegistry")
registry_cls = doImportType(registry_cls_name)
if registry_cls is cls:
raise ValueError("Can not instantiate the abstract base Registry from config")

Check warning on line 102 in python/lsst/daf/butler/registry/_registry_factory.py

View check run for this annotation

Codecov / codecov/patch

python/lsst/daf/butler/registry/_registry_factory.py#L102

Added line #L102 was not covered by tests
if not issubclass(registry_cls, ButlerRegistry):
raise TypeError(

Check warning on line 104 in python/lsst/daf/butler/registry/_registry_factory.py

View check run for this annotation

Codecov / codecov/patch

python/lsst/daf/butler/registry/_registry_factory.py#L104

Added line #L104 was not covered by tests
f"Registry class obtained from config {registry_cls_name} is not a ButlerRegistry class."
)
return registry_cls, config

@classmethod
def create_from_config(
cls,
config: RegistryConfig | str | None = None,
dimensionConfig: DimensionConfig | str | None = None,
butlerRoot: ResourcePathExpression | None = None,
) -> ButlerRegistry:
"""Create registry database and return `ButlerRegistry` instance.
This method initializes database contents, database must be empty
prior to calling this method.
Parameters
----------
config : `RegistryConfig` or `str`, optional
Registry configuration, if missing then default configuration will
be loaded from registry.yaml.
dimensionConfig : `DimensionConfig` or `str`, optional
Dimensions configuration, if missing then default configuration
will be loaded from dimensions.yaml.
butlerRoot : convertible to `lsst.resources.ResourcePath`, optional
Path to the repository root this `Registry` will manage.
Returns
-------
registry : `ButlerRegistry`
A new `ButlerRegistry` instance.
Notes
-----
This class will determine the concrete `ButlerRegistry` subclass to
use from configuration. Each subclass should implement this method
even if it can not create a registry.
"""
registry_cls, registry_config = cls.determine_trampoline(config)
return registry_cls.createFromConfig(registry_config, dimensionConfig, butlerRoot)

@classmethod
def from_config(
cls,
config: ButlerConfig | RegistryConfig | Config | str,
butlerRoot: ResourcePathExpression | None = None,
writeable: bool = True,
defaults: RegistryDefaults | None = None,
) -> ButlerRegistry:
"""Create `ButlerRegistry` subclass instance from ``config``.
Registry database must be initialized prior to calling this method.
Parameters
----------
config : `ButlerConfig`, `RegistryConfig`, `Config` or `str`
Registry configuration
butlerRoot : convertible to `lsst.resources.ResourcePath`, optional
Path to the repository root this `Registry` will manage.
writeable : `bool`, optional
If `True` (default) create a read-write connection to the database.
defaults : `~lsst.daf.butler.registry.RegistryDefaults`, optional
Default collection search path and/or output `~CollectionType.RUN`
collection.
Returns
-------
registry : `ButlerRegistry` (subclass)
A new `ButlerRegistry` subclass instance.
Notes
-----
This class will determine the concrete `ButlerRegistry` subclass to
use from configuration. Each subclass should implement this method.
"""
# The base class implementation should trampoline to the correct
# subclass. No implementation should ever use this implementation
# directly. If no class is specified, default to the standard
# registry.
registry_cls, registry_config = cls.determine_trampoline(config)
return registry_cls.fromConfig(config, butlerRoot, writeable, defaults)
4 changes: 2 additions & 2 deletions tests/test_dimensions.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@
TimespanDatabaseRepresentation,
YamlRepoImportBackend,
)
from lsst.daf.butler.registry import ButlerRegistry, RegistryConfig
from lsst.daf.butler.registry import RegistryConfig, RegistryFactory

DIMENSION_DATA_FILE = os.path.normpath(
os.path.join(os.path.dirname(__file__), "data", "registry", "hsc-rc2-subset.yaml")
Expand All @@ -64,7 +64,7 @@ def loadDimensionData() -> DataCoordinateSequence:
# data and retreive it as a set of DataCoordinate objects.
config = RegistryConfig()
config["db"] = "sqlite://"
registry = ButlerRegistry.createFromConfig(config)
registry = RegistryFactory.create_from_config(config)
with open(DIMENSION_DATA_FILE) as stream:
backend = YamlRepoImportBackend(stream, registry)
backend.register()
Expand Down
4 changes: 2 additions & 2 deletions tests/test_obscore.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@
StorageClassFactory,
)
from lsst.daf.butler.registries.sql import SqlRegistry
from lsst.daf.butler.registry import ButlerRegistry, Registry, RegistryConfig
from lsst.daf.butler.registry import ButlerRegistry, Registry, RegistryConfig, RegistryFactory
from lsst.daf.butler.registry.obscore import (
DatasetTypeConfig,
ObsCoreConfig,
Expand Down Expand Up @@ -67,7 +67,7 @@ def make_registry(
) -> ButlerRegistry:
"""Create new empty Registry."""
config = self.make_registry_config(collections, collection_type)
registry = ButlerRegistry.createFromConfig(config, butlerRoot=self.root)
registry = RegistryFactory.create_from_config(config, butlerRoot=self.root)
self.initialize_registry(registry)
return registry

Expand Down

0 comments on commit 429e7af

Please sign in to comment.