Repository navigation
fix: unmask delete errors, make values/items membership work, drop debug prints - #2
Merged
thorwhalen merged 1 commit intoSep 22, 2026
Conversation
…bug prints Three contained defects, all reproduced offline with an in-memory boto3 stand-in (no AWS, no DynamoDB Local) in the new tests/test_offline.py. 1. `DynamoDbPartitionPersister.__delitem__` did `getattr(e, "__name__")` inside `except Exception as e`. Exception *instances* never carry `__name__` (it lives on the class), so every failing delete raised `AttributeError: 'ClientError' object has no attribute '__name__'` and destroyed the real error. `DynamoDbBasePersister.__delitem__` guarded the same line with `hasattr` and so silently never detected a missing key either. Both now share `is_no_such_key_error`, which reads the backend error code from `error.response['Error']['Code']` (falling back to the exception's class name). The real exception propagates, and the intended `NoSuchKeyError` becomes reachable for the first time. 2. The nested `ValuesView.__contains__` / `ItemsView.__contains__` called `self._mapping.contains_value(...)` / `contains_item(...)`, methods that exist neither here nor in dol — so `v in store.values()` and `item in store.items()` both raised `AttributeError`. They are now implemented against the fast paths that do exist: values reuse the single-scan `iter_values`, items do one point read via `get`. 3. Removed three debug prints that shipped in the library: `x:` in `decimal_to_float` (recursive, so once per value *and* per nested element on every read), `obj:` in `format_get_item` (every `__getitem__`), and `getitem:` in `DynamoDbPartitionReader`. Backward compatible: no public name is removed, renamed or re-signatured. Each changed path today either raises `AttributeError` unconditionally or writes to stdout, so there is no working behaviour to preserve. Refs #1 (the footnote defects only; the structural dol-hooks refactor stays open). Claude-Session: https://claude.ai/code/session_01L1aQPB34n7PU7jmbztSjBe
thorwhalen
deleted the
fix/delitem-error-masking-membership-views-and-debug-prints
branch
September 22, 2026 12:49
thorwhalen
added a commit
that referenced
this pull request
Sep 22, 2026
… longer KeyErrors (#3) DynamoDB DeleteItem succeeds silently on an absent key (NoSuchKey is an S3 code), so #2's NoSuchKey mapping never fired: del store[missing] did nothing. Delete now asks for ReturnValues=ALL_OLD and raises NoSuchKeyError when nothing was deleted. __getitem__ no longer turns every exception (throttling, auth, missing table) into NoSuchKeyError, which made .get() and the items view that #2 added report present keys as absent; only a missing Item or a key the schema rejects (ValidationException) is a KeyError now. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Three contained defects, all reproduced offline with an in-memory boto3 stand-in
(no AWS, no DynamoDB Local) in the new
tests/test_offline.py:DynamoDbPartitionPersister.__delitem__didgetattr(e, "__name__")insideexcept Exception as e. Exception instances never carry__name__, so everyfailing delete raised
AttributeErrorand destroyed the real error.DynamoDbBasePersister.__delitem__guarded the same line withhasattrand sosilently never detected a missing key either. Both now share
is_no_such_key_error, reading the backend error code fromerror.response['Error']['Code'].ValuesView.__contains__/ItemsView.__contains__calledself._mapping.contains_value(...)/contains_item(...), methods that existneither here nor in dol — so
v in store.values()anditem in store.items()both raised
AttributeError. Now implemented against the fast paths that doexist (single-scan
iter_values, one point read viaget).Backward compatible: no public name is removed, renamed or re-signatured. Each
changed path today either raises
AttributeErrorunconditionally or writes tostdout, so there is no working behaviour to preserve.
Refs #1 — this fixes the footnote defects only; the structural dol-hooks refactor
in that issue stays open, so this PR does not close it.
Branch sat pushed with green CI and no PR for two weeks (thorwhalen/fleet_stuff
cleanup). Verified: master has not moved since the branch was cut (no rebase
needed), and
wads ci-localis green on py3.10 and py3.12 (11/11 checks, 11 testspassed on each).
dynamodolhas no dependents in the fleet manifest.🤖 Generated with Claude Code