Skip to content

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

Description

@dimitri-yatsenko

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.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions