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 13, 2023
1 parent 1e82440 commit 020fe5b
Show file tree
Hide file tree
Showing 12 changed files with 200 additions and 58 deletions.
9 changes: 6 additions & 3 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,8 +225,8 @@ def __init__(
butlerRoot = self._config.configDir
if writeable is None:
writeable = run is not None
self._registry = ButlerRegistry.fromConfig(
self._config, butlerRoot=butlerRoot, writeable=writeable, defaults=defaults
self._registry = RegistryFactory(self._config).from_config(
butlerRoot=butlerRoot, writeable=writeable, defaults=defaults
)
self._datastore = Datastore.fromConfig(
self._config, self._registry.getDatastoreBridgeManager(), butlerRoot=butlerRoot
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(registryConfig).create_from_config(
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(config).create_from_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
128 changes: 128 additions & 0 deletions python/lsst/daf/butler/registry/_registry_factory.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,128 @@
# 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.
Parameters
----------
config : `RegistryConfig` or `str`, optional
Registry configuration, if missing then default configuration will
be loaded from registry.yaml.
Notes
-----
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.
"""

def __init__(self, config: ButlerConfig | RegistryConfig | Config | str | None):
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 66 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#L66

Added line #L66 was not covered by tests
self._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 not issubclass(registry_cls, ButlerRegistry):
raise TypeError(

Check warning on line 73 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#L73

Added line #L73 was not covered by tests
f"Registry class obtained from config {registry_cls_name} is not a ButlerRegistry class."
)
self._registry_cls = registry_cls

def create_from_config(
self,
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
----------
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.
"""
return self._registry_cls.createFromConfig(self._config, dimensionConfig, butlerRoot)

def from_config(
self,
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
----------
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.
"""
return self._registry_cls.fromConfig(self._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(config).create_from_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(config).create_from_config(butlerRoot=self.root)
self.initialize_registry(registry)
return registry

Expand Down
6 changes: 3 additions & 3 deletions tests/test_postgresql.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@

import sqlalchemy
from lsst.daf.butler import Timespan, ddl
from lsst.daf.butler.registry import ButlerRegistry
from lsst.daf.butler.registry import ButlerRegistry, RegistryFactory
from lsst.daf.butler.registry.databases.postgresql import PostgresqlDatabase, _RangeTimespanType
from lsst.daf.butler.registry.tests import DatabaseTests, RegistryTests
from lsst.daf.butler.tests.utils import makeTestTempDir, removeTestTempDir
Expand Down Expand Up @@ -245,9 +245,9 @@ def makeRegistry(self, share_repo_with: ButlerRegistry | None = None) -> ButlerR
config["db"] = self.server.url()
config["namespace"] = namespace
if share_repo_with is None:
return ButlerRegistry.createFromConfig(config)
return RegistryFactory(config).create_from_config()
else:
return ButlerRegistry.fromConfig(config)
return RegistryFactory(config).from_config()


class PostgresqlRegistryNameKeyCollMgrUUIDTestCase(PostgresqlRegistryTests, unittest.TestCase):
Expand Down
4 changes: 2 additions & 2 deletions tests/test_quantumBackedButler.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@
RegistryConfig,
StorageClass,
)
from lsst.daf.butler.registry import ButlerRegistry
from lsst.daf.butler.registry import RegistryFactory
from lsst.daf.butler.tests.utils import makeTestTempDir, removeTestTempDir
from lsst.resources import ResourcePath

Expand All @@ -55,7 +55,7 @@ def setUp(self) -> None:

# Make a butler and import dimension definitions.
registryConfig = RegistryConfig(self.config.get("registry"))
ButlerRegistry.createFromConfig(registryConfig, butlerRoot=self.root)
RegistryFactory(registryConfig).create_from_config(butlerRoot=self.root)
self.butler = Butler(self.config, writeable=True, run="RUN")
self.butler.import_(filename=os.path.join(TESTDIR, "data", "registry", "base.yaml"))

Expand Down

0 comments on commit 020fe5b

Please sign in to comment.