Merge pull request #188 from lsst-sqre/tickets/DM-51412

DM-51412: Add support for returning hips lists for multiple data releases.

Author: stvoutsin

.pre-commit-config.yaml

@@ -8,12 +8,12 @@ repos:
       - id: trailing-whitespace
 
   - repo: https://github.com/astral-sh/uv-pre-commit
-    rev: 0.6.14
+    rev: 0.7.13
     hooks:
       - id: uv-lock
 
   - repo: https://github.com/astral-sh/ruff-pre-commit
-    rev: v0.11.5
+    rev: v0.11.13
     hooks:
       - id: ruff
         args: [--fix, --exit-non-zero-on-fix]

Dockerfile

@@ -12,7 +12,7 @@
 #   - Runs a non-root user.
 #   - Sets up the entrypoint and port.
 
-FROM python:3.13.3-slim-bookworm AS base-image
+FROM python:3.13.4-slim-bookworm AS base-image
 
 # Update system packages
 COPY scripts/install-base-packages.sh .

changelog.d/20250618_164915_steliosvoutsinas_DM_51412.md

@@ -0,0 +1,11 @@
+<!-- Delete the sections that don't apply -->
+
+### New features
+
+- Change Config to load configuration from local file, to support more complex structures for the hips release data
+- Introduce v2 hips router. Add an additional hips dependency that builds a hips list per data release, given the data release lists and their paths via configuration
+- Add new tests for v2 and legacy hips list endpoint
+
+### Other changes
+
+- Upgraded dependencies and python version

pyproject.toml

@@ -23,7 +23,7 @@ dependencies = [
     "httpx",
     "jinja2",
     "lsst-daf-butler[remote]",
-    "pydantic>2",
+    "pydantic>=2.11",
     "pydantic-settings",
     "pyyaml",
     "safir>=6.2.0",

src/datalinker/config.py

@@ -4,25 +4,38 @@
 
 from datetime import timedelta
 from pathlib import Path
-from typing import Annotated
+from typing import Annotated, Self
 
-from pydantic import Field, HttpUrl, SecretStr
+import yaml
+from pydantic import BaseModel, Field, HttpUrl, model_validator
 from pydantic_settings import BaseSettings, SettingsConfigDict
 from safir.logging import LogLevel, Profile
 from safir.pydantic import HumanTimedelta
 
-__all__ = [
-    "Config",
-    "config",
-]
+__all__ = ["Config", "HiPSDatasetConfig"]
+
+
+class HiPSDatasetConfig(BaseModel):
+    """Configuration for a single HiPS dataset."""
+
+    url: Annotated[
+        HttpUrl,
+        Field(title="Base URL", description="Base URL for this HiPS dataset"),
+    ]
+
+    paths: Annotated[
+        list[str],
+        Field(
+            title="HiPS paths",
+            description="List of available HiPS paths",
+        ),
+    ]
 
 
 class Config(BaseSettings):
     """Configuration for datalinker."""
 
-    model_config = SettingsConfigDict(
-        env_prefix="DATALINKER_", case_sensitive=False
-    )
+    model_config = SettingsConfigDict(extra="forbid", populate_by_name=True)
 
     cutout_sync_url: Annotated[
         HttpUrl,
@@ -31,30 +44,65 @@ class Config(BaseSettings):
             description=(
                 "URL to the sync API for the SODA service that does cutouts"
             ),
+            validation_alias="cutoutSyncUrl",
         ),
     ]
 
-    hips_base_url: Annotated[HttpUrl, Field(title="Base URL for HiPS lists")]
+    hips_base_url: Annotated[
+        HttpUrl,
+        Field(title="Base URL for HiPS lists", validation_alias="hipsBaseUrl"),
+    ]
+
+    hips_datasets: Annotated[
+        dict[str, HiPSDatasetConfig],
+        Field(
+            title="HiPS dataset configurations",
+            description=(
+                "Mapping of dataset names to their configuration. "
+                "Each dataset has a base URL and list of available HiPS paths."
+            ),
+            validation_alias="hipsDatasets",
+        ),
+    ] = {}
+
+    hips_default_dataset: Annotated[
+        str, Field(validation_alias="hipsDefaultDataset")
+    ] = ""
+    """The dataset to serve from v1 routes. Must be a key in hips_datasets"""
 
     hips_path_prefix: Annotated[
         str,
         Field(
             title="URL prefix for HiPS API",
             description="URL prefix used to inject the HiPS list file",
+            validation_alias="hipsPathPrefix",
         ),
     ] = "/api/hips"
 
+    hips_v2_path_prefix: Annotated[
+        str,
+        Field(
+            title="URL prefix for HiPS API",
+            description="URL prefix used to inject the HiPS list file",
+            validation_alias="hipsV2PathPrefix",
+        ),
+    ] = "/api/hips/v2"
+
     links_lifetime: Annotated[
         HumanTimedelta,
         Field(
             title="Lifetime of image links replies",
             description="Should match the lifetime of signed URLs from Butler",
+            validation_alias="linksLifetime",
         ),
     ] = timedelta(hours=1)
 
     log_level: Annotated[
         LogLevel,
-        Field(title="Log level of the application's logger"),
+        Field(
+            title="Log level of the application's logger",
+            validation_alias="logLevel",
+        ),
     ] = LogLevel.INFO
 
     name: Annotated[str, Field(title="Application name")] = "datalinker"
@@ -67,6 +115,7 @@ class Config(BaseSettings):
                 "This URL prefix is used for the IVOA DataLink API and for"
                 " any other helper APIs exposed via DataLink descriptors"
             ),
+            validation_alias="pathPrefix",
         ),
     ] = "/api/datalink"
 
@@ -75,28 +124,100 @@ class Config(BaseSettings):
         Field(title="Application logging profile"),
     ] = Profile.production
 
+    tap_metadata_url: Annotated[
+        Path | None,
+        Field(
+            title="URL to TAP schema metadata",
+            description=(
+                "URL containing TAP schema metadata used to construct queries"
+            ),
+            validation_alias="tapMetadataUrl",
+        ),
+    ] = None
+
     tap_metadata_dir: Annotated[
         Path | None,
         Field(
             title="Path to TAP YAML metadata",
             description=(
                 "Directory containing YAML metadata files about TAP schema"
             ),
+            validation_alias="tapMetadataDir",
         ),
     ] = None
 
-    slack_webhook: Annotated[
-        SecretStr | None, Field(title="Slack webhook for exception reporting")
-    ] = None
+    slack_alerts: bool = Field(
+        False,
+        title="Enable Slack alerts",
+        description=(
+            "Whether to enable Slack alerts. If true, ``slack_webhook`` must"
+            " also be set."
+        ),
+        validation_alias="slackAlerts",
+    )
 
-    token: Annotated[
-        str,
-        Field(
-            title="Token for API authentication",
-            description="Token to use to authenticate to the HiPS service",
+    slack_webhook: str | None = Field(
+        None,
+        title="Slack webhook for alerts",
+        description=(
+            "If set, failures creating user labs or file servers and any"
+            " uncaught exceptions in the Nublado controller will be"
+            " reported to Slack via this webhook"
         ),
-    ]
+        validation_alias="DATALINKER_SLACK_WEBHOOK",
+    )
 
+    token: str = Field(
+        title="Token for API authentication",
+        description="Token to use to authenticate to the HiPS service",
+        validation_alias="DATALINKER_TOKEN",
+    )
 
-config = Config()
-"""Configuration for datalinker."""
+    def has_hips_datasets(self) -> bool:
+        """Check if any HiPS datasets are configured."""
+        return bool(self.hips_datasets)
+
+    def get_default_hips_dataset(self) -> HiPSDatasetConfig:
+        """Return the HiPS dataset config for the default dataset.
+
+        Returns
+        -------
+        HiPSDatasetConfig | None
+            The default dataset configuration, or None if not configured.
+        """
+        return self.hips_datasets[self.hips_default_dataset]
+
+    @model_validator(mode="after")
+    def validate_default_hips_dataset(self) -> Self:
+        """Validate that the default HiPS dataset exists if specified."""
+        if self.hips_default_dataset:
+            if not self.hips_datasets:
+                msg = (
+                    f"HiPS dataset key {self.hips_default_dataset} specified "
+                    "but no datasets are configured in hips_datasets"
+                )
+                raise ValueError(msg)
+            if self.hips_default_dataset not in self.hips_datasets:
+                msg = (
+                    f"HiPS dataset key {self.hips_default_dataset} not found. "
+                    f"Available datasets: {list(self.hips_datasets.keys())}"
+                )
+                raise ValueError(msg)
+        return self
+
+    @classmethod
+    def from_file(cls, path: Path) -> Self:
+        """Construct a Configuration object from a configuration file.
+
+        Parameters
+        ----------
+        path
+            Path to the configuration file in YAML.
+
+        Returns
+        -------
+        Config
+            The corresponding `Configuration` object.
+        """
+        with path.open("r") as f:
+            return cls.model_validate(yaml.safe_load(f))

src/datalinker/constants.py

@@ -1,5 +1,15 @@
 """Constants for datalinker."""
 
+from pathlib import Path
+
+__all__ = ["CONFIG_PATH", "CONFIG_PATH_ENV_VAR"]
+
+CONFIG_PATH = Path("/etc/datalinker/config.yaml")
+"""Default path to configuration."""
+
+CONFIG_PATH_ENV_VAR = "DATALINKER_CONFIG_PATH"
+"""Env var to load config path from."""
+
 ADQL_COMPOUND_TABLE_REGEX = r"^([a-zA-Z0-9_]+\.)?[a-zA-Z0-9_.]+$"
 """ADQL table with optional prefix."""
 
@@ -8,16 +18,3 @@
 
 ADQL_IDENTIFIER_REGEX = r"^[a-zA-Z0-9_]+$"
 """ADQL table (without prefix)."""
-
-HIPS_DATASETS = (
-    "images/color_gri",
-    "images/color_riz",
-    "images/band_u",
-    "images/band_g",
-    "images/band_r",
-    "images/band_i",
-    "images/band_z",
-    "images/band_y",
-    "images/2MASS/Color",
-)
-"""HiPS data sets to include in the HiPS list."""

src/datalinker/dependencies/config.py

@@ -0,0 +1,65 @@
+"""Config dependency for FastAPI.
+
+Stolen from Gafaelfawr & Crawlspace.
+"""
+
+from __future__ import annotations
+
+import os
+from pathlib import Path
+
+from ..config import Config
+from ..constants import CONFIG_PATH, CONFIG_PATH_ENV_VAR
+
+__all__ = ["ConfigDependency", "config_dependency"]
+
+
+class ConfigDependency:
+    """Provides the configuration as a dependency.
+
+    We want a production deployment to default to one configuration path, but
+    allow that path to be overridden by the test suite and, if the path
+    changes, to reload the configuration (which allows sharing the same set of
+    global singletons across multiple tests).  Do this by loading the config
+    dynamically when it's first requested and reloading it whenever the
+    configuration path is changed.
+    """
+
+    def __init__(self) -> None:
+        config_path = os.getenv(CONFIG_PATH_ENV_VAR, CONFIG_PATH)
+        self._config_path = Path(config_path)
+        self._config: Config | None = None
+
+    async def __call__(self) -> Config:
+        """Load the configuration if necessary and return it."""
+        return self.config()
+
+    @property
+    def config_path(self) -> Path:
+        """Path to the configuration file."""
+        return self._config_path
+
+    def config(self) -> Config:
+        """Load the configuration if necessary and return it.
+
+        This is equivalent to using the dependency as a callable except that
+        it's not async and can therefore be used from non-async functions.
+        """
+        if not self._config:
+            self._config = Config.from_file(self._config_path)
+        return self._config
+
+    def set_config_path(self, path: Path) -> None:
+        """Change the configuration path and reload the config.
+
+        Parameters
+        ----------
+        path
+            The new configuration path.
+        """
+        self._config_path = path
+        self._config = Config.from_file(path)
+
+
+config_dependency = ConfigDependency()
+"""The dependency that will return the current configuration."""