Skip to content

docs(codecs): thread the connection config in the SchemaCodec example - #1549

Open
MilagrosMarin wants to merge 1 commit into
datajoint:masterfrom
MilagrosMarin:docs/thread-config-in-schemacodec-example
Open

docs(codecs): thread the connection config in the SchemaCodec example#1549
MilagrosMarin wants to merge 1 commit into
datajoint:masterfrom
MilagrosMarin:docs/thread-config-in-schemacodec-example

Conversation

@MilagrosMarin

Copy link
Copy Markdown
Contributor

The SchemaCodec class docstring's Example:: block omits config= on both _build_path and _get_backend, so a codec written from it resolves its store against the module-level dj.config rather than the calling connection's. Where a caller holds connections with different store configurations, that config belongs to none of them — the codec either reads the wrong store or, if the global spec carries no usable credentials, fails validation.

The built-in object and npy codecs already read key["_config"] and pass it to both helpers. The example now shows the same, with a note above it explaining why. It also omitted store_name on _build_path, which is where partition and prefix settings come from.

Docs only — the executable AST is unchanged.

Worth considering separately: having _get_backend accept key= and read _config itself would remove the trap rather than document around it. That's an API change, so it isn't here.

The subclass example omitted config= on both _build_path and _get_backend, so
a codec written from it resolves its store against the module-level dj.config
instead of the calling connection's. In a process holding connections for
several users that config belongs to none of them, and on a pod with no
ambient credentials it belongs to nothing at all.

The built-in object and npy codecs already read key["_config"] and pass it to
both helpers; the example now shows the same, and a note above it says why.
The example also omitted store_name on _build_path, which is where partition
and prefix settings come from.

Docs only: the executable AST is unchanged.
@MilagrosMarin MilagrosMarin added the documentation Issues related to documentation label Sep 3, 2026

@ttngu207 ttngu207 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approve. Docstring change to the SchemaCodec example that also fixes an omission the plugin bugs didn't happen to hit — the example was missing store_name= on _build_path, which is where partition and prefix settings come from.

Your own note nails the follow-up: "having _get_backend accept key= and read _config itself would remove the trap rather than document around it." Documenting around a trap only helps the next codec author who reads the docstring. Worth filing a separate issue against this repo to change the SchemaCodec._get_backend / _build_path signatures to read _config off key= themselves, so a third-party codec can't silently drop it again. Happy to open that follow-up if you'd like.

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

Labels

documentation Issues related to documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants