Skip to content

Commit 52eff8f

Browse files
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.
1 parent bdfb1a5 commit 52eff8f

9 files changed

Lines changed: 137 additions & 32 deletions

File tree

‎src/datajoint/builtin_codecs/filepath.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -164,7 +164,7 @@ def encode(
164164

165165
return {
166166
"path": path,
167-
"store": store_name,
167+
"store": config.resolve_store_name(store_name, use_filepath_default=True),
168168
"size": size,
169169
"is_dir": is_dir,
170170
"timestamp": datetime.now(timezone.utc).isoformat(),

‎src/datajoint/builtin_codecs/npy.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -356,7 +356,7 @@ def encode(
356356
# Return metadata (includes numpy-specific shape/dtype)
357357
return {
358358
"path": path,
359-
"store": store_name,
359+
"store": config.resolve_store_name(store_name),
360360
"dtype": str(value.dtype),
361361
"shape": list(value.shape),
362362
}

‎src/datajoint/builtin_codecs/object.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -168,7 +168,7 @@ def encode(
168168
timestamp = datetime.now(timezone.utc)
169169
metadata = {
170170
"path": path,
171-
"store": store_name,
171+
"store": config.resolve_store_name(store_name),
172172
"size": size,
173173
"ext": ext,
174174
"is_dir": is_dir,

‎src/datajoint/builtin_codecs/schema.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ def encode(self, value, *, key=None, context=None, store_name=None):
6666
)
6767
backend = self._get_backend(store_name, config=config)
6868
backend.put_buffer(serialize(value), path)
69-
return {"path": path, "store": store_name, ...}
69+
return {"path": path, "store": config.resolve_store_name(store_name), ...}
7070
7171
def decode(self, stored, *, key=None, context=None):
7272
config = self._codec_config(key, context)

‎src/datajoint/gc.py‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -113,10 +113,13 @@ def __init__(self, *schemas: "Schema", store: str | None = None, config=None) ->
113113
if not schemas:
114114
raise DataJointError("At least one schema must be provided")
115115
self.schemas = schemas
116-
self.store = store
116+
# An empty name (the bare ``@`` form) means the default store, as None
117+
# does. Kept as "", it would match no reference in _references, and every
118+
# stored file would look orphaned.
119+
self.store = store or None
117120
self.config = config if config is not None else schemas[0].connection._config
118-
# References written with the bare ``@`` form record an empty store name
119-
# (older metadata may record none); both mean the default store.
121+
# References record the resolved store name. Metadata written before
122+
# that may record an empty name or none; both mean the default store.
120123
self._default_store = self.config.stores.get("default")
121124
# Resolve the store eagerly: validates it exists and pins the backend
122125
# and section prefixes for this collector's lifetime (one store only).

‎src/datajoint/hash_registry.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -250,7 +250,7 @@ def put_hash(
250250
"hash": content_hash,
251251
"path": path, # Always stored for retrieval
252252
"schema": schema_name,
253-
"store": store_name,
253+
"store": config.resolve_store_name(store_name),
254254
"size": len(data),
255255
}
256256

‎src/datajoint/settings.py‎

Lines changed: 53 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -447,6 +447,58 @@ def convert_path(cls, v: Any) -> Path | None:
447447
return None
448448
return Path(v) if not isinstance(v, Path) else v
449449

450+
def resolve_store_name(self, store: str | None = None, *, use_filepath_default: bool = False) -> str:
451+
"""
452+
Name the configured store that a store reference resolves to.
453+
454+
An empty or missing name means the default store: ``stores.default``,
455+
or ``stores.filepath_default`` for filepath references. Codecs record
456+
the resolved name in their metadata, so a stored object stays tied to
457+
the store it was written to even if the default is later re-pointed.
458+
459+
Parameters
460+
----------
461+
store : str, optional
462+
Store name as declared. ``""`` (the bare ``@`` form) and None both
463+
mean the default store.
464+
use_filepath_default : bool, optional
465+
If True, resolve the default to stores.filepath_default instead of
466+
stores.default. Default: False.
467+
468+
Returns
469+
-------
470+
str
471+
Name of a key in ``stores``.
472+
473+
Raises
474+
------
475+
DataJointError
476+
If the default store is needed but not configured.
477+
"""
478+
if store:
479+
return store
480+
# An empty name is the bare ``@`` form (``<npy@>``, ``<object@>``, ...),
481+
# which the declaration keeps as "" so the codec can tell "store
482+
# requested" from "no store"; it means the default store.
483+
if use_filepath_default:
484+
# Filepath references use separate default (not part of OAS)
485+
if "filepath_default" not in self.stores:
486+
raise DataJointError(
487+
"stores.filepath_default is not configured. "
488+
"Set stores.filepath_default or specify store explicitly with <filepath@store>"
489+
)
490+
store = self.stores["filepath_default"]
491+
else:
492+
# Integrated storage (hash, schema) uses stores.default
493+
if "default" not in self.stores:
494+
raise DataJointError("stores.default is not configured")
495+
store = self.stores["default"]
496+
497+
if not isinstance(store, str):
498+
default_key = "filepath_default" if use_filepath_default else "default"
499+
raise DataJointError(f"stores.{default_key} must be a string")
500+
return store
501+
450502
def get_store_spec(self, store: str | None = None, *, use_filepath_default: bool = False) -> dict[str, Any]:
451503
"""
452504
Get configuration for a storage store.
@@ -470,27 +522,7 @@ def get_store_spec(self, store: str | None = None, *, use_filepath_default: bool
470522
DataJointError
471523
If store is not configured or has invalid config.
472524
"""
473-
# Handle default store. An empty name is the bare ``@`` form (``<npy@>``,
474-
# ``<object@>``, ...), which the declaration keeps as "" so the codec can
475-
# tell "store requested" from "no store"; it means the default store.
476-
if not store:
477-
if use_filepath_default:
478-
# Filepath references use separate default (not part of OAS)
479-
if "filepath_default" not in self.stores:
480-
raise DataJointError(
481-
"stores.filepath_default is not configured. "
482-
"Set stores.filepath_default or specify store explicitly with <filepath@store>"
483-
)
484-
store = self.stores["filepath_default"]
485-
else:
486-
# Integrated storage (hash, schema) uses stores.default
487-
if "default" not in self.stores:
488-
raise DataJointError("stores.default is not configured")
489-
store = self.stores["default"]
490-
491-
if not isinstance(store, str):
492-
default_key = "filepath_default" if use_filepath_default else "default"
493-
raise DataJointError(f"stores.{default_key} must be a string")
525+
store = self.resolve_store_name(store, use_filepath_default=use_filepath_default)
494526

495527
# Check store exists
496528
if store not in self.stores:

‎tests/integration/test_gc.py‎

Lines changed: 44 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
Tests for garbage collection (gc.py).
33
"""
44

5+
import json
56
from contextlib import ExitStack
67
from unittest.mock import MagicMock, patch
78

@@ -448,17 +449,57 @@ def test_roundtrip(self, schema_bare):
448449
assert np.array_equal(np.asarray((GcBareNpyTest & "rid=1").fetch1("waveform")), waveform)
449450

450451
def test_collecting_the_default_store_by_name_keeps_bare_at_files(self, schema_bare):
451-
"""A collector bound to the default store by name must count bare-@ references.
452+
"""A collector bound to the default store by name must count bare-@ references."""
453+
waveform = np.arange(64, dtype="float32")
454+
GcBareNpyTest.insert1({"rid": 1, "waveform": waveform})
455+
path = (GcBareNpyTest & "rid=1").fetch1("waveform").path
456+
457+
stats = _gc(schema_bare, store="local").collect(dry_run=False)
458+
459+
assert stats["schema_paths_referenced"] >= 1
460+
assert not any(orphan.endswith(path) for orphan in stats["orphaned_schema_paths"])
461+
assert np.array_equal(np.asarray((GcBareNpyTest & "rid=1").fetch1("waveform")), waveform)
462+
463+
def test_metadata_records_the_resolved_store(self, schema_bare):
464+
"""A bare-@ write records the store it went to, not the empty name."""
465+
GcBareNpyTest.insert1({"rid": 1, "waveform": np.arange(8, dtype="float32")})
466+
467+
(raw,) = (GcBareNpyTest & "rid=1").proj("waveform").cursor(as_dict=True)
468+
metadata = raw["waveform"]
469+
if isinstance(metadata, str): # MySQL returns JSON as text, PostgreSQL as a dict
470+
metadata = json.loads(metadata)
471+
472+
assert metadata["store"] == "local"
473+
474+
def test_repointing_the_default_store_keeps_existing_files(self, schema_bare):
475+
"""Re-pointing stores.default leaves existing bare-@ objects readable and collected.
452476
453-
Their metadata records an empty store name; if the scan compared it to "local"
454-
literally, the files would be reported as orphans and deleted.
477+
References record "local", so after the default moves to "raw" they still read
478+
from "local", and a collector bound to "local" still counts them.
455479
"""
456480
waveform = np.arange(64, dtype="float32")
457481
GcBareNpyTest.insert1({"rid": 1, "waveform": waveform})
458482
path = (GcBareNpyTest & "rid=1").fetch1("waveform").path
459483

484+
dj.config.stores["default"] = "raw"
485+
assert np.array_equal(np.asarray((GcBareNpyTest & "rid=1").fetch1("waveform")), waveform)
486+
460487
stats = _gc(schema_bare, store="local").collect(dry_run=False)
461488

489+
assert not any(orphan.endswith(path) for orphan in stats["orphaned_schema_paths"])
490+
assert np.array_equal(np.asarray((GcBareNpyTest & "rid=1").fetch1("waveform")), waveform)
491+
492+
def test_empty_store_name_is_the_default_collector(self, schema_bare):
493+
"""``store=""`` builds the same collector as no store, and deletes no live file."""
494+
waveform = np.arange(64, dtype="float32")
495+
GcBareNpyTest.insert1({"rid": 1, "waveform": waveform})
496+
path = (GcBareNpyTest & "rid=1").fetch1("waveform").path
497+
498+
collector = _gc(schema_bare, store="")
499+
assert collector.store is None
500+
501+
stats = collector.collect(dry_run=False)
502+
462503
assert stats["schema_paths_referenced"] >= 1
463504
assert not any(orphan.endswith(path) for orphan in stats["orphaned_schema_paths"])
464505
assert np.array_equal(np.asarray((GcBareNpyTest & "rid=1").fetch1("waveform")), waveform)

‎tests/unit/test_settings.py‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -434,6 +434,35 @@ def test_get_store_spec_empty_name_is_filepath_default(self):
434434
finally:
435435
dj.config.stores = original_stores
436436

437+
def test_resolve_store_name(self):
438+
"""An empty or missing name resolves to the configured default; a name to itself."""
439+
original_stores = dj.config.stores.copy()
440+
try:
441+
dj.config.stores = {
442+
"default": "main",
443+
"filepath_default": "raw_data",
444+
"main": {"protocol": "file", "location": "/tmp/main"},
445+
"raw_data": {"protocol": "file", "location": "/data/acquisition"},
446+
"archive": {"protocol": "file", "location": "/tmp/archive"},
447+
}
448+
assert dj.config.resolve_store_name("") == "main"
449+
assert dj.config.resolve_store_name(None) == "main"
450+
assert dj.config.resolve_store_name("archive") == "archive"
451+
assert dj.config.resolve_store_name("", use_filepath_default=True) == "raw_data"
452+
assert dj.config.resolve_store_name("archive", use_filepath_default=True) == "archive"
453+
finally:
454+
dj.config.stores = original_stores
455+
456+
def test_resolve_store_name_without_default_raises(self):
457+
"""The bare ``@`` form with no stores.default configured is an error."""
458+
original_stores = dj.config.stores.copy()
459+
try:
460+
dj.config.stores = {"main": {"protocol": "file", "location": "/tmp/main"}}
461+
with pytest.raises(DataJointError, match="stores.default is not configured"):
462+
dj.config.resolve_store_name("")
463+
finally:
464+
dj.config.stores = original_stores
465+
437466
def test_get_store_spec_no_default_configured(self):
438467
"""Test error when stores.default is not configured."""
439468
original_stores = dj.config.stores.copy()

0 commit comments

Comments
 (0)