Merge pull request #86 from lsst/tickets/DM-48656

DM-48656: Fix ApdbReplica.from_uri to understand new YAML config format

Author: JeremyMcCormick

.pre-commit-config.yaml

@@ -8,7 +8,7 @@ repos:
       - id: end-of-file-fixer
       - id: trailing-whitespace
   - repo: https://github.com/psf/black-pre-commit-mirror
-    rev: 24.10.0
+    rev: 25.1.0
     hooks:
       - id: black
         # It is recommended to specify the latest version of Python
@@ -17,7 +17,7 @@ repos:
         # https://pre-commit.com/#top_level-default_language_version
         language_version: python3.11
   - repo: https://github.com/pycqa/isort
-    rev: 5.13.2
+    rev: 6.0.0
     hooks:
       - id: isort
         name: isort (python)

python/lsst/dax/apdb/apdb.py

@@ -32,7 +32,6 @@
 from lsst.resources import ResourcePathExpression
 from lsst.sphgeom import Region
 
-from .apdbIndex import ApdbIndex
 from .apdbSchema import ApdbTables
 from .config import ApdbConfig
 from .factory import make_apdb
@@ -82,22 +81,6 @@ def from_uri(cls, uri: ResourcePathExpression) -> Apdb:
             Instance of `Apdb` class, the type of the returned instance is
             determined by configuration.
         """
-        if isinstance(uri, str) and uri.startswith("label:"):
-            tag, _, label = uri.partition(":")
-            index = ApdbIndex()
-            # Try to find YAML format first, and pex_config if YAML is not
-            # found. During transitional period we support conversion of
-            # pex_config format to a new pydantic format.
-            try:
-                uri = index.get_apdb_uri(label, "yaml")
-            except ValueError as yaml_exc:
-                try:
-                    uri = index.get_apdb_uri(label, "pex_config")
-                except ValueError:
-                    # If none is found then re-raise exception from yaml
-                    # attempt, but add a note that pex_config is missing too.
-                    yaml_exc.add_note(f"Legacy label {label}/pex_config is also missing.")
-                    raise yaml_exc from None
         config = ApdbConfig.from_uri(uri)
         return make_apdb(config)
 

python/lsst/dax/apdb/apdbReplica.py

@@ -27,14 +27,12 @@
 from abc import ABC, abstractmethod
 from collections.abc import Collection, Iterable, Sequence
 from dataclasses import dataclass
-from typing import TYPE_CHECKING, cast
+from typing import TYPE_CHECKING
 
 import astropy.time
-from lsst.pex.config import Config
-from lsst.resources import ResourcePath, ResourcePathExpression
+from lsst.resources import ResourcePathExpression
 
 from .apdb import ApdbConfig
-from .apdbIndex import ApdbIndex
 from .factory import make_apdb_replica
 
 if TYPE_CHECKING:
@@ -146,16 +144,7 @@ def from_uri(cls, uri: ResourcePathExpression) -> ApdbReplica:
             Instance of `ApdbReplica` class, the type of the returned instance
             is determined by configuration.
         """
-        if isinstance(uri, str) and uri.startswith("label:"):
-            tag, _, label = uri.partition(":")
-            index = ApdbIndex()
-            # Current format for config files is "pex_config"
-            format = "pex_config"
-            uri = index.get_apdb_uri(label, format)
-        path = ResourcePath(uri)
-        config_str = path.read().decode()
-        # Assume that this is ApdbConfig, make_apdb will raise if not.
-        config = cast(ApdbConfig, Config._fromPython(config_str))
+        config = ApdbConfig.from_uri(uri)
         return make_apdb_replica(config)
 
     @classmethod

python/lsst/dax/apdb/config.py

@@ -32,6 +32,7 @@
 from lsst.resources import ResourcePath, ResourcePathExpression
 from pydantic import BaseModel, Field
 
+from .apdbIndex import ApdbIndex
 from .factory import config_type_for_name
 
 
@@ -97,6 +98,23 @@ def from_uri(cls, uri: ResourcePathExpression) -> ApdbConfig:
         config : `ApdbConfig`
             Apdb configuration object.
         """
+        if isinstance(uri, str) and uri.startswith("label:"):
+            tag, _, label = uri.partition(":")
+            index = ApdbIndex()
+            # Try to find YAML format first, and pex_config if YAML is not
+            # found. During transitional period we support conversion of
+            # pex_config format to a new pydantic format.
+            try:
+                uri = index.get_apdb_uri(label, "yaml")
+            except ValueError as yaml_exc:
+                try:
+                    uri = index.get_apdb_uri(label, "pex_config")
+                except ValueError:
+                    # If none is found then re-raise exception from yaml
+                    # attempt, but add a note that pex_config is missing too.
+                    yaml_exc.add_note(f"Legacy label {label}/pex_config is also missing.")
+                    raise yaml_exc from None
+
         path = ResourcePath(uri)
         config_bytes = path.read()
 

python/lsst/dax/apdb/sql/apdbSql.py

@@ -19,8 +19,7 @@
 # You should have received a copy of the GNU General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
-"""Module defining Apdb class and related methods.
-"""
+"""Module defining Apdb class and related methods."""
 
 from __future__ import annotations
 

python/lsst/dax/apdb/sql/apdbSqlReplica.py

@@ -19,8 +19,7 @@
 # You should have received a copy of the GNU General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
-"""Module defining Apdb class and related methods.
-"""
+"""Module defining Apdb class and related methods."""
 
 from __future__ import annotations
 

python/lsst/dax/apdb/sql/apdbSqlSchema.py

@@ -19,8 +19,7 @@
 # You should have received a copy of the GNU General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
-"""Module responsible for APDB schema operations.
-"""
+"""Module responsible for APDB schema operations."""
 
 from __future__ import annotations
 

tests/test_apdbSql.py

@@ -19,8 +19,7 @@
 # You should have received a copy of the GNU General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
-"""Unit test for Apdb class.
-"""
+"""Unit test for Apdb class."""
 
 import gc
 import os
@@ -32,7 +31,7 @@
 
 import lsst.utils.tests
 import sqlalchemy
-from lsst.dax.apdb import Apdb, ApdbConfig, ApdbTables
+from lsst.dax.apdb import Apdb, ApdbConfig, ApdbReplica, ApdbTables
 from lsst.dax.apdb.pixelization import Pixelization
 from lsst.dax.apdb.sql import ApdbSql, ApdbSqlConfig
 from lsst.dax.apdb.tests import ApdbSchemaUpdateTest, ApdbTest
@@ -264,6 +263,12 @@ def test_remove_database_file(self) -> None:
         with self.assertRaisesRegex(sqlalchemy.exc.OperationalError, "unable to open database file"):
             Apdb.from_uri(self.config_path)
 
+    def test_make_apdb_replica(self) -> None:
+        """Check that we can make ApdbReplica instance from config URI."""
+        ApdbReplica.from_uri(self.config_path)
+        with self.assertRaises(FileNotFoundError):
+            Apdb.from_uri(self.bad_config_path)
+
 
 class MyMemoryTestCase(lsst.utils.tests.MemoryTestCase):
     """Run file leak tests."""

tests/test_apdbSqlSchema.py

@@ -19,8 +19,7 @@
 # You should have received a copy of the GNU General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
-"""Unit test for ApdbSqlSchema class.
-"""
+"""Unit test for ApdbSqlSchema class."""
 
 import os
 import unittest