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

Missing return type in lambda_handler_decorator leads to 'Untyped decorator makes function "..." untyped' errors in mypy strict mode #1060

Closed
huonw opened this issue Mar 7, 2022 · 4 comments
Labels
bug Something isn't working p3

Comments

@huonw
Copy link
Contributor

huonw commented Mar 7, 2022

What were you trying to accomplish?

I'm trying to use aws_lambda_powertools.utilities.data_classes.event_source with mypy --strict, and receive errors about Untyped decorator makes function "handler" untyped. This is because lambda_handler_decorator is missing a return value, so mypy doesn't know anything about any function decorated with it (including event_source):

https://github.com/awslabs/aws-lambda-powertools-python/blob/22f8b9a7f47a2a4682c6f3f03536ac848ae7dc0d/aws_lambda_powertools/middleware_factory/factory.py#L15

Expected Behavior

@event_source and other decorators using @lambda_handler_decorator should work in mypy strict mode.

Current Behavior

Mypy emits errors.

Possible Solution

  1. Add an appropriate Callable[something, here] return value to lambda_handler_decorator
  2. Run mypy --strict in this package's CI

Steps to Reproduce (for bugs)

requirements:

aws-lambda-powertools==1.25.1
aws-xray-sdk==2.9.0
boto3==1.21.13
botocore==1.24.13
fastjsonschema==2.15.3
future==0.18.2
jmespath==0.10.0
mypy-extensions==0.4.3
mypy==0.931
python-dateutil==2.8.2
s3transfer==0.5.2
six==1.16.0
tomli==2.0.1
typing-extensions==4.1.1
urllib3==1.26.8
wrapt==1.13.3

input.py:

from typing import Any

from aws_lambda_powertools.middleware_factory import lambda_handler_decorator

from aws_lambda_powertools.utilities.data_classes import (
    event_source,
    KinesisStreamEvent,
)


@lambda_handler_decorator
def custom_decorator() -> None:
    return


@event_source(data_class=KinesisStreamEvent)
@custom_decorator
def handler(event: KinesisStreamEvent, context: Any) -> None:
    pass

invocation:

$ mypy --strict input.py 
input.py:16: error: Untyped decorator makes function "handler" untyped
input.py:17: error: Untyped decorator makes function "handler" untyped

Environment

  • Powertools version used: 1.25.1
  • Packaging format (Layers, PyPi): N/A
  • AWS Lambda function runtime: N/A
  • Debugging logs N/A
@huonw huonw added bug Something isn't working triage Pending triage from maintainers labels Mar 7, 2022
@heitorlessa
Copy link
Contributor

hey @huonw thanks for yet another high quality bug report! We should definitely go for option 1. This utility predates our experience with Mypy. It's also due for a refresh on docs, and making a generic decorator factory for any sync Python function.

The reason we didn't choose --strict is that it'd raise the bar to contributions, as we've noticed (back then) some of the errors were too picky.

Are you able to run without strict until we get other high priority fixes in place? I'd estimate looking into this next week.

Thanks again!

@huonw
Copy link
Contributor Author

huonw commented Mar 7, 2022

Ah, that's fair enough. I've done two attempts in #1066: first commit is a more complex version that I couldn't get to work, while the second undoes most of it to make a small improvement.

As a middle-ground between the current state and --strict, one could activate strict on key files/modules (not sure if that is possible), or just enable specific additional strictnesses. For instance, disallow_untyped_defs would've caught this by ensuring that lambda_handler_decorator has a return type. Arguably, since this is a library, it'd be good to have that set to ensure that the public API is fully typed. (I'm not sure if one can relax it for _'d functions. 🤷‍♂️)

@heitorlessa
Copy link
Contributor

hey @huonw I haven't forgotten this - fixing some important bugs on Logger and Logger utils, and will start reviewing this.

I'd like to run a full scan using --strict to see how much it's nitpick vs actual using disallow_untyped_defs only. I know @ran-isenberg ran into a similar issue on Parameters which I'm still puzzled by it. Whatever it happens, we should change re-evaluate our Mypy settings and strike a balance on easing contributions and strictness - --strict does get us closer to use something like Mypyc, or even Cython in a remote future for areas that require strict performance

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

github-actions bot commented Apr 8, 2022

This is now released under 1.25.7 version!

@github-actions github-actions bot closed this as completed Apr 8, 2022
@github-actions github-actions bot removed the pending-release Fix or implementation already in dev waiting to be released label Apr 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working p3
Projects
None yet
Development

No branches or pull requests

2 participants