From c5e2cca0086398b080dfae5868cd12773a1728b4 Mon Sep 17 00:00:00 2001 From: MilagrosMarin Date: Thu, 3 Sep 2026 15:42:06 +0200 Subject: [PATCH 1/2] fix: resolve stores from the connection config, not global dj.config encode and decode called _build_path and _get_backend without config=, so both fell back to the module-level dj.config. In a process serving many users each connection carries its own store credentials on dj.Instance.config, while the global config holds only what the image was built with. On a pod with no ambient AWS credentials that surfaced as DataJointError: Missing S3 configuration: access_key, secret_key on every figpack column, because the keyless spec satisfies Config.get_store_spec (key existence) and is only rejected later by StorageBackend._validate_spec (truthiness). encode had the same omission on _build_path, which reads schema_prefix off the store spec and so wrote under the wrong prefix. Matches the built-in object and npy codecs, which thread config through both helpers. Callers that pass no _config are unaffected and keep resolving against the global store. --- src/dj_figpack_codecs/codec.py | 21 ++++- tests/test_config_threading.py | 135 +++++++++++++++++++++++++++++++++ 2 files changed, 152 insertions(+), 4 deletions(-) create mode 100644 tests/test_config_threading.py diff --git a/src/dj_figpack_codecs/codec.py b/src/dj_figpack_codecs/codec.py index 15b1bbe..395f4a1 100644 --- a/src/dj_figpack_codecs/codec.py +++ b/src/dj_figpack_codecs/codec.py @@ -158,10 +158,21 @@ def encode( # Extract context using inherited helper schema, table, field, primary_key = self._extract_context(key) + # The connection's own config. Both helpers below fall back to global + # dj.config when this is None, which in a process serving many users is + # someone else's store — or, on a pod with no ambient credentials, none. + config = (key or {}).get("_config") + # Build schema-addressed storage path (folder, so no extension in path building) # We'll append .zarr to make it clear it's a Zarr folder path, token = self._build_path( - schema, table, field, primary_key, ext=".zarr", store_name=store_name + schema, + table, + field, + primary_key, + ext=".zarr", + store_name=store_name, + config=config, ) # Extract metadata before saving @@ -178,7 +189,7 @@ def encode( # FigpackRef.serve_under(), so the store never duplicates viewer code. value.save(str(bundle_path), title=title, description=description) - backend = self._get_backend(store_name) + backend = self._get_backend(store_name, config=config) backend.put_folder(str(bundle_path / "data.zarr"), path) # Return metadata @@ -198,12 +209,14 @@ def decode(self, stored: dict, *, key: dict | None = None) -> FigpackRef: stored : dict JSON metadata from database. key : dict, optional - Primary key values (unused). + Context dict. Only ``_config`` is read — the connection's config, + which carries that user's store credentials. Returns ------- FigpackRef Lazy reference with metadata access and display methods. """ - backend = self._get_backend(stored.get("store")) + config = (key or {}).get("_config") + backend = self._get_backend(stored.get("store"), config=config) return FigpackRef(stored, backend) diff --git a/tests/test_config_threading.py b/tests/test_config_threading.py new file mode 100644 index 0000000..c521943 --- /dev/null +++ b/tests/test_config_threading.py @@ -0,0 +1,135 @@ +# Copyright 2026 DataJoint Inc. +# SPDX-License-Identifier: Apache-2.0 + +"""Store resolution must follow the caller's config, not global ``dj.config``. + +One dashboard process serves many users. Each connection carries its own +credentials on ``dj.Instance.config``, while global ``dj.config`` holds only +whatever the image was built with. The fetch path hands the connection's config +to every codec as ``key["_config"]``; a codec that ignores it reads the wrong +store, and on a pod with no ambient AWS credentials it raises instead. +""" + +import datajoint as dj +import pytest +from datajoint.settings import Config + +from dj_figpack_codecs import FigpackCodec + +#: What a dashboard pod's global config holds when no AWS keys are in the +#: environment. ``get_store_spec`` accepts it because every required key is +#: present; ``StorageBackend`` then rejects it because they are falsy. +KEYLESS_GLOBAL_S3 = { + "protocol": "s3", + "endpoint": "s3.us-east-2.amazonaws.com", + "bucket": "from-global-config", + "location": "global/outbox", + "access_key": None, + "secret_key": None, + "schema_prefix": "prefix_from_global", +} + +#: What works-api hands back for the signed-in user, landing on the Instance. +CONNECTION_S3 = { + "protocol": "s3", + "endpoint": "s3.us-east-2.amazonaws.com", + "bucket": "from-connection-config", + "location": "connection/outbox", + "access_key": "AKIAEXAMPLEEXAMPLE12", + "secret_key": "s" * 40, +} + +STORED = { + "path": "prefix_from_global/s/t/id=1/visualization_abcd1234.zarr", + "store": "general", + "title": "Test Visualization", + "description": "A test plot", +} + + +def _connection_config(store_spec): + """A connection-scoped Config, as ``dj.Instance`` builds for one user.""" + cfg = Config() + cfg["stores"] = {"general": dict(store_spec)} + return cfg + + +@pytest.fixture +def keyless_global_store(): + """Global ``dj.config`` carrying the credential-less spec.""" + original = dict(dj.config.get("stores") or {}) + dj.config["stores"] = {"general": dict(KEYLESS_GLOBAL_S3)} + yield + dj.config["stores"] = original + + +@pytest.fixture +def global_store(request): + """Global ``dj.config`` carrying a usable spec (the ambient / worker case).""" + original = dict(dj.config.get("stores") or {}) + dj.config["stores"] = {"general": dict(request.param)} + yield + dj.config["stores"] = original + + +def test_decode_resolves_store_from_connection_config(keyless_global_store): + """decode must read the store off ``key["_config"]``.""" + ref = FigpackCodec().decode(STORED, key={"_config": _connection_config(CONNECTION_S3)}) + + assert ref._backend.spec["bucket"] == "from-connection-config" + assert ref._backend.spec["access_key"] == CONNECTION_S3["access_key"] + + +@pytest.mark.parametrize( + "global_store", + [{**KEYLESS_GLOBAL_S3, "access_key": "AKIAGLOBALGLOBAL1234", "secret_key": "g" * 40}], + indirect=True, +) +def test_decode_without_config_still_uses_global(global_store): + """No ``_config`` — an ambient caller such as a worker — keeps the global store.""" + ref = FigpackCodec().decode(STORED, key=None) + + assert ref._backend.spec["bucket"] == "from-global-config" + + +def test_encode_writes_through_connection_config( + keyless_global_store, sample_figpack_view, sample_context, temp_store +): + """encode must resolve its backend from ``_config``, not the global store.""" + cfg = _connection_config( + { + "protocol": "file", + "location": str(temp_store), + "schema_prefix": "prefix_from_connection", + } + ) + + metadata = FigpackCodec().encode( + sample_figpack_view, key={**sample_context, "_config": cfg}, store_name="general" + ) + + assert (temp_store / metadata["path"] / ".zmetadata").exists() + + +def test_encode_builds_path_from_connection_config( + keyless_global_store, sample_figpack_view, sample_context, temp_store, mock_backend, mocker +): + """``_build_path`` reads ``schema_prefix`` off the store spec, so resolving it + against the global config writes under the wrong prefix — silently, since the + keyless spec still satisfies ``get_store_spec``. Backend resolution is mocked + out here so this stays red even if only ``_get_backend`` is threaded.""" + cfg = _connection_config( + { + "protocol": "file", + "location": str(temp_store), + "schema_prefix": "prefix_from_connection", + } + ) + codec = FigpackCodec() + mocker.patch.object(codec, "_get_backend", return_value=mock_backend) + + metadata = codec.encode( + sample_figpack_view, key={**sample_context, "_config": cfg}, store_name="general" + ) + + assert metadata["path"].startswith("prefix_from_connection/") From b2764b17eedfeda38ed71d027b1874e5610d160c Mon Sep 17 00:00:00 2001 From: MilagrosMarin Date: Thu, 3 Sep 2026 16:52:52 +0200 Subject: [PATCH 2/2] chore: drop the second version declaration in __init__ MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit pyproject sets the version from the git tag via hatch-vcs and writes _version.py at build time, which the package imports. The literal above that import was dead — the import overwrites it — so it only ever drifted, and it had to be hand-synced on each release. __all__ also listed __version__ twice. --- src/dj_figpack_codecs/__init__.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/dj_figpack_codecs/__init__.py b/src/dj_figpack_codecs/__init__.py index 587e2c8..61119da 100644 --- a/src/dj_figpack_codecs/__init__.py +++ b/src/dj_figpack_codecs/__init__.py @@ -37,10 +37,8 @@ def make(self, key): url = ref.serve_under("assets/serve") # Materialize a servable viewer bundle """ -__version__ = "0.2.0" - from .codec import FigpackCodec from .ref import FigpackRef from ._version import version as __version__ -__all__ = ["__version__", "FigpackCodec", "FigpackRef", "__version__"] +__all__ = ["__version__", "FigpackCodec", "FigpackRef"]