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

Docs: Invalid return type in middleware example #3556

Closed
1 task done
kamilturek opened this issue Dec 23, 2023 · 4 comments · Fixed by #3569
Closed
1 task done

Docs: Invalid return type in middleware example #3556

kamilturek opened this issue Dec 23, 2023 · 4 comments · Fixed by #3569
Assignees
Labels
documentation Improvements or additions to documentation middleware_factory Middleware factory utility

Comments

@kamilturek
Copy link
Contributor

What were you searching in the docs?

All examples on the middleware factory page present a middleware function with return type of Callable which seems incorrect.

image

The actual return type of the middleware_before function is the same as the one of the lambda handler - dict this case. Only after the function is decorated, it'll be a Callable but I believe the function definition should anyway have the return type of dict.

A small reproducible example:

from typing import Callable

from aws_lambda_powertools.middleware_factory import lambda_handler_decorator

LambdaHandler = Callable[[dict, dict], dict]


@lambda_handler_decorator
def middleware_add_headers(
    handler: LambdaHandler, event: dict, context: dict
) -> Callable:
    return handler(event, context)


@middleware_add_headers
def lambda_handler(event: dict, context: dict) -> dict:
    return {
        "statusCode": 200,
        "body": "hello",
    }

Mypy output:

$ mypy main.py
handler.py:12: error: Incompatible return value type (got "dict[Any, Any]", expected "Callable[..., Any]")  [return-value]
Found 1 error in 1 file (checked 1 source file)

Is this related to an existing documentation section?

https://docs.powertools.aws.dev/lambda/python/latest/utilities/middleware_factory/#middleware-with-before-logic

How can we improve?

All invalid examples should be updated with correct return types (seems like dict in all cases).

Got a suggestion in mind?

I'm happy to help with resolving this issue.

Acknowledgment

  • I understand the final update might be different from my proposed suggestion, or refused.
@kamilturek kamilturek added documentation Improvements or additions to documentation triage Pending triage from maintainers labels Dec 23, 2023
Copy link

boring-cyborg bot commented Dec 23, 2023

Thanks for opening your first issue here! We'll come back to you as soon as we can.
In the meantime, check out the #python channel on our Powertools for AWS Lambda Discord: Invite link

@leandrodamascena
Copy link
Contributor

Hello @kamilturek! Thanks for opening this issue! Static type checking in Python is sometimes challenging.

We use mypy to check files in the aws_lambda_powertools and examples folders and if any file has a problem with static checking, our CI fails, but in this case, it doesn't fail and I was inquisitive about this.

I'm not the biggest mypy expert out there, but from what I understand, our examples don't fail because we're not noting typing the handler parameter in middleware_add_headers, and as a result, mypy can't correctly infer the type of the return from this execution. I changed one of the examples so that lambda_handler has a more specific type (just like you) and now I can catch the error.

Would you like to open a PR to fix this? We try to make our documentation as assertive as possible for our customers; this case is certainly something we need to fix quickly.

thanks.

@leandrodamascena leandrodamascena self-assigned this Dec 26, 2023
@leandrodamascena leandrodamascena added middleware_factory Middleware factory utility and removed triage Pending triage from maintainers labels Dec 26, 2023
@kamilturek
Copy link
Contributor Author

Hey @leandrodamascena, thanks for looking into this and verifying my report.

I'd be very happy to create a PR. Will do it soon.

Thanks.

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.

@leandrodamascena leandrodamascena moved this from Coming soon to Shipped in Powertools for AWS Lambda (Python) Mar 18, 2024
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 middleware_factory Middleware factory utility
Projects
Status: Shipped
Development

Successfully merging a pull request may close this issue.

2 participants