powertools-lambda-python icon indicating copy to clipboard operation
powertools-lambda-python copied to clipboard

refactor(event_source): fix type hint

Open robmoss2k opened this issue 1 year ago • 8 comments

Issue number: #3568

Summary

Changes

Add the Any return type to the event_source function definition. The handler argument already specifies that it will return Any and event_source returns whatever handler returns.

User experience

mypy --strict won't throw error: Untyped decorator makes function "handler" untyped [misc] any longer.

Checklist

If your change doesn't seem to apply, please leave them unchecked.

Is this a breaking change?

RFC issue number:

Checklist:

  • [ ] Migration process documented
  • [ ] Implement warnings (if it can live side by side)

Acknowledgment

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.

robmoss2k avatar Apr 19 '24 12:04 robmoss2k

Thanks a lot for your first contribution! Please check out our contributing guidelines and don't hesitate to ask whatever you need. In the meantime, check out the #python channel on our Powertools for AWS Lambda Discord: Invite link

boring-cyborg[bot] avatar Apr 19 '24 12:04 boring-cyborg[bot]

Fixes #3568.

robmoss2k avatar Apr 19 '24 12:04 robmoss2k

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 96.16%. Comparing base (e14e768) to head (a3960b3). Report is 326 commits behind head on develop.

:exclamation: Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #4165      +/-   ##
===========================================
- Coverage    96.38%   96.16%   -0.22%     
===========================================
  Files          214      218       +4     
  Lines        10030    10459     +429     
  Branches      1846     1934      +88     
===========================================
+ Hits          9667    10058     +391     
- Misses         259      283      +24     
- Partials       104      118      +14     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov-commenter avatar Apr 19 '24 16:04 codecov-commenter

Hello @robmoss2k! Thanks for submitting this PR and putting effort to solve this problem.

Can you export the libs that are installed in your virtual environment and how you are running the tests? I'm still having problems here with strict check in mypy.

My tests applying this fix:

requirements.txt

aws-lambda-powertools==2.36.0
aws-xray-sdk==2.12.1
boto3==1.34.10
botocore==1.34.10
fastjsonschema==2.19.1
jmespath==0.10.0
mypy==1.8.0
mypy-extensions==1.0.0
python-dateutil==2.8.2
s3transfer==0.10.0
six==1.16.0
tomli==2.0.1
typing_extensions==4.9.0
urllib3==1.26.18
wrapt==1.16.0

main.py

from typing import Any
from aws_lambda_powertools.utilities.data_classes import SQSEvent, event_source
from aws_lambda_powertools.utilities.typing import LambdaContext

@event_source(data_class=SQSEvent)
def handler(event: SQSEvent, _context: LambdaContext) -> dict[str, Any]:
    return {}

running mypy

(.venv) ➜  leo mypy -V
mypy 1.8.0 (compiled: yes)
(.venv) ➜  leo mypy --strict main.py
main.py:5: error: Untyped decorator makes function "handler" untyped  [misc]
Found 1 error in 1 file (checked 1 source file)

Thanks and please let me know if I'm missing something.

leandrodamascena avatar Apr 19 '24 17:04 leandrodamascena

It seems this change is breaking a lot of things on our CI

rubenfonseca avatar Apr 25 '24 13:04 rubenfonseca

It seems this change is breaking a lot of things on our CI

It's not finished. I need a solid block of time to spend on it. Don't approve it before it's finished. I will get back to it. Feels like I've bitten off more than I can chew... But I don't like to lose.

robmoss2k avatar Apr 25 '24 17:04 robmoss2k

No worries, it's a hard task, as not everything can be made into strict typing -- for the decorator factory, the new ParamSpec type will be of great help.

heitorlessa avatar May 07 '24 08:05 heitorlessa

Hello @robmoss2k! Thank you for putting in the effort here and helping us make the project more strict/strongly typed, which is a big challenge, we really appreciate your effort. But we are closing this PR as not planned for now.

We are about to release Powertools v3 and we will not prioritize that at the moment as we will put v2 in maintenance mode soon. Another thing is that refactoring method signatures and introducing changes to such important utilities can create some unexpected side effects, which we don't want at this point.

For Powertools v4, we plan to refactor the modules and make the project more strict typed.

Again, thanks for your time here and feel free to reopen if you want.

leandrodamascena avatar Jul 09 '24 09:07 leandrodamascena