Robustly wrapping stores that have key- and value-taking methods #86
Replies: 2 comments
Option G — spec-carried boundary codecs on a flat proxy (design study + prototype)Following up on the maintainer's proposal (2026-08-10, relayed off-thread — putting it on record here): study wrapt's object-proxy design; wrap incoming/outgoing keys and values in all methods per a specification (e.g. a Protocol annotated with KT/VT); generalize beyond KT/VT to arbitrary "types of interest"; accumulate wrap layers in two flat lists (encoders/decoders) and compile them for speed and validation; take care that method-calls-method never double-applies. This is now evaluated as Option G in this thread's option space:
The short version
The doc's §9 rescopes migration honestly (P2 is not "wrap_kvs becomes a facade" — inner-layer |
Decisions on the Option G open questions (maintainer, 2026-08-10)All six questions from
Next milestones per the (rescoped) plan: P1 — census-family adapters trial |
Uh oh!
There was an error while loading. Please reload this page.
Design rationale, consolidated so the next person does not re-derive it. Not a work item —
the work items are #83, #82, #18, #10, #6, #5, and PRs #84 / #85.
dol's core promise is that you can wrap a store with layers of transformations and keep usingit as a Mapping. That promise holds for the Mapping protocol and fails for everything else:
a wrapped store's non-dunder methods receive the outer, unmapped key, and its methods'
internal
self[k]bypasses the transforms. Those are the two halves of one root cause, and a13-package census says the family has quietly been living with both.
This writes down what we know, what we tried, and what each option actually costs — including
three options that look right and are not.
1. The mechanism, precisely
dolwraps by delegation (has-a):wrap_kvs(SomeClass)returns aStoresubclass holdingan instance of
SomeClassinself.store. The transform hooks(
_id_of_key/_key_of_id/_data_of_obj/_obj_of_data) live on the wrapper. The innerinstance never sees them.
Two consequences, in opposite directions:
self[k]bypasses the transformsAnd two delegation routes, which matters because a fix covering one is a silent no-op on the
other:
Store.__getattr__mk_relative_path_storesubclassesdol/base.py:742DelegatedAttribute.__get__delegate_toinstalls one descriptor per attr ofdir(wrapped)dol/base.py:279, installeddol/base.py:416-480Both return the method bound to the leaf.
Nothing raises. Capability detection cannot see it either — a
@runtime_checkableProtocolchecks method presence, and since 3.12
isinstanceusesgetattr_static, soisinstance(w, SupportsUrlFor)isTruefor a class-wrap andFalsefor an instance-wrap. Inneither case does it say anything about whether the key is right.
2. Scale — the census
13 sibling packages surveyed, every claim re-verified by source read plus a runnable repro.
Index and per-package issues in #83.
The defect is overwhelmingly latent: it bites when a user applies a key codec, and most of
these packages never wrap their own stores. 12 survey claims were refuted outright. Being
accurate about this matters — an earlier draft of the s3dol write-up said "already destroying
data in production", and the evidence does not support that.
What it does support:
focal— confirmed-live throughout, inherited fromdol.filesys.Files.dolitself —Files(d).is_valid_key(k)isFalsefor a key the store yields, breakingall(s.is_valid_key(k) for k in s). Fixed in fix: two delegation bugs in dol's own code (content_url, filesys key validation) #85.cosmodol.CosmosItems.replace— under a key codec, silently overwrites a different,real document in full.
pydrivedol.GDReader.get_url— returns a URL for the wrong file and grantsanyone/reader permission on it.
xdol,unbox,lexis— the wrap_kvs will wrap the instance but self of instance is not wrapped #18 direction, already migrated towrapped_self.The shape that stays clean is instructive:
azuredol'sContainerStorehas essentially nokey-taking public methods. Its rich per-object surface lives on
BlobHandle, which binds itsblob at construction, so a key codec over the store cannot corrupt it. It is not safe because of
where its prefix lives; it is safe because it has almost no seam to get wrong.
3. The option space
A — document it, use
wrapped_selfShips today, and it is what #18's design doc recommends for Phase 1.
But
wrapped_selfhas its own silent failure mode. A delegated bound method holds noreference to the wrapper, so when nothing else does, the wrapper is freed before the body runs
and
_register_wrapper_backref's cleanup removes the registry entry — indistinguishablefrom "never wrapped":
Where the leaf owns a prefix, the wrong answer is a plausible
str, so a type check cannotcatch it. The predicate is "no live strong reference", not "temporary" — a temporary in a
reference cycle silently starts working, so the failure is intermittent. Reproduced on
CPython 3.10–3.14. Detail in #18.
Verdict: a real guardrail, and better than nothing — but not a correctness mechanism, and it
should stop being described as one.
B — declarative key-method registration
_key_methods = {'url_for': 0, 'delete_many': ('iter', 0)}, with the wrapper generating mappeddelegates.
dolalready has this:wrap_kvs(ingoing_key_methods=…, outcoming_key_methods=…)(
dol/trans.py). It is untested (the TODO says so) and verified broken for leaf-definedmethods on both wrap paths —
getattr(store_cls, name)at decoration time finds aDelegatedAttributeor nothing, and the generated body callssuper(store_cls, self).<name>(),which does not consult
Store.__getattr__. It fails loudly, at least. Building on it meansreplacing it.
The deeper problem is that a method the author forgets to declare fails the same silent way. The
part that actually holds is not the registry but a reflective test that enumerates public
methods and fails on any undeclared one.
Verdict: the guard is worth more than the registry.
C — free functions
url_for(store, k)instead ofstore.url_for(k), resolving through the chain withinner_most_key.This is
dol's own idiom (content_url,get_content,put_content,add_content), and itsidesteps A's lifetime problem because the caller holds the store.
But it is not categorically safe.
inner_most_keywalks.storeapplying each layer's_id_of_key, which breaks when a layer is not adolStore:dol-shipped wrapper__getattr__-passthrough delegatorlogs/b.txtlogs/logs/b.txt.store-holding middle layerlogs/x/b.txtx/b.txtA passthrough
__getattr__resolves_id_of_keyto the leaf's bound method, so the walkapplies it twice; a middle layer without
_id_of_keytruncates the walk. Everydol-shippedwrapper is safe — but
dol/base.py:514-518ships a documented hand-rolledDelegatorrecipe ofexactly the breaking shape.
Note also that
dol's own instance of C,content_url, had the bug until #85: it did a flatgetattr(store, 'url_for')(key).Verdict: more reliable than A, not reliable. Good for operations that are not keyed.
D — rebind delegated methods to the wrapper
Rejected, with running-code evidence, in
misc/docs/dol_issue18_design.md§4. The fataldefect is not blast radius: rebinding binds
selfto the innermostWrap, so under aPipestack it does not fix the case it exists to fix — 4 of the 6 sites that survey foundare
Pipestacks — and stacked-codec writes gain a partial-transform corruption surface. Plusstatically-undetectable crashes: a leaf method calling
super().__getitem__(k)compilessuper(SomeClass, self)withselfnow aWrap→TypeError.Its Phase-1 instructions say: do not touch
DelegatedAttribute.__get__, thedelegate_tocopyloop, or the signature graft.
Verdict: do not re-propose without new information.
E — capabilities as parallel Mappings
Two designs get conflated under this name, and only one works.
As a Mapping-valued attribute (
store.urls) — broken. A wrapper does not re-wrap such anattribute:
The outer store's keys are
['b'], sostore.urls[k]KeyErrors for every key it has. This is#10's territory. (There is a precedent for the propagation half:
Store.__init__(
dol/base.py:720-727) copiesKeysView/ValuesView/ItemsViewup from the leaf and bindsthem to the outer store. Hard-coded to three names, but it is propagation plus outer
binding, which is simpler than re-wrapping.)
As a sibling store — correct by construction. Expose the capability as its own
KvReaderover the same key space:
__getitem__is the one thingdolmaps correctly at every depth, so this needs noinner_most_key, nowrapped_self, no guard. Verified correct unreferenced, underPipe,under
cached_keys, under a value codec, and under the non-Storepassthrough layer where Cis silently wrong.
Its cost is real: the user must wrap the sibling in parallel with the data store, because
deriving the codec chain onto a sibling is exactly #10.
Verdict: the best answer available today.
s3doladopted it(ADR-0011).
F — is-a wrapping
Make
wrap_kvs(Class)return a real subclass ofClass. Thenselfinside a leaf method isthe wrapper, the transform hooks are on the MRO, and both #83 and #18 disappear — along with #6,
since the
__signature__graft exists only becauseWrapis a generic shell rather than a realsubclass.
misc/docs/dol_issue18_design.mdrecommends this as the terminal direction for 0.4/1.0, with astaged plan (opt-in
mode='isa'first, default flip gated on a green dependents run). It isx-large, coupled to #5, and its §9 still lists "commit to C as the terminal direction now?" as
an open question for the maintainer.
Verdict: the actual fix. Everything above is what you do until it lands.
4. What this implies for adapter authors today
The selection criterion that turned out to matter is not "which is the permanent answer" but
"which is correct on 0.3.x AND harmlessly redundant once is-a lands".
lever, and it is what makes
azuredolrobust. A capability that binds its key atconstruction (a handle) cannot be corrupted by a key codec at all.
__getitem__— sibling stores (E-as-store). Correct byconstruction, no primitive.
Store-layer caveat.wrapped_selfonly as a guardrail (A), never as a correctness argument.taking a key. Note a name heuristic is not enough: it misses
delete(name, ...),delete_many(keys),batch(operations),sync_to(target)— several of the worst shapes.5. Open questions
asks. Everything else is a holding pattern, so the answer changes how much to invest in the
holding pattern.
wrapped_self's lost-reference hole be fixed, or left as a known limitation? Atwo-line change to
_register_wrapper_backrefmakes it detectable-and-loud (detail in wrap_kvs will wrap the instance but self of instance is not wrapped #18).Cheap, but it hardens a mechanism F would delete.
dolgrow the sibling-store pattern as a first-class thing — a helper thatre-derives a wrapper chain onto a sibling store, which is Making
conditional_data_transbetter, and generally; recursively applying wrappers #10 — or stay out of it and letadapters ask users to wrap in parallel?
SupportsUrlForstay method-shaped? It currently forces every backend to expose akeyed method, which is the shape this whole thread is about. A store-shaped or
function-shaped seam would not.
inner_most_key? Mapping a key outward (leaf → caller) has nohelper, and anything returning keys (
prefixes,walk, a query) needs one.References
Issues: #83 (this problem + census) · #18 (root cause,
wrapped_self) · #82 (prefixcorruption) · #10 (recursive wrapping) · #6 (signature freeze) · #5 (wrapper class control).
PRs: #84, #85.
In-tree:
misc/docs/dol_issue18_design.md,misc/docs/dol_issue10_design.md,misc/docs/dol_content_metadata_bifurcation.md§2.2.Downstream: s3dol#14,
s3dol ADR-0011.
All reactions