Skip to content

Commit 0b8e7f6

Browse files
fix: Load any stores.<name>.<attr> secret file; drop Py<3.10 dead code
Two changes: 1. settings.py — Config._load_secrets() Was hardcoded to only auto-load stores.<name>.access_key and stores.<name>.secret_key from the secrets directory. That whitelist made sense when only the built-in s3 / gcs / azure stores existed, but it's wrong for plugin-registered adapters that define their own secret fields. Concrete example: a plugin that wraps Databricks Unity Catalog Volumes authenticates via a Bearer token. Its store spec wants a `token` field. Today the file .secrets/stores.uc.token is silently ignored, forcing users to either rename to a misleading "access_key" or to set the value programmatically at notebook startup (defeating the auto-load story). This change drops the whitelist. Any stores.<name>.<attr> file under the secrets directory is loaded into dj.config["stores"][<name>][<attr>]. The existing "don't override pre-configured values" precedence is preserved — config file and env vars still win. New test: TestStoreSecrets.test_load_store_arbitrary_attr confirms that `token` and `api_key` fields load via the same path as access_key/secret_key. 2. storage_adapter.py — _discover_adapters() Removed the try/except wrapping importlib.metadata.entry_points(). The fallback path was for Python < 3.10 (where entry_points() returned a SelectableGroups dict instead of accepting the group= kwarg), but DataJoint's minimum supported Python is 3.10 (per requires-python in pyproject.toml). The dead branch was also producing a mypy type error (SelectableGroups.get signature).
1 parent ada2ee4 commit 0b8e7f6

3 files changed

Lines changed: 34 additions & 22 deletions

File tree

src/datajoint/settings.py

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -691,26 +691,27 @@ def _load_secrets(self, secrets_dir: Path) -> None:
691691
self.database.password = db_password
692692
logger.debug(f"Loaded database.password from {secrets_dir}")
693693

694-
# Load per-store secrets (stores.<name>.access_key, stores.<name>.secret_key)
695-
# Iterate through all files in secrets directory
694+
# Load per-store secrets from any stores.<name>.<attr> file.
695+
# The attr name is recorded as-is on stores.<name>; this lets
696+
# plugin-registered adapters define their own secret fields
697+
# (e.g. a Bearer ``token`` for HTTP-based protocols) without
698+
# forcing AWS-style ``access_key`` / ``secret_key`` naming.
696699
if secrets_dir.is_dir():
697700
for secret_file in secrets_dir.iterdir():
698701
if not secret_file.is_file() or secret_file.name.startswith("."):
699702
continue
700703

701704
parts = secret_file.name.split(".")
702-
# Check for stores.<name>.access_key or stores.<name>.secret_key pattern
703705
if len(parts) == 3 and parts[0] == "stores":
704706
store_name, attr = parts[1], parts[2]
705-
if attr in ("access_key", "secret_key"):
706-
value = secret_file.read_text().strip()
707-
# Initialize store dict if needed
708-
if store_name not in self.stores:
709-
self.stores[store_name] = {}
710-
# Only set if not already present
711-
if attr not in self.stores[store_name]:
712-
self.stores[store_name][attr] = value
713-
logger.debug(f"Loaded stores.{store_name}.{attr} from {secrets_dir}")
707+
value = secret_file.read_text().strip()
708+
# Initialize store dict if needed
709+
if store_name not in self.stores:
710+
self.stores[store_name] = {}
711+
# Only set if not already present (config / env vars win)
712+
if attr not in self.stores[store_name]:
713+
self.stores[store_name][attr] = value
714+
logger.debug(f"Loaded stores.{store_name}.{attr} from {secrets_dir}")
714715

715716
@contextmanager
716717
def override(self, **kwargs: Any) -> Iterator["Config"]:

src/datajoint/storage_adapter.py

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -86,16 +86,9 @@ def get_storage_adapter(protocol: str) -> StorageAdapter | None:
8686

8787
def _discover_adapters() -> None:
8888
"""Load storage adapters from datajoint.storage entry points."""
89-
try:
90-
from importlib.metadata import entry_points
91-
except ImportError:
92-
logger.debug("importlib.metadata not available, skipping adapter discovery")
93-
return
94-
95-
try:
96-
eps = entry_points(group="datajoint.storage")
97-
except TypeError:
98-
eps = entry_points().get("datajoint.storage", [])
89+
from importlib.metadata import entry_points
90+
91+
eps = entry_points(group="datajoint.storage")
9992

10093
for ep in eps:
10194
if ep.name in _adapter_registry:

tests/unit/test_settings.py

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -487,6 +487,24 @@ def test_secrets_do_not_override_existing(self, tmp_path):
487487
finally:
488488
cfg.stores = original_stores
489489

490+
def test_load_store_arbitrary_attr(self, tmp_path):
491+
"""Plugin-registered adapters can use arbitrary secret-field names."""
492+
# e.g. an HTTP-based protocol that authenticates with a Bearer token
493+
secrets_dir = tmp_path / SECRETS_DIRNAME
494+
secrets_dir.mkdir()
495+
(secrets_dir / "stores.bearer_store.token").write_text("dapibdfXXXX")
496+
(secrets_dir / "stores.bearer_store.api_key").write_text("ak_yyy")
497+
498+
cfg = settings.Config()
499+
original_stores = cfg.stores.copy()
500+
try:
501+
cfg._load_secrets(secrets_dir)
502+
503+
assert cfg.stores["bearer_store"]["token"] == "dapibdfXXXX"
504+
assert cfg.stores["bearer_store"]["api_key"] == "ak_yyy"
505+
finally:
506+
cfg.stores = original_stores
507+
490508

491509
class TestDisplaySettings:
492510
"""Test display-related settings."""

0 commit comments

Comments
 (0)