From f3bc90e4f7077fe6c54d1993bf589a1a996199f1 Mon Sep 17 00:00:00 2001 From: Dimitri Yatsenko Date: Thu, 8 Oct 2026 21:16:55 -0500 Subject: [PATCH 1/4] Resolve an empty store name to the default store The bare-@ forms (, , and the other store codecs) keep the store name as "" so a codec can tell "store requested" from "no store". Config.get_store_spec treated only None as the default, so inserting, fetching, or collecting such a column raised "Storage '' is requested but not configured in stores". An empty name now resolves to stores.default, or to stores.filepath_default for filepath references. The garbage collector compared each reference's recorded store name with the store it collects. Bare-@ references record "", so a collector bound to the default store by name counted their files as orphans and, with dry_run=False, deleted files that existing rows still reference. A reference with an empty or missing store name now matches the default store. --- src/datajoint/gc.py | 5 +++- src/datajoint/settings.py | 6 +++-- tests/integration/test_gc.py | 46 ++++++++++++++++++++++++++++++++++++ tests/unit/test_settings.py | 28 ++++++++++++++++++++++ 4 files changed, 82 insertions(+), 3 deletions(-) diff --git a/src/datajoint/gc.py b/src/datajoint/gc.py index 68820ab1e..c6836e13d 100644 --- a/src/datajoint/gc.py +++ b/src/datajoint/gc.py @@ -115,6 +115,9 @@ def __init__(self, *schemas: "Schema", store: str | None = None, config=None) -> self.schemas = schemas self.store = store self.config = config if config is not None else schemas[0].connection._config + # References written with the bare ``@`` form record an empty store name + # (older metadata may record none); both mean the default store. + self._default_store = self.config.stores.get("default") # Resolve the store eagerly: validates it exists and pins the backend # and section prefixes for this collector's lifetime (one store only). self.backend = get_store_backend(store, config=self.config) @@ -174,7 +177,7 @@ def _references(self, heading_attr: str, verbose: bool) -> set[str]: try: for row in table.proj(attr_name).cursor(as_dict=True): for path, ref_store in attr.codec.referenced_paths(row[attr_name]): - if self.store is None or ref_store == self.store: + if self.store is None or (ref_store or self._default_store) == self.store: referenced.add(path) except Exception as e: logger.warning(f"Error scanning {table_name}.{attr_name}: {e}") diff --git a/src/datajoint/settings.py b/src/datajoint/settings.py index 9a185f821..454b8e0f4 100644 --- a/src/datajoint/settings.py +++ b/src/datajoint/settings.py @@ -470,8 +470,10 @@ def get_store_spec(self, store: str | None = None, *, use_filepath_default: bool DataJointError If store is not configured or has invalid config. """ - # Handle default store - if store is None: + # Handle default store. An empty name is the bare ``@`` form (````, + # ````, ...), which the declaration keeps as "" so the codec can + # tell "store requested" from "no store"; it means the default store. + if not store: if use_filepath_default: # Filepath references use separate default (not part of OAS) if "filepath_default" not in self.stores: diff --git a/tests/integration/test_gc.py b/tests/integration/test_gc.py index a9bd5a4a5..af4f85fcc 100644 --- a/tests/integration/test_gc.py +++ b/tests/integration/test_gc.py @@ -402,6 +402,52 @@ def test_custom_codec_survives_collect(self, schema_custom): assert not (deleted_paths & stored_after), f"deleted row's orphan not reclaimed: {deleted_paths & stored_after}" +class GcBareNpyTest(dj.Manual): + definition = """ + rid : int32 + --- + waveform : + """ + + +class TestBareAtUsesDefaultStore: + """```` with no store name writes to, reads from, and is collected as stores.default.""" + + @pytest.fixture + def schema_bare(self, connection_test, prefix, mock_stores): + dj.config.stores["default"] = "local" + schema = dj.Schema( + f"{prefix}_test_gc_bare_at", + context={"GcBareNpyTest": GcBareNpyTest}, + connection=connection_test, + ) + schema(GcBareNpyTest) + yield schema + schema.drop() + + def test_roundtrip(self, schema_bare): + waveform = np.arange(64, dtype="float32") + GcBareNpyTest.insert1({"rid": 1, "waveform": waveform}) + + assert np.array_equal(np.asarray((GcBareNpyTest & "rid=1").fetch1("waveform")), waveform) + + def test_collecting_the_default_store_by_name_keeps_bare_at_files(self, schema_bare): + """A collector bound to the default store by name must count bare-@ references. + + Their metadata records an empty store name; if the scan compared it to "local" + literally, the files would be reported as orphans and deleted. + """ + waveform = np.arange(64, dtype="float32") + GcBareNpyTest.insert1({"rid": 1, "waveform": waveform}) + path = (GcBareNpyTest & "rid=1").fetch1("waveform").path + + stats = _gc(schema_bare, store="local").collect(dry_run=False) + + assert stats["schema_paths_referenced"] >= 1 + assert not any(orphan.endswith(path) for orphan in stats["orphaned_schema_paths"]) + assert np.array_equal(np.asarray((GcBareNpyTest & "rid=1").fetch1("waveform")), waveform) + + class TestDirectoryObjects: """Directory-valued objects (e.g. Zarr stores) — the referenced metadata path is a directory PREFIX whose stored form is many chunk files plus a diff --git a/tests/unit/test_settings.py b/tests/unit/test_settings.py index 22de19d14..ce8572894 100644 --- a/tests/unit/test_settings.py +++ b/tests/unit/test_settings.py @@ -396,6 +396,34 @@ def test_get_store_spec_default_store(self): finally: dj.config.stores = original_stores + def test_get_store_spec_empty_name_is_default_store(self): + """An empty store name (the bare ``@`` in ````) resolves to stores.default.""" + original_stores = dj.config.stores.copy() + try: + dj.config.stores["default"] = "my_default" + dj.config.stores["my_default"] = { + "protocol": "file", + "location": "/tmp/default", + } + assert dj.config.get_store_spec("") == dj.config.get_store_spec(None) + assert dj.config.get_store_spec("")["location"] == "/tmp/default" + finally: + dj.config.stores = original_stores + + def test_get_store_spec_empty_name_is_filepath_default(self): + """For filepath references, an empty store name resolves to stores.filepath_default.""" + original_stores = dj.config.stores.copy() + try: + dj.config.stores["filepath_default"] = "raw_data" + dj.config.stores["raw_data"] = { + "protocol": "file", + "location": "/data/acquisition", + } + spec = dj.config.get_store_spec("", use_filepath_default=True) + assert spec["location"] == "/data/acquisition" + finally: + dj.config.stores = original_stores + def test_get_store_spec_no_default_configured(self): """Test error when stores.default is not configured.""" original_stores = dj.config.stores.copy() From 60bfa7e36d4a296b639649ad518c923f64815c02 Mon Sep 17 00:00:00 2001 From: Dimitri Yatsenko Date: Thu, 8 Oct 2026 21:37:30 -0500 Subject: [PATCH 2/4] Test that the bare @ form raises when no default store is configured An empty store name means the default store only when stores.default is set. Without it, get_store_spec and an insert into a column must raise "stores.default is not configured" rather than pick some store. --- tests/integration/test_gc.py | 16 ++++++++++++++++ tests/unit/test_settings.py | 10 ++++++++++ 2 files changed, 26 insertions(+) diff --git a/tests/integration/test_gc.py b/tests/integration/test_gc.py index af4f85fcc..04f30fd5e 100644 --- a/tests/integration/test_gc.py +++ b/tests/integration/test_gc.py @@ -425,6 +425,22 @@ def schema_bare(self, connection_test, prefix, mock_stores): yield schema schema.drop() + def test_insert_without_default_store_raises(self, connection_test, prefix, mock_stores): + """```` with no stores.default configured fails loudly on insert.""" + dj.config.stores.pop("default", None) + schema = dj.Schema( + f"{prefix}_test_gc_bare_at_no_default", + context={"GcBareNpyTest": GcBareNpyTest}, + connection=connection_test, + ) + try: + schema(GcBareNpyTest) + with pytest.raises(DataJointError, match="stores.default is not configured"): + GcBareNpyTest.insert1({"rid": 1, "waveform": np.arange(4, dtype="float32")}) + assert len(GcBareNpyTest()) == 0 + finally: + schema.drop() + def test_roundtrip(self, schema_bare): waveform = np.arange(64, dtype="float32") GcBareNpyTest.insert1({"rid": 1, "waveform": waveform}) diff --git a/tests/unit/test_settings.py b/tests/unit/test_settings.py index ce8572894..14752d62d 100644 --- a/tests/unit/test_settings.py +++ b/tests/unit/test_settings.py @@ -410,6 +410,16 @@ def test_get_store_spec_empty_name_is_default_store(self): finally: dj.config.stores = original_stores + def test_get_store_spec_empty_name_without_default_raises(self): + """With no stores.default, the bare ``@`` form names no store: an error, not a guess.""" + original_stores = dj.config.stores.copy() + try: + dj.config.stores = {"main": {"protocol": "file", "location": "/tmp/main"}} + with pytest.raises(DataJointError, match="stores.default is not configured"): + dj.config.get_store_spec("") + finally: + dj.config.stores = original_stores + def test_get_store_spec_empty_name_is_filepath_default(self): """For filepath references, an empty store name resolves to stores.filepath_default.""" original_stores = dj.config.stores.copy() From bdfb1a529538603bef8d8de7dbed46c8e2f74b44 Mon Sep 17 00:00:00 2001 From: Dimitri Yatsenko Date: Tue, 13 Oct 2026 14:27:53 -0500 Subject: [PATCH 3/4] ci: re-trigger checks From 52eff8f9e653e5dc5b0ef33e4d45273046535f40 Mon Sep 17 00:00:00 2001 From: Dimitri Yatsenko Date: Tue, 13 Oct 2026 16:24:06 -0500 Subject: [PATCH 4/4] fix: record the resolved store name; an empty name builds the default collector GarbageCollector(schema, store="") built a collector whose store stayed "", so _references matched no reference and collect(dry_run=False) would have deleted every file in the default store. An empty name now builds the default collector, as None does. Codecs recorded the store name as declared, so a bare-@ reference stored "" and followed stores.default: re-pointing the default would break reads, and a collector bound to the old store by name would delete live files. Codecs now record the resolved name (new Config.resolve_store_name, which get_store_spec also uses), so each object stays with the store it was written to. Bare @ could not be written before this change, so no stored metadata has an empty name; metadata that records none still means the current default. --- src/datajoint/builtin_codecs/filepath.py | 2 +- src/datajoint/builtin_codecs/npy.py | 2 +- src/datajoint/builtin_codecs/object.py | 2 +- src/datajoint/builtin_codecs/schema.py | 2 +- src/datajoint/gc.py | 9 ++- src/datajoint/hash_registry.py | 2 +- src/datajoint/settings.py | 74 +++++++++++++++++------- tests/integration/test_gc.py | 47 ++++++++++++++- tests/unit/test_settings.py | 29 ++++++++++ 9 files changed, 137 insertions(+), 32 deletions(-) diff --git a/src/datajoint/builtin_codecs/filepath.py b/src/datajoint/builtin_codecs/filepath.py index f20047963..9ba128c86 100644 --- a/src/datajoint/builtin_codecs/filepath.py +++ b/src/datajoint/builtin_codecs/filepath.py @@ -164,7 +164,7 @@ def encode( return { "path": path, - "store": store_name, + "store": config.resolve_store_name(store_name, use_filepath_default=True), "size": size, "is_dir": is_dir, "timestamp": datetime.now(timezone.utc).isoformat(), diff --git a/src/datajoint/builtin_codecs/npy.py b/src/datajoint/builtin_codecs/npy.py index a2bb7519a..c6eab2118 100644 --- a/src/datajoint/builtin_codecs/npy.py +++ b/src/datajoint/builtin_codecs/npy.py @@ -356,7 +356,7 @@ def encode( # Return metadata (includes numpy-specific shape/dtype) return { "path": path, - "store": store_name, + "store": config.resolve_store_name(store_name), "dtype": str(value.dtype), "shape": list(value.shape), } diff --git a/src/datajoint/builtin_codecs/object.py b/src/datajoint/builtin_codecs/object.py index 755688262..138f8a9c2 100644 --- a/src/datajoint/builtin_codecs/object.py +++ b/src/datajoint/builtin_codecs/object.py @@ -168,7 +168,7 @@ def encode( timestamp = datetime.now(timezone.utc) metadata = { "path": path, - "store": store_name, + "store": config.resolve_store_name(store_name), "size": size, "ext": ext, "is_dir": is_dir, diff --git a/src/datajoint/builtin_codecs/schema.py b/src/datajoint/builtin_codecs/schema.py index 7a55e68a8..87a8f6148 100644 --- a/src/datajoint/builtin_codecs/schema.py +++ b/src/datajoint/builtin_codecs/schema.py @@ -66,7 +66,7 @@ def encode(self, value, *, key=None, context=None, store_name=None): ) backend = self._get_backend(store_name, config=config) backend.put_buffer(serialize(value), path) - return {"path": path, "store": store_name, ...} + return {"path": path, "store": config.resolve_store_name(store_name), ...} def decode(self, stored, *, key=None, context=None): config = self._codec_config(key, context) diff --git a/src/datajoint/gc.py b/src/datajoint/gc.py index c6836e13d..97f8c89d2 100644 --- a/src/datajoint/gc.py +++ b/src/datajoint/gc.py @@ -113,10 +113,13 @@ def __init__(self, *schemas: "Schema", store: str | None = None, config=None) -> if not schemas: raise DataJointError("At least one schema must be provided") self.schemas = schemas - self.store = store + # An empty name (the bare ``@`` form) means the default store, as None + # does. Kept as "", it would match no reference in _references, and every + # stored file would look orphaned. + self.store = store or None self.config = config if config is not None else schemas[0].connection._config - # References written with the bare ``@`` form record an empty store name - # (older metadata may record none); both mean the default store. + # References record the resolved store name. Metadata written before + # that may record an empty name or none; both mean the default store. self._default_store = self.config.stores.get("default") # Resolve the store eagerly: validates it exists and pins the backend # and section prefixes for this collector's lifetime (one store only). diff --git a/src/datajoint/hash_registry.py b/src/datajoint/hash_registry.py index fb6f22818..7a14c76f7 100644 --- a/src/datajoint/hash_registry.py +++ b/src/datajoint/hash_registry.py @@ -250,7 +250,7 @@ def put_hash( "hash": content_hash, "path": path, # Always stored for retrieval "schema": schema_name, - "store": store_name, + "store": config.resolve_store_name(store_name), "size": len(data), } diff --git a/src/datajoint/settings.py b/src/datajoint/settings.py index 454b8e0f4..38339a1d9 100644 --- a/src/datajoint/settings.py +++ b/src/datajoint/settings.py @@ -447,6 +447,58 @@ def convert_path(cls, v: Any) -> Path | None: return None return Path(v) if not isinstance(v, Path) else v + def resolve_store_name(self, store: str | None = None, *, use_filepath_default: bool = False) -> str: + """ + Name the configured store that a store reference resolves to. + + An empty or missing name means the default store: ``stores.default``, + or ``stores.filepath_default`` for filepath references. Codecs record + the resolved name in their metadata, so a stored object stays tied to + the store it was written to even if the default is later re-pointed. + + Parameters + ---------- + store : str, optional + Store name as declared. ``""`` (the bare ``@`` form) and None both + mean the default store. + use_filepath_default : bool, optional + If True, resolve the default to stores.filepath_default instead of + stores.default. Default: False. + + Returns + ------- + str + Name of a key in ``stores``. + + Raises + ------ + DataJointError + If the default store is needed but not configured. + """ + if store: + return store + # An empty name is the bare ``@`` form (````, ````, ...), + # which the declaration keeps as "" so the codec can tell "store + # requested" from "no store"; it means the default store. + if use_filepath_default: + # Filepath references use separate default (not part of OAS) + if "filepath_default" not in self.stores: + raise DataJointError( + "stores.filepath_default is not configured. " + "Set stores.filepath_default or specify store explicitly with " + ) + store = self.stores["filepath_default"] + else: + # Integrated storage (hash, schema) uses stores.default + if "default" not in self.stores: + raise DataJointError("stores.default is not configured") + store = self.stores["default"] + + if not isinstance(store, str): + default_key = "filepath_default" if use_filepath_default else "default" + raise DataJointError(f"stores.{default_key} must be a string") + return store + def get_store_spec(self, store: str | None = None, *, use_filepath_default: bool = False) -> dict[str, Any]: """ Get configuration for a storage store. @@ -470,27 +522,7 @@ def get_store_spec(self, store: str | None = None, *, use_filepath_default: bool DataJointError If store is not configured or has invalid config. """ - # Handle default store. An empty name is the bare ``@`` form (````, - # ````, ...), which the declaration keeps as "" so the codec can - # tell "store requested" from "no store"; it means the default store. - if not store: - if use_filepath_default: - # Filepath references use separate default (not part of OAS) - if "filepath_default" not in self.stores: - raise DataJointError( - "stores.filepath_default is not configured. " - "Set stores.filepath_default or specify store explicitly with " - ) - store = self.stores["filepath_default"] - else: - # Integrated storage (hash, schema) uses stores.default - if "default" not in self.stores: - raise DataJointError("stores.default is not configured") - store = self.stores["default"] - - if not isinstance(store, str): - default_key = "filepath_default" if use_filepath_default else "default" - raise DataJointError(f"stores.{default_key} must be a string") + store = self.resolve_store_name(store, use_filepath_default=use_filepath_default) # Check store exists if store not in self.stores: diff --git a/tests/integration/test_gc.py b/tests/integration/test_gc.py index 04f30fd5e..2a90d73a7 100644 --- a/tests/integration/test_gc.py +++ b/tests/integration/test_gc.py @@ -2,6 +2,7 @@ Tests for garbage collection (gc.py). """ +import json from contextlib import ExitStack from unittest.mock import MagicMock, patch @@ -448,17 +449,57 @@ def test_roundtrip(self, schema_bare): assert np.array_equal(np.asarray((GcBareNpyTest & "rid=1").fetch1("waveform")), waveform) def test_collecting_the_default_store_by_name_keeps_bare_at_files(self, schema_bare): - """A collector bound to the default store by name must count bare-@ references. + """A collector bound to the default store by name must count bare-@ references.""" + waveform = np.arange(64, dtype="float32") + GcBareNpyTest.insert1({"rid": 1, "waveform": waveform}) + path = (GcBareNpyTest & "rid=1").fetch1("waveform").path + + stats = _gc(schema_bare, store="local").collect(dry_run=False) + + assert stats["schema_paths_referenced"] >= 1 + assert not any(orphan.endswith(path) for orphan in stats["orphaned_schema_paths"]) + assert np.array_equal(np.asarray((GcBareNpyTest & "rid=1").fetch1("waveform")), waveform) + + def test_metadata_records_the_resolved_store(self, schema_bare): + """A bare-@ write records the store it went to, not the empty name.""" + GcBareNpyTest.insert1({"rid": 1, "waveform": np.arange(8, dtype="float32")}) + + (raw,) = (GcBareNpyTest & "rid=1").proj("waveform").cursor(as_dict=True) + metadata = raw["waveform"] + if isinstance(metadata, str): # MySQL returns JSON as text, PostgreSQL as a dict + metadata = json.loads(metadata) + + assert metadata["store"] == "local" + + def test_repointing_the_default_store_keeps_existing_files(self, schema_bare): + """Re-pointing stores.default leaves existing bare-@ objects readable and collected. - Their metadata records an empty store name; if the scan compared it to "local" - literally, the files would be reported as orphans and deleted. + References record "local", so after the default moves to "raw" they still read + from "local", and a collector bound to "local" still counts them. """ waveform = np.arange(64, dtype="float32") GcBareNpyTest.insert1({"rid": 1, "waveform": waveform}) path = (GcBareNpyTest & "rid=1").fetch1("waveform").path + dj.config.stores["default"] = "raw" + assert np.array_equal(np.asarray((GcBareNpyTest & "rid=1").fetch1("waveform")), waveform) + stats = _gc(schema_bare, store="local").collect(dry_run=False) + assert not any(orphan.endswith(path) for orphan in stats["orphaned_schema_paths"]) + assert np.array_equal(np.asarray((GcBareNpyTest & "rid=1").fetch1("waveform")), waveform) + + def test_empty_store_name_is_the_default_collector(self, schema_bare): + """``store=""`` builds the same collector as no store, and deletes no live file.""" + waveform = np.arange(64, dtype="float32") + GcBareNpyTest.insert1({"rid": 1, "waveform": waveform}) + path = (GcBareNpyTest & "rid=1").fetch1("waveform").path + + collector = _gc(schema_bare, store="") + assert collector.store is None + + stats = collector.collect(dry_run=False) + assert stats["schema_paths_referenced"] >= 1 assert not any(orphan.endswith(path) for orphan in stats["orphaned_schema_paths"]) assert np.array_equal(np.asarray((GcBareNpyTest & "rid=1").fetch1("waveform")), waveform) diff --git a/tests/unit/test_settings.py b/tests/unit/test_settings.py index 14752d62d..a2fd26b3f 100644 --- a/tests/unit/test_settings.py +++ b/tests/unit/test_settings.py @@ -434,6 +434,35 @@ def test_get_store_spec_empty_name_is_filepath_default(self): finally: dj.config.stores = original_stores + def test_resolve_store_name(self): + """An empty or missing name resolves to the configured default; a name to itself.""" + original_stores = dj.config.stores.copy() + try: + dj.config.stores = { + "default": "main", + "filepath_default": "raw_data", + "main": {"protocol": "file", "location": "/tmp/main"}, + "raw_data": {"protocol": "file", "location": "/data/acquisition"}, + "archive": {"protocol": "file", "location": "/tmp/archive"}, + } + assert dj.config.resolve_store_name("") == "main" + assert dj.config.resolve_store_name(None) == "main" + assert dj.config.resolve_store_name("archive") == "archive" + assert dj.config.resolve_store_name("", use_filepath_default=True) == "raw_data" + assert dj.config.resolve_store_name("archive", use_filepath_default=True) == "archive" + finally: + dj.config.stores = original_stores + + def test_resolve_store_name_without_default_raises(self): + """The bare ``@`` form with no stores.default configured is an error.""" + original_stores = dj.config.stores.copy() + try: + dj.config.stores = {"main": {"protocol": "file", "location": "/tmp/main"}} + with pytest.raises(DataJointError, match="stores.default is not configured"): + dj.config.resolve_store_name("") + finally: + dj.config.stores = original_stores + def test_get_store_spec_no_default_configured(self): """Test error when stores.default is not configured.""" original_stores = dj.config.stores.copy()