Tighten configuration settings and validation

Use a DATALINKER_ prefix for all configuration environment variables
and stop using SAFIR_ as a prefix for settings that are passed to
Safir. Make mandatory environment variables actually mandatory, and
set them to dummy values in tox to avoid issues with the test suite.
Declare URLs as HttpUrl so that Pydantic will validate them.

changelog.d/20240126_160310_rra_DM_42689.md

@@ -0,0 +1,4 @@
+### New features
+
+- Standardize using a `DATALINKER_` prefix for all environment variables used to configure datalinker.
+- Diagnose more errors in environment variable settings and fail on startup if the configuration is not valid.

src/datalinker/config.py

@@ -2,11 +2,11 @@
 
 from __future__ import annotations
 
-import os
 from enum import Enum
+from pathlib import Path
 from typing import Annotated
 
-from pydantic import Field, HttpUrl, TypeAdapter
+from pydantic import Field, HttpUrl
 from pydantic_settings import BaseSettings, SettingsConfigDict
 from safir.logging import LogLevel, Profile
 
@@ -17,14 +17,6 @@
 ]
 
 
-def _get_butler_repositories() -> dict[str, str] | None:
-    json = os.getenv("DAF_BUTLER_REPOSITORIES", None)
-    if json is not None:
-        return TypeAdapter(dict[str, str]).validate_json(json)
-
-    return None
-
-
 class StorageBackend(Enum):
     """Possible choices for a storage backend."""
 
@@ -36,47 +28,47 @@ class Config(BaseSettings):
     """Configuration for datalinker."""
 
     cutout_sync_url: Annotated[
-        str,
+        HttpUrl,
         Field(
             title="URL to SODA sync API",
             description=(
                 "URL to the sync API for the SODA service that does cutouts"
             ),
         ),
-    ] = ""
+    ]
 
-    hips_base_url: Annotated[str, Field(title="Base URL for HiPS lists")] = ""
+    hips_base_url: Annotated[HttpUrl, Field(title="Base URL for HiPS lists")]
 
     tap_metadata_dir: Annotated[
-        str,
+        Path | None,
         Field(
             title="Path to TAP YAML metadata",
             description=(
                 "Directory containing YAML metadata files about TAP schema"
             ),
         ),
-    ] = ""
+    ] = None
 
     token: Annotated[
         str,
         Field(
             title="Token for API authentication",
             description="Token to use to authenticate to the HiPS service",
         ),
-    ] = ""
+    ]
 
     storage_backend: Annotated[
         StorageBackend,
         Field(
             title="Storage backend",
             description="Which storage backend to use for uploaded files",
-            validation_alias="STORAGE_BACKEND",
         ),
     ] = StorageBackend.GCS
 
     s3_endpoint_url: Annotated[
-        str, Field(title="Storage API URL", validation_alias="S3_ENDPOINT_URL")
-    ] = "https://storage.googleapis.com"
+        HttpUrl,
+        Field(title="Storage API URL", validation_alias="S3_ENDPOINT_URL"),
+    ] = HttpUrl("https://storage.googleapis.com")
 
     # TODO(DM-42660): butler_repositories can be removed once there is a
     # release of daf_butler available that handles DAF_BUTLER_REPOSITORIES
@@ -97,14 +89,7 @@ class Config(BaseSettings):
 
     name: Annotated[
         str,
-        Field(
-            title="Application name",
-            description=(
-                "The application's name, which doubles as the root HTTP"
-                " endpoint path"
-            ),
-            validation_alias="SAFIR_NAME",
-        ),
+        Field(title="Application name"),
     ] = "datalinker"
 
     path_prefix: Annotated[
@@ -130,16 +115,12 @@ class Config(BaseSettings):
         Profile,
         Field(
             title="Application logging profile",
-            validation_alias="SAFIR_PROFILE",
         ),
-    ] = Profile.development
+    ] = Profile.production
 
     log_level: Annotated[
         LogLevel,
-        Field(
-            title="Log level of the application's logger",
-            validation_alias="SAFIR_LOG_LEVEL",
-        ),
+        Field(title="Log level of the application's logger"),
     ] = LogLevel.INFO
 
     slack_webhook: Annotated[

src/datalinker/dependencies/hips.py

@@ -58,7 +58,7 @@ async def _build_hips_list(
         """
         lists = []
         for dataset in HIPS_DATASETS:
-            url = config.hips_base_url + f"/{dataset}"
+            url = str(config.hips_base_url).rstrip("/") + f"/{dataset}"
             r = await client.get(
                 url + "/properties",
                 headers={"Authorization": f"bearer {config.token}"},

src/datalinker/dependencies/tap.py

@@ -1,6 +1,5 @@
 """Dependency that caches information about the TAP schema."""
 
-from pathlib import Path
 
 import yaml
 
@@ -49,9 +48,9 @@ def _load_data(self) -> TAPMetadata:
         """Load and cache the schema data."""
         if not config.tap_metadata_dir:
             return {}
-        columns: TAPMetadata = {}
 
-        for data_path in Path(config.tap_metadata_dir).iterdir():
+        columns: TAPMetadata = {}
+        for data_path in config.tap_metadata_dir.iterdir():
             if data_path.suffix != ".yaml":
                 continue
             with data_path.open("r") as fh:

src/datalinker/handlers/external.py

@@ -278,7 +278,7 @@ def links(
             "id": id,
             "image_url": image_url,
             "image_size": image_uri.size(),
-            "cutout_sync_url": config.cutout_sync_url,
+            "cutout_sync_url": str(config.cutout_sync_url),
         },
         media_type="application/x-votable+xml",
     )
@@ -332,7 +332,7 @@ def _upload_to_s3(image_uri: str, expiry: timedelta) -> str:
     key = image_uri_parts.path[1:]
 
     s3_client = client(
-        "s3", endpoint_url=config.s3_endpoint_url, region_name="us-east-1"
+        "s3", endpoint_url=str(config.s3_endpoint_url), region_name="us-east-1"
     )
 
     return s3_client.generate_presigned_url(

tests/conftest.py

@@ -14,6 +14,7 @@
 from fastapi import FastAPI
 from httpx import AsyncClient
 from moto import mock_s3
+from pydantic import HttpUrl
 from safir.testing.gcs import MockStorageClient, patch_google_storage
 
 from datalinker import main
@@ -23,27 +24,29 @@
 
 
 @pytest_asyncio.fixture
-async def app() -> AsyncIterator[FastAPI]:
+async def app(monkeypatch: MonkeyPatch) -> AsyncIterator[FastAPI]:
     """Return a configured test application.
 
     Wraps the application in a lifespan manager so that startup and shutdown
     events are sent during test execution.
     """
-    config.tap_metadata_dir = str(Path(__file__).parent / "data")
+    monkeypatch.setattr(
+        config, "tap_metadata_dir", Path(__file__).parent / "data"
+    )
     async with LifespanManager(main.app):
         yield main.app
-    config.tap_metadata_dir = ""
 
 
 @pytest_asyncio.fixture
 async def client(app: FastAPI) -> AsyncIterator[AsyncClient]:
-    """Return an ``httpx.AsyncClient`` configured to talk to the test app."""
+    """Return an ``httpx.AsyncClient`` configured to talk to the test app.
+
+    Mock the Gafaelfawr delegated token header, needed by endpoints that use
+    Butler.
+    """
+    headers = {"X-Auth-Request-Token": "sometoken"}
     async with AsyncClient(
-        app=app,
-        base_url="https://example.com/",
-        # Mock the Gafaelfawr delegated token header, needed by endpoints that
-        # use Butler
-        headers={"x-auth-request-token": "sometoken"},
+        app=app, base_url="https://example.com/", headers=headers
     ) as client:
         yield client
 
@@ -64,11 +67,13 @@ def s3(monkeypatch: MonkeyPatch) -> Iterator[boto3.client]:
     monkeypatch.setenv("AWS_SESSION_TOKEN", "testing")
     monkeypatch.setattr(config, "storage_backend", StorageBackend.S3)
     monkeypatch.setattr(
-        config, "s3_endpoint_url", "https://s3.amazonaws.com/bucket"
+        config, "s3_endpoint_url", HttpUrl("https://s3.amazonaws.com/bucket")
     )
     with mock_s3():
         yield boto3.client(
-            "s3", endpoint_url=config.s3_endpoint_url, region_name="us-east-1"
+            "s3",
+            endpoint_url=str(config.s3_endpoint_url),
+            region_name="us-east-1",
         )
 
 
@@ -79,7 +84,7 @@ def mock_google_storage(
     """Mock out the Google Cloud Storage API."""
     monkeypatch.setattr(config, "storage_backend", StorageBackend.GCS)
     monkeypatch.setattr(
-        config, "s3_endpoint_url", "https://storage.googleapis.com"
+        config, "s3_endpoint_url", HttpUrl("https://storage.googleapis.com")
     )
     yield from patch_google_storage(
         expected_expiration=timedelta(hours=1),

tests/handlers/hips_test.py

@@ -7,15 +7,17 @@
 
 import pytest
 import respx
+from _pytest.monkeypatch import MonkeyPatch
 from httpx import AsyncClient, Response
+from pydantic import HttpUrl
 
 from datalinker.config import config
 from datalinker.constants import HIPS_DATASETS
 
 
 @pytest.mark.asyncio
 async def test_hips_list(
-    client: AsyncClient, respx_mock: respx.Router
+    client: AsyncClient, respx_mock: respx.Router, monkeypatch: MonkeyPatch
 ) -> None:
     hips_list_template = (
         Path(__file__).parent.parent / "data" / "hips-properties"
@@ -34,11 +36,10 @@ async def test_hips_list(
         )
         hips_lists.append(hips_list)
 
-    try:
-        config.hips_base_url = "https://hips.example.com"
-        r = await client.get("/api/hips/list")
-        assert r.status_code == 200
-        assert r.headers["Content-Type"].startswith("text/plain")
-        assert r.text == "\n".join(hips_lists)
-    finally:
-        config.hips_base_url = ""
+    monkeypatch.setattr(
+        config, "hips_base_url", HttpUrl("https://hips.example.com")
+    )
+    r = await client.get("/api/hips/list")
+    assert r.status_code == 200
+    assert r.headers["Content-Type"].startswith("text/plain")
+    assert r.text == "\n".join(hips_lists)

tox.ini

@@ -9,6 +9,10 @@ deps =
     -r{toxinidir}/requirements/dev.txt
 commands =
     pytest -vvv --cov=datalinker --cov-branch --cov-report= {posargs}
+setenv =
+    DATALINKER_CUTOUT_SYNC_URL = https://example.com/api/cutout
+    DATALINKER_HIPS_BASE_URL = https://example.com/api/hips
+    DATALINKER_TOKEN = some-gafaelfawr-token
 
 [testenv:coverage-report]
 description = Compile coverage from each test run.