Repository navigation
Conversation
…igured expiry The lock timeout was cached in __init__ before configure() applied IdempotencyConfig, so min(10, 3600) always produced 10 seconds. Compute it in _acquire_lock from the configured expires_after_seconds instead.
|
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
There was a problem hiding this comment.
Reads correct to me; I did not run the tests here.
On develop the timeout is frozen in __init__ right after super().__init__() sets the 3600 default, and configure() only runs later from the decorator, so min(10, 3600) was always 10. Computing it in _acquire_lock from self.expires_after_seconds fixes that and keeps the cap. Nothing else referenced _orphan_lock_timeout. The test goes through idempotent with a fake client and reads only the ex passed to set, which fits the public-API rule in AGENTS.md, and closes #8726 sits on its own line.
Validation: read the diff at ada9293 against base 111f93b, redis.py and base.py on develop. Not run: pytest, mypy.
|
Thanks for the confirmation! Let me know if there's anything else needed for this to be merged. |
|
Hi @blacksoxx, I noticed the branch is currently behind develop. Would you prefer me to rebase and force-push to clean up the history, or will this be handled automatically during the squash merge? Thanks! |
|
I am a contributor here, not a maintainer, so the merge mechanics are theirs to decide. For what it is worth: the PR shows as mergeable, "behind" only means develop moved on, and PRs here land as single squash commits, so a rebase is only needed if a conflict appears or a maintainer asks for one. |



Summary
The Redis/Cache orphan lock timeout was computed in
CachePersistenceLayer.__init__, right aftersuper().__init__()had setexpires_after_secondsto its 3600-second default.min(10, 3600)therefore always produced 10, and the laterconfigure()call — which applies the user'sIdempotencyConfig— never changed it. A lock could outlive the record it protects whenexpires_after_secondswas below 10, leaving later calls unable to take over an orphaned record.The timeout is now computed at the point of use in
_acquire_lock, fromself.expires_after_secondsafter configuration. The 10-second cap is preserved for longer expiries.Test
Added
test_orphan_lock_ttl_follows_configured_expiryintests/functional/idempotency/_redis/test_redis_layer.py. It drives the publicidempotentdecorator with a fake Redis client that records theexargument of everysetcall, seeds an expired INPROGRESS record, and checks theexused for the:lockkey:expires_after_seconds=2-> lockex=2(was 10 before the fix)expires_after_seconds=30-> lockex=10(cap unchanged)Verification run locally (terminal output):
33 passed.ruff format aws_lambda_powertools tests examples --check:1398 files already formatted;ruff check aws_lambda_powertools tests examples:All checks passed!.mypyon the changed source:Success: no issues found in 1 source file.uv run pytest tests/functional/idempotency tests/unit/idempotency -q->210 passed.auth_alpha/oauth2timing andevent_handler/_pydanticPurePath encoding) fail on this Windows/Python 3.14 setup; I confirmed both fail identically on an unmodifieddevelopcheckout, so they are pre-existing environment failures, not regressions from this change.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.
closes #8726