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

Idempotent documentation examples could be misleading #657

Closed
walmsles opened this issue Aug 27, 2021 · 6 comments
Closed

Idempotent documentation examples could be misleading #657

walmsles opened this issue Aug 27, 2021 · 6 comments
Assignees
Labels
documentation Improvements or additions to documentation p1
Milestone

Comments

@walmsles
Copy link
Contributor

walmsles commented Aug 27, 2021

What were you initially searching for in the docs?
How to use the Idempotent utility

Is this related to an existing part of the documentation? Please share a link
Link to documentation section

Describe how we could make it clearer

The example uses the following config:

config = IdempotencyConfig(event_key_jmespath="body")

What I feel is missing from the documentation is that the Idempotent decorator does not do any type coersion on the event_key_jmespath so this data hash key used in dynamodb is based on the String data in the event body attribute.

For a standard APIGatewayProxyEvent for a json REST Api that means the following body strings are different even though logically the API Json payloads are actually identical (in API terms). If I follow the example verbatim the following body values will be seen as different API invocations:

{
   "more_data": "more data 1",
"data": "test message 1"
}
{
   "more_data": "more data 1",
   "data": "test message 1"
}
{

   "more_data": "more data 1",







"data": "test message 1"

}

maps to a body of "{\n\n "more_data": "more data 1",\n\n\n\n\n\n\n\n"data": "test message 1"\n\n}" in the API Gateway event (i.e. includes all the white space).

I feel the example should include the event source for API Gateway events resulting in the following:

import json
from aws_lambda_powertools.utilities.idempotency import (
    IdempotencyConfig, DynamoDBPersistenceLayer, idempotent
)
from aws_lambda_powertools.utilities.data_classes import (
    event_source, APIGatewayProxyEvent
)

persistence_layer = DynamoDBPersistenceLayer(table_name="IdempotencyTable")

config = IdempotencyConfig(event_key_jmespath="json_body")
@event_source(data_class=APIGatewayProxyEvent)
@idempotent(config=config, persistence_store=persistence_layer)
def handler(event:APIGatewayProxyEvent, context):
    body = json.loads(event['body'])
    payment = create_subscription_payment(
        user=body['user'],
        product=body['product_id']
    )
    ...
    return {
        "payment_id": payment.id,
        "message": "success",
        "statusCode": 200
    }

If you have a proposed update, please share it here
Maybe change the example to include the event_source coersion to stop people copying that example thinking their API will be idempotent - when it will when the exact body string is used. Its not obvious from the documentation (or maybe its just me)

@walmsles walmsles added the documentation Improvements or additions to documentation label Aug 27, 2021
@walmsles walmsles changed the title Idempo Idempotent docuemntation examples could be misleading Aug 27, 2021
@walmsles walmsles changed the title Idempotent docuemntation examples could be misleading Idempotent documentation examples could be misleading Aug 27, 2021
@walmsles
Copy link
Contributor Author

Happy to craft an update if you feel its warranted

@walmsles
Copy link
Contributor Author

Can also use config as follows:

config = IdempotencyConfig(event_key_jmespath="powertools_json(body)")

which is simpler for a json REST API

@heitorlessa
Copy link
Contributor

heitorlessa commented Aug 27, 2021 via email

@walmsles
Copy link
Contributor Author

Give me a couple of days and will look at adding some use cases and additional explanation around using Idempotent utility based on the above and include use of powertools_json. powertools_json is mentioned at the end of the idmepotent page so you didn't forget it - just not obvious at first. I created a simple app based on the example and was re-testing the fix for #638 and had it still failing so decided to delve a lot deeper and arrived here (the fix is fine which I knew).

I notice there is a separate issue #631 for testing and idempotency doc - so let me work on expanding examples in docs for this issue I raised.

Go back to your holiday I will submit something in a few days - Documentation is hard

@to-mc
Copy link
Contributor

to-mc commented Sep 27, 2021

Thanks @walmsles! I've had a look at the PR and requested a few small changes there.

@heitorlessa heitorlessa added this to the 1.21.0 milestone Sep 28, 2021
@heitorlessa
Copy link
Contributor

Closing this now as it's available in the staged docs: https://awslabs.github.io/aws-lambda-powertools-python/develop/utilities/idempotency/

We'll work on exposing JMESPath Functions to all customers along with our handy functions, so they can use them like we do anywhere in their code.

Thank you so much for the help again @walmsles !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation p1
Projects
None yet
Development

No branches or pull requests

3 participants