Pass configured CAMS credentials through provider assembly; separate missing optional packages from broken imports

python/siac/adapters/atmo/cams.py

@@ -1183,7 +1183,9 @@ def _download_cams_file(self, obs_time: datetime) -> Path | None:
         """Download CAMS data using the official CDS API request pattern."""
         try:
             cdsapi = self._import_cdsapi()
-        except ImportError:
+        except ImportError as exc:
+            if not isinstance(exc, ModuleNotFoundError) or exc.name != "cdsapi":
+                raise ImportError("cdsapi is installed but failed to import.") from exc
             logger.warning("cdsapi is not installed; cannot auto-download CAMS data")
             return None
 

python/siac/adapters/auth.py

@@ -95,6 +95,8 @@ def _load_optional_dependency(module_name: str, install_hint: str) -> Any:
     try:
         return import_module(module_name)
     except ModuleNotFoundError as exc:  # pragma: no cover - env-dependent
+        if exc.name != module_name:
+            raise
         raise AuthenticationError(install_hint) from exc
 
 

python/siac/adapters/data/earthaccess_source.py

@@ -58,7 +58,9 @@ def _get_module(self) -> Any:
         if self._earthaccess is None:
             try:
                 self._earthaccess = importlib.import_module("earthaccess")
-            except ImportError as exc:  # pragma: no cover - env-dependent
+            except ImportError as exc:
+                if not isinstance(exc, ModuleNotFoundError) or exc.name != "earthaccess":
+                    raise ImportError("earthaccess is installed but failed to import.") from exc
                 raise ImportError(
                     "earthaccess is required for NASA Earthdata access. "
                     "Install with `pip install earthaccess`."

python/siac/adapters/earthdata_common.py

@@ -16,7 +16,9 @@
 
 try:
     from pyhdf.SD import SD, SDC  # type: ignore[import-untyped]
-except ModuleNotFoundError:  # pragma: no cover - optional dependency
+except ImportError as exc:
+    if not isinstance(exc, ModuleNotFoundError) or exc.name != "pyhdf":
+        raise ImportError("pyhdf is installed but failed to import.") from exc
 
     class _MissingSDC:
         READ = 0

python/siac/algorithms/cloud/providers/omnicloudmask.py

@@ -81,11 +81,8 @@ def _default_predictor(
         try:
             omnicloudmask = import_module("omnicloudmask")
         except ImportError as exc:
-            # Narrowed from ``except Exception`` (REVIEW.md §2.1):
-            # only swallow import-time failures here. Other exception
-            # classes (a partial install raising AttributeError during
-            # module init, for example) should propagate so the user can
-            # see the real cause.
+            if not isinstance(exc, ModuleNotFoundError) or exc.name != "omnicloudmask":
+                raise ImportError("omnicloudmask is installed but failed to import.") from exc
             raise ImportError(
                 "omnicloudmask is required for SIAC cloud masking. "
                 "Install the 'omnicloudmask' package."

python/siac/algorithms/rt/lut/http_zip_store.py

@@ -557,6 +557,8 @@ def _build_s3_filesystem(storage_options: dict[str, Any]) -> AsyncFileSystem:
     try:
         import s3fs  # type: ignore[import-untyped]
     except ImportError as exc:
+        if not isinstance(exc, ModuleNotFoundError) or exc.name != "s3fs":
+            raise ImportError("s3fs is installed but failed to import.") from exc
         raise ImportError(
             "s3fs is required to read s3:// LUT zip paths. Install with `pip install s3fs`."
         ) from exc

python/siac/app/_assembly_providers.py

@@ -8,6 +8,7 @@
 import numpy as np
 
 from siac.adapters.atmo import CAMSProvider
+from siac.adapters.auth import CredentialManager
 from siac.adapters.earthdata import earthaccess_source_from_auth
 from siac.app.registry import (
     ATMO_PROVIDER_REGISTRY,
@@ -18,7 +19,6 @@
 if TYPE_CHECKING:
     from collections.abc import Callable
 
-    from siac.adapters.auth import CredentialManager
     from siac.algorithms.surface.brdf_monthly_composite import MonthlyCompositeCollection
     from siac.domain.protocols import AtmosphericPriorProvider, MonthlyCompositeProvider
     from siac.runtime import ObservationBundle
@@ -47,7 +47,7 @@ def _build_cams_provider(
             atmo_config.data_path,
             temporal_interp=atmo_config.temporal_interpolation == "linear",
             download_missing=atmo_config.download_missing,
-            auth=auth,
+            auth=auth if auth is not None else CredentialManager.from_config(config),
             cache_dir=atmo_config.cache_dir,
         ),
     )

python/siac/app/_assembly_surface.py

@@ -10,6 +10,7 @@
 import numpy as np
 
 from siac.adapters.atmo import CAMSProvider
+from siac.adapters.auth import CredentialManager
 from siac.algorithms.surface.brdf_whittaker import BRDFWhittakerDeriver
 from siac.algorithms.surface.kernel_model import KernelModelDeriver
 from siac.app._assembly_providers import (
@@ -20,7 +21,6 @@
 from siac.observability import current_execution_observer, observe_stage
 
 if TYPE_CHECKING:
-    from siac.adapters.auth import CredentialManager
     from siac.domain.sensors import SensorConfig
     from siac.runtime import AtmosphericState, ObservationBundle, SurfacePrior
     from siac.workflows.pipeline import SurfacePriorFn
@@ -730,7 +730,7 @@ def _resolve_bestpixel_anchor_atmo_source(
         data_path,
         temporal_interp=False,
         download_missing=True,
-        auth=auth,
+        auth=auth if auth is not None else CredentialManager.from_config(config),
         cache_dir=cache_dir,
     )
     return provider.get_prior

tests/unit/test_cams_credentials.py

@@ -0,0 +1,77 @@
+"""Configured CAMS credentials reach cdsapi through provider assembly."""
+
+from __future__ import annotations
+
+import sys
+from datetime import datetime
+from types import SimpleNamespace
+from typing import TYPE_CHECKING
+
+import pytest
+
+from siac.adapters.auth import CredentialManager
+from siac.app._assembly_providers import _build_cams_provider
+from siac.app._assembly_surface import _resolve_bestpixel_anchor_atmo_source
+from siac.config import SIACConfig
+
+if TYPE_CHECKING:
+    from pathlib import Path
+
+
+@pytest.mark.parametrize("factory", ["registered", "anchor"])
+@pytest.mark.parametrize("credentials", ["config", "manager", "defaults"])
+def test_cams_assembly_passes_credentials_to_cdsapi(
+    monkeypatch: pytest.MonkeyPatch,
+    tmp_path: Path,
+    factory: str,
+    credentials: str,
+) -> None:
+    config = SIACConfig()
+    if credentials != "defaults":
+        config.auth.cds.api_key = "configured-test-key"
+    auth = None
+    if credentials == "manager":
+        auth = CredentialManager()
+        auth.set_credentials("cds", key="manager-test-key")
+
+    captured: dict[str, object] = {}
+
+    class FakeClient:
+        def __init__(self, **kwargs):
+            captured["kwargs"] = kwargs
+
+        def retrieve(self, dataset, request):
+            return self
+
+        def download(self, path):
+            captured["path"] = path
+
+    monkeypatch.setitem(sys.modules, "cdsapi", SimpleNamespace(Client=FakeClient))
+    if factory == "registered":
+        config.providers.atmo.data_path = str(tmp_path)
+        provider = _build_cams_provider(config, auth)
+    else:
+        anchor_config = SimpleNamespace(
+            auth=config.auth,
+            paths=SimpleNamespace(cache_root=tmp_path),
+            algorithms=SimpleNamespace(
+                surface_prior=SimpleNamespace(
+                    bestpixel_predict_visible=True,
+                    bestpixel_predict_visible_anchor_source="cams",
+                    bestpixel_predict_visible_cams_data_path=str(tmp_path),
+                )
+            ),
+        )
+        prior = _resolve_bestpixel_anchor_atmo_source(anchor_config, auth)
+        provider = prior.__self__
+
+    output = provider._download_cams_file(datetime(2024, 1, 2))
+    expected = {
+        "config": {"key": "configured-test-key"},
+        "manager": {"key": "manager-test-key"},
+        "defaults": {},
+    }
+    assert captured["kwargs"] == expected[credentials]
+    assert captured["path"] == str(output)
+    if auth is not None:
+        assert provider._auth is auth

tests/unit/test_optional_import_errors.py

@@ -0,0 +1,106 @@
+"""Optional imports distinguish missing packages from broken installations."""
+
+from __future__ import annotations
+
+import builtins
+import importlib
+import runpy
+from datetime import datetime
+from typing import TYPE_CHECKING
+
+import pytest
+
+from siac.adapters import auth, earthdata_common
+from siac.adapters.atmo.cams import CAMSProvider
+from siac.adapters.data.earthaccess_source import EarthAccessSource
+from siac.algorithms.cloud.providers.omnicloudmask import OmniCloudMaskProvider
+from siac.algorithms.rt.lut.http_zip_store import _build_s3_filesystem
+from siac.errors import AuthenticationError
+
+if TYPE_CHECKING:
+    from pathlib import Path
+
+
+def _import_optional(package: str, tmp_path: Path):
+    if package == "earthaccess":
+        return EarthAccessSource()._get_module()
+    if package == "omnicloudmask":
+        return OmniCloudMaskProvider._default_predictor(None)
+    if package == "s3fs":
+        return _build_s3_filesystem({})
+    if package == "cdsapi":
+        return CAMSProvider(tmp_path)._download_cams_file(datetime(2024, 1, 2))
+    return runpy.run_path(earthdata_common.__file__)
+
+
+def _fail_import(monkeypatch: pytest.MonkeyPatch, package: str, error: ImportError) -> None:
+    original_import = builtins.__import__
+    original_import_module = importlib.import_module
+
+    def import_package(name, *args, **kwargs):
+        if name.split(".")[0] == package:
+            raise error
+        return original_import(name, *args, **kwargs)
+
+    def import_module(name, *args, **kwargs):
+        if name == package:
+            raise error
+        return original_import_module(name, *args, **kwargs)
+
+    monkeypatch.setattr(builtins, "__import__", import_package)
+    monkeypatch.setattr(importlib, "import_module", import_module)
+    monkeypatch.setattr(auth, "import_module", import_module)
+    monkeypatch.setattr("siac.adapters.atmo.cams.import_module", import_module)
+    monkeypatch.setattr(
+        "siac.algorithms.cloud.providers.omnicloudmask.import_module", import_module
+    )
+
+
+@pytest.mark.parametrize("package", ["earthaccess", "omnicloudmask", "s3fs", "cdsapi", "pyhdf"])
+@pytest.mark.parametrize("failure", ["transitive_missing", "incompatible"])
+def test_optional_installed_package_import_failure(
+    monkeypatch: pytest.MonkeyPatch, tmp_path: Path, package: str, failure: str
+) -> None:
+    error = (
+        ModuleNotFoundError("No module named dependency", name="dependency")
+        if failure == "transitive_missing"
+        else ImportError("cannot import name from dependency")
+    )
+    _fail_import(monkeypatch, package, error)
+    with pytest.raises(ImportError, match=f"{package} is installed but failed to import") as raised:
+        _import_optional(package, tmp_path)
+    assert raised.value.__cause__ is error
+
+
+@pytest.mark.parametrize("package", ["earthaccess", "omnicloudmask", "s3fs", "cdsapi", "pyhdf"])
+def test_optional_missing_package_keeps_install_hint(
+    monkeypatch: pytest.MonkeyPatch, tmp_path: Path, package: str
+) -> None:
+    error = ModuleNotFoundError(f"No module named {package}", name=package)
+    _fail_import(monkeypatch, package, error)
+    if package == "cdsapi":
+        assert _import_optional(package, tmp_path) is None
+    elif package == "pyhdf":
+        module = _import_optional(package, tmp_path)
+        with pytest.raises(ModuleNotFoundError, match="pyhdf is required"):
+            module["SD"]("unused.hdf")
+    else:
+        with pytest.raises(ImportError, match=f"{package} is required") as raised:
+            _import_optional(package, tmp_path)
+        assert raised.value.__cause__ is error
+
+
+def test_auth_optional_import_preserves_missing_dependency(monkeypatch: pytest.MonkeyPatch) -> None:
+    error = ModuleNotFoundError("No module named dependency", name="dependency")
+    _fail_import(monkeypatch, "cdsapi", error)
+    with pytest.raises(ModuleNotFoundError) as raised:
+        auth._load_optional_dependency("cdsapi", "cdsapi is not installed.")
+    assert raised.value is error
+
+
+def test_auth_optional_import_keeps_missing_package_hint(monkeypatch: pytest.MonkeyPatch) -> None:
+    error = ModuleNotFoundError("No module named cdsapi", name="cdsapi")
+    _fail_import(monkeypatch, "cdsapi", error)
+    with pytest.raises(AuthenticationError, match="cdsapi is not installed") as raised:
+        auth._load_optional_dependency("cdsapi", "cdsapi is not installed.")
+    assert raised.value.__cause__ is error