Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature request: Support raise_on_no_idempotency_key for idempotent_function #1663

Closed
1 of 2 tasks
dmwesterhoff opened this issue Oct 27, 2022 · 6 comments · Fixed by #1669
Closed
1 of 2 tasks

Feature request: Support raise_on_no_idempotency_key for idempotent_function #1663

dmwesterhoff opened this issue Oct 27, 2022 · 6 comments · Fixed by #1669
Assignees
Labels
bug Something isn't working

Comments

@dmwesterhoff
Copy link

Use case

Trying to use idempotent_function in an "optional" manner. Example:

import random

@idemp_f(data_keyword_argument="key")
def echo(key: str = None):
    return random.randint(1, 1000000)

When called like echo(key=None) will raise RuntimeError, even when raise_on_no_idempotency_key is set to False

Solution/User Experience

Unless there is good reason not to respect the IdempotencyConfig.raise_on_no_idempotency_key value. Perhaps adding a similar parameter to idempotent_function like raise_on_no_data_keyword_argument or something to that effect.

Alternative solutions

No response

Acknowledgment

@dmwesterhoff dmwesterhoff added feature-request feature request triage Pending triage from maintainers labels Oct 27, 2022
@heitorlessa
Copy link
Contributor

Hey @dmwesterhoff could you post the full stack trace, please?

That seems like an issue actually more than a feature request - a full error will help speed up the diagnostics tomorrow.

Thanks a lot for flagging this!!

@heitorlessa heitorlessa added bug Something isn't working and removed triage Pending triage from maintainers feature-request feature request labels Oct 27, 2022
@heitorlessa
Copy link
Contributor

Sending from mobile, excuse me from verbosity and typos

It's a bug! It was meant to be a guard against a keyword argument not being present (L137).

This should be a check on whether the data kwarg exists in kwargs - accidentally, this is getting the value, and checking if it's falsy. This means even key=False would fail here (no good!).

As it's late here (8:48pm), I can make the fix tomorrow and cut a release with other merged enhancements -- if you have the bandwidth, please feel free to send a PR and I can review & expedite tomorrow.

https://github.com/awslabs/aws-lambda-powertools-python/blob/develop/aws_lambda_powertools/utilities/idempotency/idempotency.py

@heitorlessa
Copy link
Contributor

@leandrodamascena need your review to this bugfix PR to add to today's release.

@github-actions
Copy link
Contributor

⚠️COMMENT VISIBILITY WARNING⚠️

This issue is now closed. Please be mindful that future comments are hard for our team to see.

If you need more assistance, please either tag a team member or open a new issue that references this one.

If you wish to keep having a conversation with other community members under this issue feel free to do so.

@github-actions github-actions bot added the pending-release Fix or implementation already in dev waiting to be released label Oct 28, 2022
@github-actions
Copy link
Contributor

This is now released under 2.1.0 version!

@github-actions github-actions bot removed the pending-release Fix or implementation already in dev waiting to be released label Oct 31, 2022
@dmwesterhoff
Copy link
Author

Sorry for late reply, just seeing this now -- I was unsure of if this was unintended side effect (bug) or a feature :). Thanks for the swift resolution, much appreciated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
2 participants