Codec encode/decode: separate connection context from primary key in the `key` argument

Open
#1,550 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Assessment

Difficulty
5/5
Estimated time
Over a week
Newbie friendliness
35/100
Issue type
Refactor
Clarity
Mostly clear
Activity status
Active
Tech stack
python

Research direction

Start by tracing SchemaCodec.encode/decode and _extract_context to see how primary keys and connection context flow into _build_path and _get_backend. Review the built-in object, npy, hash, filepath, attach, and blob codecs, plus the cited third-party codecs. Done means the signature and callers use separate key and context values, and required config is passed explicitly without the global fallback.

Written by the indexing model from the issue text.

Description

Background

`SchemaCodec.encode(self, value, *, key=None, store_name=None)` and `.decode(self, stored, *, key=None)` overload a single `key` dict with two different kinds of data:

  • primary key values (the row this codec call is writing/reading)
  • connection context — `_schema`, `_table`, `_field`, and critically `_config` (the calling connection's `Config`, carrying that connection's store credentials)

`_extract_context` separates these by convention — anything with a leading underscore is context, everything else is primary key:

```python
def _extract_context(self, key: dict | None) -> tuple[str, str, str, dict]:
key = dict(key) if key else {}
schema = key.pop("_schema", "unknown")
table = key.pop("_table", "unknown")
field = key.pop("field", "data")
primary_key = {k: v for k, v in key.items() if not k.startswith("
")}
return schema, table, field, primary_key
```

`config` is read directly by codec authors as `(key or {}).get("_config")` and threaded manually into `_build_path`/`_get_backend`.

Two problems this causes

1. Naming collision risk. The underscore-prefix convention is not structurally enforced — it works today only because DataJoint's identifier grammar disallows attribute names starting with `_`. That's an implicit, undocumented invariant; nothing raises if it's ever violated (a future internal/system column, a future relaxation of the naming grammar), and a colliding key would silently vanish from `primary_key` rather than error.

2. Wrong failure mode for a required dependency. `config` is functionally required for correct store resolution in any multi-connection process (e.g., a dashboard serving several users), but it's passed as an optional dict key a codec author must remember to `.get()` out and thread through by hand. Forgetting to do so doesn't fail loudly — it silently falls back to global `dj.config`, which either raises a confusing `DataJointError: Missing S3 configuration` deep inside `StorageBackend._validate_spec`, or worse, resolves a different-but-valid store with no error at all.

That second failure mode is what actually happened, twice, independently: `dj-figpack-codecs#6` and `dj-canvasxpress-codecs#3` both omitted `config=` on `_build_path`/`_get_backend`, both following the pattern shown in `SchemaCodec`'s own docstring example (fixed in #1549, docs-only).

Proposed direction

Make connection context an explicit, separately named parameter rather than a dict key:

```python
def encode(self, value, *, key=None, context=None, store_name=None): ...
def decode(self, stored, *, key=None, context=None): ...
```

`key` reverts to meaning exactly what it means everywhere else in DataJoint (fetch, insert, `make()`) — the primary key dict, nothing else. `context` (a dict or small structure) carries `schema`, `table`, `field`, `config`.

If `_build_path`/`_get_backend` also made `config` a required parameter rather than defaulting to `None` + global fallback, forgetting to pass it becomes a loud `TypeError` at the call site instead of a quiet wrong-store bug discovered later in production — a strictly better failure mode than the current one, and better than just nesting context under a single `key["_context"]` entry (which fixes the collision risk but not the silent-fallback problem).

Cost

Breaking change to `Codec.encode`/`decode` and every codec reading `key["_config"]` or calling `_extract_context` — built-ins (`object`, `npy`, `hash`, `filepath`, `attach`, `blob`) plus third-party codecs (`dj-figpack-codecs`, `dj-canvasxpress-codecs`). Worth doing now rather than later — the codec system is still being shaken out, evidenced by this same-week cluster of config-threading fixes, so the migration surface is as small as it will ever be.

Raised following review of #1549, which fixes the docstring example to match current (flawed) behavior — this issue proposes revisiting the underlying signature instead.

Dominant language
Python
Stars
197
Forks
98
Avg merge
6d 10h
Merged PRs (30d)
4

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from datajoint/datajoint-python

All issues in datajoint/datajoint-python

Similar issues

More Python issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.