Repository navigation
fix(idempotency): add deterministic support for set, frozenset, and bytes types - #8741
Conversation
powertools-for-aws-oss-automation
Bot
added
the
size/L
|
Thanks a lot for your first contribution! Please check out our contributing guidelines and don't hesitate to ask whatever you need. |
blacksoxx
left a comment
blacksoxx
left a comment
There was a problem hiding this comment.
Comment. The encoder change does what #8725 asks and the two insertion orders hashing to one key is the test that matters; one small gap in the docs.
Reviewed head fc3b4a5a against develop at 3b261550; both unchanged just before posting, PR open. Validation: diff read against the current default(); the key=repr fallback is deterministic for equal contents and an element that is not serialisable still fails when the sorted list is encoded, so no silent hash. Not run here: the idempotency suites. History: #8725 is the maintainers' own issue, assigned to you; no earlier PR for these types (searched "idempotency set frozenset bytes", "idempotency set bytes hash", 100 results per query, capped).
[P3] docs/utilities/idempotency.md line 195 lists which values the hash encoder converts (UUID, paths, dates, enums); sets, frozensets (as sorted lists) and bytes (base64) belong in that sentence now, otherwise the docs still say they are unsupported. No unrelated scope changes.
I would merge this with that line added; the decision is with the maintainers.
Manishram-ai
left a comment
Manishram-ai
left a comment
There was a problem hiding this comment.
I tried this with a set of frozensets and the key changed between processes, details on the set branch below.
| if isinstance(obj, (set, frozenset)): | ||
| try: | ||
| return sorted(obj) | ||
| except TypeError: | ||
| return sorted(obj, key=repr) |
There was a problem hiding this comment.
sorted() doesn't raise for a set of frozensets, because frozensets compare as subsets, so it just returns them in whatever order the set iterates. String hashes change between processes, so the same payload can get a different idempotency key after a cold start. I ran {"groups": {frozenset({"admins"}), frozenset({"billing"}), frozenset({"ops", "dev"})}} through the encoder with PYTHONHASHSEED 1 to 6 and got three different orders. The repr fallback has the same problem, since a frozenset's repr follows hash order too.
Sorting by each item's JSON gives the same order every time. With this, all six seeds give [["admins"], ["billing"], ["dev", "ops"]], and test_idempotency_hash_serialization.py still passes:
| if isinstance(obj, (set, frozenset)): | |
| try: | |
| return sorted(obj) | |
| except TypeError: | |
| return sorted(obj, key=repr) | |
| if isinstance(obj, (set, frozenset)): | |
| # Sort by each item's JSON so the order doesn't depend on hashing. sorted() alone isn't enough: | |
| # frozensets compare as subsets, and their repr() follows hash order, which changes between processes. | |
| return sorted(obj, key=lambda item: json.dumps(item, cls=_IdempotencyKeyEncoder, sort_keys=True)) |
leandrodamascena
left a comment
leandrodamascena
left a comment
There was a problem hiding this comment.
Thanks @Neeraj111010! Sorting the sets and base64 for bytes only touches types that blow up today, so nothing stored changes its hash, which was my main worry. Ran the idempotency suite on top of today's develop, all green. Goes out in 3.41.0, no need to rebase, you'll be in the release notes.
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #8741 +/- ##
===========================================
- Coverage 97.18% 97.17% -0.02%
===========================================
Files 323 323
Lines 16562 16573 +11
Branches 1497 1499 +2
===========================================
+ Hits 16096 16105 +9
- Misses 326 328 +2
Partials 140 140 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
8932649
into
aws-powertools:develop
|
Awesome work, congrats on your first merged pull request and thank you for helping improve everyone's experience! |
|
Awesome work, congrats on your first merged pull request and thank you for helping improve everyone's experience! |
Issue number: closes #8725
Summary
This PR adds native support for
set,frozenset, andbytestypes to the AWS Lambda Powertools_IdempotencyKeyEncoder. It ensures sets and frozensets are sorted deterministically regardless of insertion order, and encodes bytes using Base64 so they can be securely and reliably hashed and validated in the idempotency utilityChanges
aws_lambda_powertools/utilities/idempotency/persistence/base.py):setandfrozensetby converting them into sorted lists to guarantee identical hashes across varying element insertion orders, with safe fallback sorting (key=repr) for non-comparable items.bytes(base64.b64encode(obj).decode("utf-8")).tests/functional/idempotency/_boto3/test_idempotency_hash_serialization.py):setandbytesfrom unsupported error test cases.test_set_frozenset_and_bytes_supportto validate persistent replay, payload validation, and insertion order determinism.User experience
set,frozenset, orbytesobjects to an idempotent function would raise aTypeErrorbecause they were not JSON serializable or produced non-deterministic hashes due to unordered iteration.Verification
make lint/ruff, passing all style and formatting checks with zero errors or warnings.uv run pytest tests/functional/idempotency/, with 33 functional test cases passing successfully.By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.
Disclaimer: We value your time and bandwidth. As such, any pull requests created on non-triaged issues might not be successful.